[PATCH 00/11] kvm tools: cleanups

[PATCH 00/11] kvm tools: cleanups

[Lai Jiangshan]

There are small patches and almost are cleanups.
patch 1 and patch 10 may be 2 fixies if I'm not mental disorder.

Lai Jiangshan (11):
  check condition before wait
  fix function name
  move threadpool.c to util/
  move rbtree.o to util/
  add missing build output
  move read-write.c to util/
  move util.c to util/
  move strlcat() to util/strbuf.c
  narrow the C.S. of compat_mtx
  protect shared global id in compat_mtx C.S.
  remove duplicated code

 tools/kvm/Makefile                |   11 +++++----
 tools/kvm/builtin-run.c           |    1 +
 tools/kvm/guest_compat.c          |   39 ++++++++++++++++--------------------
 tools/kvm/include/kvm/strbuf.h    |   13 ++++++++++++
 tools/kvm/include/kvm/util.h      |   10 ---------
 tools/kvm/{ => util}/read-write.c |    0
 tools/kvm/util/strbuf.c           |   26 ++++++++++++++++++++++++
 tools/kvm/{ => util}/threadpool.c |   25 +++++++++++------------
 tools/kvm/{ => util}/util.c       |   25 -----------------------
 9 files changed, 75 insertions(+), 75 deletions(-)
 rename tools/kvm/{ => util}/read-write.c (100%)
 rename tools/kvm/{ => util}/threadpool.c (89%)
 rename tools/kvm/{ => util}/util.c (71%)

-- 
1.7.4.4

[PATCH 01/11] kvm tools: check condition before wait

[Lai Jiangshan]

We should check the condition before wait it,
otherewise we may get deadlock.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/threadpool.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/threadpool.c b/tools/kvm/threadpool.c
index fdc5fa7..2e59e62 100644
--- a/tools/kvm/threadpool.c
+++ b/tools/kvm/threadpool.c
@@ -80,12 +80,11 @@ static void *thread_pool__threadfunc(void *param)
 		struct thread_pool__job *curjob;
 
 		mutex_lock(&job_mutex);
-		pthread_cond_wait(&job_cond, &job_mutex);
-		curjob = thread_pool__job_pop();
+		while ((curjob = thread_pool__job_pop()) == NULL)
+			pthread_cond_wait(&job_cond, &job_mutex);
 		mutex_unlock(&job_mutex);
 
-		if (curjob)
-			thread_pool__handle_job(curjob);
+		thread_pool__handle_job(curjob);
 	}
 
 	pthread_cleanup_pop(0);
-- 
1.7.4.4

[PATCH 02/11] kvm tools: fix function name

[Lai Jiangshan]

In here, xxx_locked() should means "called with job_mutex locked",
the original funtion are opposite, fix them.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/threadpool.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/kvm/threadpool.c b/tools/kvm/threadpool.c
index 2e59e62..bafbcd7 100644
--- a/tools/kvm/threadpool.c
+++ b/tools/kvm/threadpool.c
@@ -15,7 +15,7 @@ static LIST_HEAD(head);
 static pthread_t	*threads;
 static long		threadcount;
 
-static struct thread_pool__job *thread_pool__job_pop(void)
+static struct thread_pool__job *thread_pool__job_pop_locked(void)
 {
 	struct thread_pool__job *job;
 
@@ -28,25 +28,25 @@ static struct thread_pool__job *thread_pool__job_pop(void)
 	return job;
 }
 
-static void thread_pool__job_push(struct thread_pool__job *job)
+static void thread_pool__job_push_locked(struct thread_pool__job *job)
 {
 	list_add_tail(&job->queue, &head);
 }
 
-static struct thread_pool__job *thread_pool__job_pop_locked(void)
+static struct thread_pool__job *thread_pool__job_pop(void)
 {
 	struct thread_pool__job *job;
 
 	mutex_lock(&job_mutex);
-	job = thread_pool__job_pop();
+	job = thread_pool__job_pop_locked();
 	mutex_unlock(&job_mutex);
 	return job;
 }
 
-static void thread_pool__job_push_locked(struct thread_pool__job *job)
+static void thread_pool__job_push(struct thread_pool__job *job)
 {
 	mutex_lock(&job_mutex);
-	thread_pool__job_push(job);
+	thread_pool__job_push_locked(job);
 	mutex_unlock(&job_mutex);
 }
 
@@ -59,11 +59,11 @@ static void thread_pool__handle_job(struct thread_pool__job *job)
 
 		if (--job->signalcount > 0)
 			/* If the job was signaled again while we were working */
-			thread_pool__job_push_locked(job);
+			thread_pool__job_push(job);
 
 		mutex_unlock(&job->mutex);
 
-		job = thread_pool__job_pop_locked();
+		job = thread_pool__job_pop();
 	}
 }
 
@@ -80,7 +80,7 @@ static void *thread_pool__threadfunc(void *param)
 		struct thread_pool__job *curjob;
 
 		mutex_lock(&job_mutex);
-		while ((curjob = thread_pool__job_pop()) == NULL)
+		while ((curjob = thread_pool__job_pop_locked()) == NULL)
 			pthread_cond_wait(&job_cond, &job_mutex);
 		mutex_unlock(&job_mutex);
 
@@ -136,7 +136,7 @@ void thread_pool__do_job(struct thread_pool__job *job)
 
 	mutex_lock(&jobinfo->mutex);
 	if (jobinfo->signalcount++ == 0)
-		thread_pool__job_push_locked(job);
+		thread_pool__job_push(job);
 	mutex_unlock(&jobinfo->mutex);
 
 	mutex_lock(&job_mutex);
-- 
1.7.4.4

[PATCH 03/11] kvm tools: move threadpool.c to util/

[Lai Jiangshan]

threadpool.c contains no kvm related code, it is just a lib.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/Makefile                |    2 +-
 tools/kvm/{ => util}/threadpool.c |    0
 2 files changed, 1 insertions(+), 1 deletions(-)
 rename tools/kvm/{ => util}/threadpool.c (100%)

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index cef3c44..b0bb266 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -76,7 +76,7 @@ OBJS	+= net/uip/csum.o
 OBJS	+= net/uip/dhcp.o
 OBJS	+= kvm-cmd.o
 OBJS	+= rbtree.o
-OBJS	+= threadpool.o
+OBJS	+= util/threadpool.o
 OBJS	+= util/parse-options.o
 OBJS	+= util/rbtree-interval.o
 OBJS	+= util/strbuf.o
diff --git a/tools/kvm/threadpool.c b/tools/kvm/util/threadpool.c
similarity index 100%
rename from tools/kvm/threadpool.c
rename to tools/kvm/util/threadpool.c
-- 
1.7.4.4

[PATCH 04/11] kvm tools: move rbtree.o to util/

[Lai Jiangshan]

rbtree.o is just a lib for kvm.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index b0bb266..9ddc3b3 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -75,7 +75,7 @@ OBJS	+= net/uip/buf.o
 OBJS	+= net/uip/csum.o
 OBJS	+= net/uip/dhcp.o
 OBJS	+= kvm-cmd.o
-OBJS	+= rbtree.o
+OBJS	+= util/rbtree.o
 OBJS	+= util/threadpool.o
 OBJS	+= util/parse-options.o
 OBJS	+= util/rbtree-interval.o
@@ -240,7 +240,7 @@ builtin-help.d: $(KVM_INCLUDE)/common-cmds.h
 
 $(OBJS):
 
-rbtree.o: ../../lib/rbtree.c
+util/rbtree.o: ../../lib/rbtree.c
 	$(Q) $(CC) -c $(CFLAGS) $< -o $@
 
 %.o: %.c
-- 
1.7.4.4

[PATCH 05/11] kvm tools: add missing build output

[Lai Jiangshan]

The missing "CC       util/rbtree.o" will be output when building.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index 9ddc3b3..aadfe25 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -241,6 +241,7 @@ builtin-help.d: $(KVM_INCLUDE)/common-cmds.h
 $(OBJS):
 
 util/rbtree.o: ../../lib/rbtree.c
+	$(E) "  CC      " $@
 	$(Q) $(CC) -c $(CFLAGS) $< -o $@
 
 %.o: %.c
-- 
1.7.4.4

[PATCH 06/11] kvm tools: move read-write.c to util/

[Lai Jiangshan]

read-write.c contains no kvm related code, it is just a lib.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/Makefile                |    2 +-
 tools/kvm/{ => util}/read-write.c |    0
 2 files changed, 1 insertions(+), 1 deletions(-)
 rename tools/kvm/{ => util}/read-write.c (100%)

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index aadfe25..bf0c9b4 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -50,7 +50,6 @@ OBJS	+= kvm.o
 OBJS	+= main.o
 OBJS	+= mmio.o
 OBJS	+= pci.o
-OBJS	+= read-write.o
 OBJS	+= term.o
 OBJS	+= util.o
 OBJS	+= virtio/blk.o
@@ -80,6 +79,7 @@ OBJS	+= util/threadpool.o
 OBJS	+= util/parse-options.o
 OBJS	+= util/rbtree-interval.o
 OBJS	+= util/strbuf.o
+OBJS	+= util/read-write.o
 OBJS	+= virtio/9p.o
 OBJS	+= virtio/9p-pdu.o
 OBJS	+= hw/vesa.o
diff --git a/tools/kvm/read-write.c b/tools/kvm/util/read-write.c
similarity index 100%
rename from tools/kvm/read-write.c
rename to tools/kvm/util/read-write.c
-- 
1.7.4.4

[PATCH 07/11] kvm tools: move util.c to util/

[Lai Jiangshan]

util.c is a lib for kvm.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/Makefile          |    2 +-
 tools/kvm/{ => util}/util.c |    0
 2 files changed, 1 insertions(+), 1 deletions(-)
 rename tools/kvm/{ => util}/util.c (100%)

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index bf0c9b4..56bdb96 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -51,7 +51,6 @@ OBJS	+= main.o
 OBJS	+= mmio.o
 OBJS	+= pci.o
 OBJS	+= term.o
-OBJS	+= util.o
 OBJS	+= virtio/blk.o
 OBJS	+= virtio/console.o
 OBJS	+= virtio/core.o
@@ -80,6 +79,7 @@ OBJS	+= util/parse-options.o
 OBJS	+= util/rbtree-interval.o
 OBJS	+= util/strbuf.o
 OBJS	+= util/read-write.o
+OBJS	+= util/util.o
 OBJS	+= virtio/9p.o
 OBJS	+= virtio/9p-pdu.o
 OBJS	+= hw/vesa.o
diff --git a/tools/kvm/util.c b/tools/kvm/util/util.c
similarity index 100%
rename from tools/kvm/util.c
rename to tools/kvm/util/util.c
-- 
1.7.4.4

[PATCH 08/11] kvm tools: move strlcat() to util/strbuf.c

[Lai Jiangshan]

strlcat() is a string related function.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/builtin-run.c        |    1 +
 tools/kvm/include/kvm/strbuf.h |   13 +++++++++++++
 tools/kvm/include/kvm/util.h   |   10 ----------
 tools/kvm/util/strbuf.c        |   26 ++++++++++++++++++++++++++
 tools/kvm/util/util.c          |   25 -------------------------
 5 files changed, 40 insertions(+), 35 deletions(-)

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 76ce657..5afc8c0 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -21,6 +21,7 @@
 #include "kvm/mutex.h"
 #include "kvm/term.h"
 #include "kvm/util.h"
+#include "kvm/strbuf.h"
 #include "kvm/vesa.h"
 #include "kvm/irq.h"
 #include "kvm/kvm.h"
diff --git a/tools/kvm/include/kvm/strbuf.h b/tools/kvm/include/kvm/strbuf.h
index e67ca20..8fdef7e 100644
--- a/tools/kvm/include/kvm/strbuf.h
+++ b/tools/kvm/include/kvm/strbuf.h
@@ -1,6 +1,19 @@
 #ifndef __STRBUF_H__
 #define __STRBUF_H__
 
+#include <sys/types.h>
+#include <string.h>
+
 int prefixcmp(const char *str, const char *prefix);
 
+extern size_t strlcat(char *dest, const char *src, size_t count);
+
+/* some inline functions */
+
+static inline const char *skip_prefix(const char *str, const char *prefix)
+{
+	size_t len = strlen(prefix);
+	return strncmp(str, prefix, len) ? NULL : str + len;
+}
+
 #endif
diff --git a/tools/kvm/include/kvm/util.h b/tools/kvm/include/kvm/util.h
index dc2e0b9..3e382fc 100644
--- a/tools/kvm/include/kvm/util.h
+++ b/tools/kvm/include/kvm/util.h
@@ -58,16 +58,6 @@ do {								\
 		__stringify(cnd) "\n");				\
 } while (0)
 
-extern size_t strlcat(char *dest, const char *src, size_t count);
-
-/* some inline functions */
-
-static inline const char *skip_prefix(const char *str, const char *prefix)
-{
-	size_t len = strlen(prefix);
-	return strncmp(str, prefix, len) ? NULL : str + len;
-}
-
 #define MSECS_TO_USECS(s) ((s) * 1000)
 
 /* Millisecond sleep */
diff --git a/tools/kvm/util/strbuf.c b/tools/kvm/util/strbuf.c
index ec77ab1..6632a14 100644
--- a/tools/kvm/util/strbuf.c
+++ b/tools/kvm/util/strbuf.c
@@ -1,5 +1,6 @@
 
 /* user defined headers */
+#include <kvm/util.h>
 #include <kvm/strbuf.h>
 
 int prefixcmp(const char *str, const char *prefix)
@@ -11,3 +12,28 @@ int prefixcmp(const char *str, const char *prefix)
 			return (unsigned char)*prefix - (unsigned char)*str;
 	}
 }
+
+/**
+ * strlcat - Append a length-limited, %NUL-terminated string to another
+ * @dest: The string to be appended to
+ * @src: The string to append to it
+ * @count: The size of the destination buffer.
+ */
+size_t strlcat(char *dest, const char *src, size_t count)
+{
+	size_t dsize = strlen(dest);
+	size_t len = strlen(src);
+	size_t res = dsize + len;
+
+	DIE_IF(dsize >= count);
+
+	dest += dsize;
+	count -= dsize;
+	if (len >= count)
+		len = count - 1;
+
+	memcpy(dest, src, len);
+	dest[len] = 0;
+
+	return res;
+}
diff --git a/tools/kvm/util/util.c b/tools/kvm/util/util.c
index 4efbce9..682ed6c 100644
--- a/tools/kvm/util/util.c
+++ b/tools/kvm/util/util.c
@@ -74,28 +74,3 @@ void die_perror(const char *s)
 	perror(s);
 	exit(1);
 }
-
-/**
- * strlcat - Append a length-limited, %NUL-terminated string to another
- * @dest: The string to be appended to
- * @src: The string to append to it
- * @count: The size of the destination buffer.
- */
-size_t strlcat(char *dest, const char *src, size_t count)
-{
-	size_t dsize = strlen(dest);
-	size_t len = strlen(src);
-	size_t res = dsize + len;
-
-	DIE_IF(dsize >= count);
-
-	dest += dsize;
-	count -= dsize;
-	if (len >= count)
-		len = count - 1;
-
-	memcpy(dest, src, len);
-	dest[len] = 0;
-
-	return res;
-}
-- 
1.7.4.4

[PATCH 09/11] kvm tools: narrow the C.S. of compat_mtx

[Lai Jiangshan]

The allocation and the copy are not required in the C.S.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/guest_compat.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/tools/kvm/guest_compat.c b/tools/kvm/guest_compat.c
index fac53f5..de97df5 100644
--- a/tools/kvm/guest_compat.c
+++ b/tools/kvm/guest_compat.c
@@ -21,20 +21,19 @@ int compat__add_message(const char *title, const char *desc)
 {
 	struct compat_message *msg;
 
-	mutex_lock(&compat_mtx);
 	msg = malloc(sizeof(*msg));
 	if (msg == NULL)
 		goto cleanup;
 
-	*msg = (struct compat_message) {
-		.id = id,
-		.title = strdup(title),
-		.desc = strdup(desc),
-	};
+	msg->title = strdup(title);
+	msg->desc = strdup(desc);
 
 	if (msg->title == NULL || msg->desc == NULL)
 		goto cleanup;
 
+	mutex_lock(&compat_mtx);
+
+	msg->id = id;
 	list_add_tail(&msg->list, &messages);
 
 	mutex_unlock(&compat_mtx);
@@ -48,8 +47,6 @@ cleanup:
 		free(msg);
 	}
 
-	mutex_unlock(&compat_mtx);
-
 	return -ENOMEM;
 }
 
@@ -101,4 +98,4 @@ int compat__print_all_messages(void)
 	mutex_unlock(&compat_mtx);
 
 	return 0;
-}
\ No newline at end of file
+}
-- 
1.7.4.4

[PATCH 10/11] kvm tools: protect shared global id in compat_mtx C.S.

[Lai Jiangshan]

The global id is shared, protect it when access it.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/guest_compat.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/guest_compat.c b/tools/kvm/guest_compat.c
index de97df5..8dd451b 100644
--- a/tools/kvm/guest_compat.c
+++ b/tools/kvm/guest_compat.c
@@ -20,6 +20,7 @@ static LIST_HEAD(messages);
 int compat__add_message(const char *title, const char *desc)
 {
 	struct compat_message *msg;
+	int msg_id;
 
 	msg = malloc(sizeof(*msg));
 	if (msg == NULL)
@@ -33,12 +34,12 @@ int compat__add_message(const char *title, const char *desc)
 
 	mutex_lock(&compat_mtx);
 
-	msg->id = id;
+	msg->id = msg_id = id++;
 	list_add_tail(&msg->list, &messages);
 
 	mutex_unlock(&compat_mtx);
 
-	return id++;
+	return msg_id;
 
 cleanup:
 	if (msg) {
-- 
1.7.4.4

[PATCH 11/11] kvm tools: remove duplicated code

[Lai Jiangshan]

Simple cleanup.

Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
 tools/kvm/guest_compat.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/tools/kvm/guest_compat.c b/tools/kvm/guest_compat.c
index 8dd451b..43b9035 100644
--- a/tools/kvm/guest_compat.c
+++ b/tools/kvm/guest_compat.c
@@ -17,6 +17,13 @@ static int id;
 static DEFINE_MUTEX(compat_mtx);
 static LIST_HEAD(messages);
 
+static void compat__free(struct compat_message *msg)
+{
+	free(msg->title);
+	free(msg->desc);
+	free(msg);
+}
+
 int compat__add_message(const char *title, const char *desc)
 {
 	struct compat_message *msg;
@@ -42,22 +49,12 @@ int compat__add_message(const char *title, const char *desc)
 	return msg_id;
 
 cleanup:
-	if (msg) {
-		free(msg->title);
-		free(msg->desc);
-		free(msg);
-	}
+	if (msg)
+		compat__free(msg);
 
 	return -ENOMEM;
 }
 
-static void compat__free(struct compat_message *msg)
-{
-	free(msg->title);
-	free(msg->desc);
-	free(msg);
-}
-
 int compat__remove_message(int id)
 {
 	struct compat_message *pos, *n;
-- 
1.7.4.4