* [Qemu-devel] [PULL 1/8] glib-compat.h: add new thread API emulation on top of pre-2.31 API
2014-06-10 5:56 [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Paolo Bonzini
@ 2014-06-10 5:56 ` Paolo Bonzini
2014-06-10 5:56 ` [Qemu-devel] [PULL 2/8] vscclient: use glib thread primitives not qemu Paolo Bonzini
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-10 5:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tokarev
From: Michael Tokarev <mjt@tls.msk.ru>
Thread API changed in glib-2.31 significantly. Before that version,
conditionals and mutexes were only allocated dynamically, using
_new()/_free() interface. in 2.31 and up, they're allocated statically
as regular variables, and old interface is deprecated.
(Note: glib docs says the new interface is available since version
2.32, but it was actually introduced in version 2.31).
Create the new interface using old primitives, by providing non-opaque
definitions of the base types (GCond and GMutex) using GOnces.
Replace #ifdeffery around GCond and GMutex in trace/simple.c and
coroutine-gthread.c too because it does not work anymore with the new
glib-compat.h.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[Use GOnce to support lazy initialization; introduce CompatGMutex
and CompatGCond. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
coroutine-gthread.c | 29 ++++--------
include/glib-compat.h | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++
trace/simple.c | 50 +++++----------------
3 files changed, 138 insertions(+), 60 deletions(-)
diff --git a/coroutine-gthread.c b/coroutine-gthread.c
index a61efe0..6bd6d6b 100644
--- a/coroutine-gthread.c
+++ b/coroutine-gthread.c
@@ -30,20 +30,14 @@ typedef struct {
CoroutineAction action;
} CoroutineGThread;
-static GStaticMutex coroutine_lock = G_STATIC_MUTEX_INIT;
+static CompatGMutex coroutine_lock;
+static CompatGCond coroutine_cond;
/* GLib 2.31 and beyond deprecated various parts of the thread API,
* but the new interfaces are not available in older GLib versions
* so we have to cope with both.
*/
#if GLIB_CHECK_VERSION(2, 31, 0)
-/* Default zero-initialisation is sufficient for 2.31+ GCond */
-static GCond the_coroutine_cond;
-static GCond *coroutine_cond = &the_coroutine_cond;
-static inline void init_coroutine_cond(void)
-{
-}
-
/* Awkwardly, the GPrivate API doesn't provide a way to update the
* GDestroyNotify handler for the coroutine key dynamically. So instead
* we track whether or not the CoroutineGThread should be freed on
@@ -84,11 +78,6 @@ static inline GThread *create_thread(GThreadFunc func, gpointer data)
#else
/* Handle older GLib versions */
-static GCond *coroutine_cond;
-static inline void init_coroutine_cond(void)
-{
- coroutine_cond = g_cond_new();
-}
static GStaticPrivate coroutine_key = G_STATIC_PRIVATE_INIT;
@@ -120,22 +109,20 @@ static void __attribute__((constructor)) coroutine_init(void)
g_thread_init(NULL);
}
#endif
-
- init_coroutine_cond();
}
static void coroutine_wait_runnable_locked(CoroutineGThread *co)
{
while (!co->runnable) {
- g_cond_wait(coroutine_cond, g_static_mutex_get_mutex(&coroutine_lock));
+ g_cond_wait(&coroutine_cond, &coroutine_lock);
}
}
static void coroutine_wait_runnable(CoroutineGThread *co)
{
- g_static_mutex_lock(&coroutine_lock);
+ g_mutex_lock(&coroutine_lock);
coroutine_wait_runnable_locked(co);
- g_static_mutex_unlock(&coroutine_lock);
+ g_mutex_unlock(&coroutine_lock);
}
static gpointer coroutine_thread(gpointer opaque)
@@ -177,17 +164,17 @@ CoroutineAction qemu_coroutine_switch(Coroutine *from_,
CoroutineGThread *from = DO_UPCAST(CoroutineGThread, base, from_);
CoroutineGThread *to = DO_UPCAST(CoroutineGThread, base, to_);
- g_static_mutex_lock(&coroutine_lock);
+ g_mutex_lock(&coroutine_lock);
from->runnable = false;
from->action = action;
to->runnable = true;
to->action = action;
- g_cond_broadcast(coroutine_cond);
+ g_cond_broadcast(&coroutine_cond);
if (action != COROUTINE_TERMINATE) {
coroutine_wait_runnable_locked(from);
}
- g_static_mutex_unlock(&coroutine_lock);
+ g_mutex_unlock(&coroutine_lock);
return from->action;
}
diff --git a/include/glib-compat.h b/include/glib-compat.h
index 1280fb2..4ae0671 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -5,6 +5,8 @@
*
* Authors:
* Anthony Liguori <aliguori@us.ibm.com>
+ * Michael Tokarev <mjt@tls.msk.ru>
+ * Paolo Bonzini <pbonzini@redhat.com>
*
* This work is licensed under the terms of the GNU GPL, version 2 or later.
* See the COPYING file in the top-level directory.
@@ -43,4 +45,121 @@ static inline gint g_poll(GPollFD *fds, guint nfds, gint timeout)
}
#endif
+#if !GLIB_CHECK_VERSION(2, 31, 0)
+/* before glib-2.31, GMutex and GCond was dynamic-only (there was a separate
+ * GStaticMutex, but it didn't work with condition variables).
+ *
+ * Our implementation uses GOnce to fake a static implementation that does
+ * not require separate initialization.
+ * We need to rename the types to avoid passing our CompatGMutex/CompatGCond
+ * by mistake to a function that expects GMutex/GCond. However, for ease
+ * of use we keep the GLib function names. GLib uses macros for the
+ * implementation, we use inline functions instead and undefine the macros.
+ */
+
+typedef struct CompatGMutex {
+ GOnce once;
+} CompatGMutex;
+
+typedef struct CompatGCond {
+ GOnce once;
+} CompatGCond;
+
+static inline gpointer do_g_mutex_new(gpointer unused)
+{
+ return (gpointer) g_mutex_new();
+}
+
+static inline void g_mutex_init(CompatGMutex *mutex)
+{
+ mutex->once = (GOnce) G_ONCE_INIT;
+}
+
+static inline void g_mutex_clear(CompatGMutex *mutex)
+{
+ assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
+ if (mutex->once.retval) {
+ g_mutex_free((GMutex *) mutex->once.retval);
+ }
+ mutex->once = (GOnce) G_ONCE_INIT;
+}
+
+static inline void (g_mutex_lock)(CompatGMutex *mutex)
+{
+ g_once(&mutex->once, do_g_mutex_new, NULL);
+ g_mutex_lock((GMutex *) mutex->once.retval);
+}
+#undef g_mutex_lock
+
+static inline gboolean (g_mutex_trylock)(CompatGMutex *mutex)
+{
+ g_once(&mutex->once, do_g_mutex_new, NULL);
+ return g_mutex_trylock((GMutex *) mutex->once.retval);
+}
+#undef g_mutex_trylock
+
+
+static inline void (g_mutex_unlock)(CompatGMutex *mutex)
+{
+ g_mutex_unlock((GMutex *) mutex->once.retval);
+}
+#undef g_mutex_unlock
+
+static inline gpointer do_g_cond_new(gpointer unused)
+{
+ return (gpointer) g_cond_new();
+}
+
+static inline void g_cond_init(CompatGCond *cond)
+{
+ cond->once = (GOnce) G_ONCE_INIT;
+}
+
+static inline void g_cond_clear(CompatGCond *cond)
+{
+ assert(cond->once.status != G_ONCE_STATUS_PROGRESS);
+ if (cond->once.retval) {
+ g_cond_free((GCond *) cond->once.retval);
+ }
+ cond->once = (GOnce) G_ONCE_INIT;
+}
+
+static inline void (g_cond_wait)(CompatGCond *cond, CompatGMutex *mutex)
+{
+ assert(mutex->once.status != G_ONCE_STATUS_PROGRESS);
+ g_once(&cond->once, do_g_cond_new, NULL);
+ g_cond_wait((GCond *) cond->once.retval, (GMutex *) mutex->once.retval);
+}
+#undef g_cond_wait
+
+static inline void (g_cond_broadcast)(CompatGCond *cond)
+{
+ g_once(&cond->once, do_g_cond_new, NULL);
+ g_cond_broadcast((GCond *) cond->once.retval);
+}
+#undef g_cond_broadcast
+
+static inline void (g_cond_signal)(CompatGCond *cond)
+{
+ g_once(&cond->once, do_g_cond_new, NULL);
+ g_cond_signal((GCond *) cond->once.retval);
+}
+#undef g_cond_signal
+
+
+/* before 2.31 there was no g_thread_new() */
+static inline GThread *g_thread_new(const char *name,
+ GThreadFunc func, gpointer data)
+{
+ GThread *thread = g_thread_create(func, data, TRUE, NULL);
+ if (!thread) {
+ g_error("creating thread");
+ }
+ return thread;
+}
+#else
+#define CompatGMutex GMutex
+#define CompatGCond GCond
+#endif /* glib 2.31 */
+
#endif
diff --git a/trace/simple.c b/trace/simple.c
index a8f923c..11ad030 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -40,28 +40,9 @@
* Trace records are written out by a dedicated thread. The thread waits for
* records to become available, writes them out, and then waits again.
*/
-#if GLIB_CHECK_VERSION(2, 32, 0)
-static GMutex trace_lock;
-#define lock_trace_lock() g_mutex_lock(&trace_lock)
-#define unlock_trace_lock() g_mutex_unlock(&trace_lock)
-#define get_trace_lock_mutex() (&trace_lock)
-#else
-static GStaticMutex trace_lock = G_STATIC_MUTEX_INIT;
-#define lock_trace_lock() g_static_mutex_lock(&trace_lock)
-#define unlock_trace_lock() g_static_mutex_unlock(&trace_lock)
-#define get_trace_lock_mutex() g_static_mutex_get_mutex(&trace_lock)
-#endif
-
-/* g_cond_new() was deprecated in glib 2.31 but we still need to support it */
-#if GLIB_CHECK_VERSION(2, 31, 0)
-static GCond the_trace_available_cond;
-static GCond the_trace_empty_cond;
-static GCond *trace_available_cond = &the_trace_available_cond;
-static GCond *trace_empty_cond = &the_trace_empty_cond;
-#else
-static GCond *trace_available_cond;
-static GCond *trace_empty_cond;
-#endif
+static CompatGMutex trace_lock;
+static CompatGCond trace_available_cond;
+static CompatGCond trace_empty_cond;
static bool trace_available;
static bool trace_writeout_enabled;
@@ -151,26 +132,26 @@ static bool get_trace_record(unsigned int idx, TraceRecord **recordptr)
*/
static void flush_trace_file(bool wait)
{
- lock_trace_lock();
+ g_mutex_lock(&trace_lock);
trace_available = true;
- g_cond_signal(trace_available_cond);
+ g_cond_signal(&trace_available_cond);
if (wait) {
- g_cond_wait(trace_empty_cond, get_trace_lock_mutex());
+ g_cond_wait(&trace_empty_cond, &trace_lock);
}
- unlock_trace_lock();
+ g_mutex_unlock(&trace_lock);
}
static void wait_for_trace_records_available(void)
{
- lock_trace_lock();
+ g_mutex_lock(&trace_lock);
while (!(trace_available && trace_writeout_enabled)) {
- g_cond_signal(trace_empty_cond);
- g_cond_wait(trace_available_cond, get_trace_lock_mutex());
+ g_cond_signal(&trace_empty_cond);
+ g_cond_wait(&trace_available_cond, &trace_lock);
}
trace_available = false;
- unlock_trace_lock();
+ g_mutex_unlock(&trace_lock);
}
static gpointer writeout_thread(gpointer opaque)
@@ -383,11 +364,7 @@ static GThread *trace_thread_create(GThreadFunc fn)
pthread_sigmask(SIG_SETMASK, &set, &oldset);
#endif
-#if GLIB_CHECK_VERSION(2, 31, 0)
thread = g_thread_new("trace-thread", fn, NULL);
-#else
- thread = g_thread_create(fn, NULL, FALSE, NULL);
-#endif
#ifndef _WIN32
pthread_sigmask(SIG_SETMASK, &oldset, NULL);
@@ -402,11 +379,6 @@ bool st_init(const char *file)
trace_pid = getpid();
-#if !GLIB_CHECK_VERSION(2, 31, 0)
- trace_available_cond = g_cond_new();
- trace_empty_cond = g_cond_new();
-#endif
-
thread = trace_thread_create(writeout_thread);
if (!thread) {
fprintf(stderr, "warning: unable to initialize simple trace backend\n");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 2/8] vscclient: use glib thread primitives not qemu
2014-06-10 5:56 [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Paolo Bonzini
2014-06-10 5:56 ` [Qemu-devel] [PULL 1/8] glib-compat.h: add new thread API emulation on top of pre-2.31 API Paolo Bonzini
@ 2014-06-10 5:56 ` Paolo Bonzini
2014-06-17 17:10 ` Ed Maste
2014-06-10 5:56 ` [Qemu-devel] [PULL 3/8] libcacard: replace qemu thread primitives with glib ones Paolo Bonzini
` (6 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-10 5:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tokarev
From: Michael Tokarev <mjt@tls.msk.ru>
Use glib-provided thread primitives in vscclient instead of
qemu ones, and do not use qemu sockets in there (open-code
call to WSAStartup() for windows to initialize things).
This way, vscclient becomes more stand-alone, independent on
qemu internals.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Alon Levy <alevy@redhat.com>
Tested-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
libcacard/vscclient.c | 70 +++++++++++++++++++++++++++------------------------
1 file changed, 37 insertions(+), 33 deletions(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 6693900..ab9b2b8 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -12,12 +12,10 @@
#ifndef _WIN32
#include <netdb.h>
+#define closesocket(x) close(x)
#endif
-#include <glib.h>
#include "qemu-common.h"
-#include "qemu/thread.h"
-#include "qemu/sockets.h"
#include "vscard_common.h"
@@ -54,7 +52,7 @@ print_usage(void) {
static GIOChannel *channel_socket;
static GByteArray *socket_to_send;
-static QemuMutex socket_to_send_lock;
+static CompatGMutex socket_to_send_lock;
static guint socket_tag;
static void
@@ -103,7 +101,7 @@ send_msg(
) {
VSCMsgHeader mhHeader;
- qemu_mutex_lock(&socket_to_send_lock);
+ g_mutex_lock(&socket_to_send_lock);
if (verbose > 10) {
printf("sending type=%d id=%u, len =%u (0x%x)\n",
@@ -117,18 +115,18 @@ send_msg(
g_byte_array_append(socket_to_send, (guint8 *)msg, length);
g_idle_add(socket_prepare_sending, NULL);
- qemu_mutex_unlock(&socket_to_send_lock);
+ g_mutex_unlock(&socket_to_send_lock);
return 0;
}
static VReader *pending_reader;
-static QemuMutex pending_reader_lock;
-static QemuCond pending_reader_condition;
+static CompatGMutex pending_reader_lock;
+static CompatGCond pending_reader_condition;
#define MAX_ATR_LEN 40
-static void *
-event_thread(void *arg)
+static gpointer
+event_thread(gpointer arg)
{
unsigned char atr[MAX_ATR_LEN];
int atr_len;
@@ -149,20 +147,20 @@ event_thread(void *arg)
/* ignore events from readers qemu has rejected */
/* if qemu is still deciding on this reader, wait to see if need to
* forward this event */
- qemu_mutex_lock(&pending_reader_lock);
+ g_mutex_lock(&pending_reader_lock);
if (!pending_reader || (pending_reader != event->reader)) {
/* wasn't for a pending reader, this reader has already been
* rejected by qemu */
- qemu_mutex_unlock(&pending_reader_lock);
+ g_mutex_unlock(&pending_reader_lock);
vevent_delete(event);
continue;
}
/* this reader hasn't been told its status from qemu yet, wait for
* that status */
while (pending_reader != NULL) {
- qemu_cond_wait(&pending_reader_condition, &pending_reader_lock);
+ g_cond_wait(&pending_reader_condition, &pending_reader_lock);
}
- qemu_mutex_unlock(&pending_reader_lock);
+ g_mutex_unlock(&pending_reader_lock);
/* now recheck the id */
reader_id = vreader_get_id(event->reader);
if (reader_id == VSCARD_UNDEFINED_READER_ID) {
@@ -178,12 +176,12 @@ event_thread(void *arg)
/* wait until qemu has responded to our first reader insert
* before we send a second. That way we won't confuse the responses
* */
- qemu_mutex_lock(&pending_reader_lock);
+ g_mutex_lock(&pending_reader_lock);
while (pending_reader != NULL) {
- qemu_cond_wait(&pending_reader_condition, &pending_reader_lock);
+ g_cond_wait(&pending_reader_condition, &pending_reader_lock);
}
pending_reader = vreader_reference(event->reader);
- qemu_mutex_unlock(&pending_reader_lock);
+ g_mutex_unlock(&pending_reader_lock);
reader_name = vreader_get_name(event->reader);
if (verbose > 10) {
printf(" READER INSERT: %s\n", reader_name);
@@ -246,7 +244,6 @@ on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
int num_capabilities =
1 + ((mhHeader->length - sizeof(VSCMsgInit)) / sizeof(uint32_t));
int i;
- QemuThread thread_id;
incoming->version = ntohl(incoming->version);
if (incoming->version != VSCARD_VERSION) {
@@ -269,7 +266,7 @@ on_host_init(VSCMsgHeader *mhHeader, VSCMsgInit *incoming)
send_msg(VSC_ReaderRemove, VSCARD_MINIMAL_READER_ID, NULL, 0);
/* launch the event_thread. This will trigger reader adds for all the
* existing readers */
- qemu_thread_create(&thread_id, "vsc/event", event_thread, NULL, 0);
+ g_thread_new("vsc/event", event_thread, NULL);
return 0;
}
@@ -379,26 +376,26 @@ do_socket_read(GIOChannel *source,
case VSC_Error:
error_msg = (VSCMsgError *) pbSendBuffer;
if (error_msg->code == VSC_SUCCESS) {
- qemu_mutex_lock(&pending_reader_lock);
+ g_mutex_lock(&pending_reader_lock);
if (pending_reader) {
vreader_set_id(pending_reader, mhHeader.reader_id);
vreader_free(pending_reader);
pending_reader = NULL;
- qemu_cond_signal(&pending_reader_condition);
+ g_cond_signal(&pending_reader_condition);
}
- qemu_mutex_unlock(&pending_reader_lock);
+ g_mutex_unlock(&pending_reader_lock);
break;
}
printf("warning: qemu refused to add reader\n");
if (error_msg->code == VSC_CANNOT_ADD_MORE_READERS) {
/* clear pending reader, qemu can't handle any more */
- qemu_mutex_lock(&pending_reader_lock);
+ g_mutex_lock(&pending_reader_lock);
if (pending_reader) {
pending_reader = NULL;
/* make sure the event loop doesn't hang */
- qemu_cond_signal(&pending_reader_condition);
+ g_cond_signal(&pending_reader_condition);
}
- qemu_mutex_unlock(&pending_reader_lock);
+ g_mutex_unlock(&pending_reader_lock);
}
break;
case VSC_Init:
@@ -601,7 +598,7 @@ connect_to_qemu(
struct addrinfo *server;
int ret, sock;
- sock = qemu_socket(AF_INET, SOCK_STREAM, 0);
+ sock = socket(AF_INET, SOCK_STREAM, 0);
if (sock < 0) {
/* Error */
fprintf(stderr, "Error opening socket!\n");
@@ -654,8 +651,20 @@ main(
int cert_count = 0;
int c, sock;
- if (socket_init() != 0)
+#ifdef _WIN32
+ WSADATA Data;
+
+ if (WSAStartup(MAKEWORD(2, 2), &Data) != 0) {
+ c = WSAGetLastError();
+ fprintf(stderr, "WSAStartup: %d\n", c);
return 1;
+ }
+#endif
+#if !GLIB_CHECK_VERSION(2, 31, 0)
+ if (!g_thread_supported()) {
+ g_thread_init(NULL);
+ }
+#endif
while ((c = getopt(argc, argv, "c:e:pd:")) != -1) {
switch (c) {
@@ -722,13 +731,8 @@ main(
}
socket_to_send = g_byte_array_new();
- qemu_mutex_init(&socket_to_send_lock);
- qemu_mutex_init(&pending_reader_lock);
- qemu_cond_init(&pending_reader_condition);
-
vcard_emul_init(command_line_options);
-
- loop = g_main_loop_new(NULL, true);
+ loop = g_main_loop_new(NULL, TRUE);
printf("> ");
fflush(stdout);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PULL 2/8] vscclient: use glib thread primitives not qemu
2014-06-10 5:56 ` [Qemu-devel] [PULL 2/8] vscclient: use glib thread primitives not qemu Paolo Bonzini
@ 2014-06-17 17:10 ` Ed Maste
2014-06-17 17:15 ` Paolo Bonzini
0 siblings, 1 reply; 15+ messages in thread
From: Ed Maste @ 2014-06-17 17:10 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Michael Tokarev, qemu-devel
On 10 June 2014 01:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Michael Tokarev <mjt@tls.msk.ru>
>
> Use glib-provided thread primitives in vscclient instead of
> qemu ones, and do not use qemu sockets in there (open-code
> call to WSAStartup() for windows to initialize things).
>
> This way, vscclient becomes more stand-alone, independent on
> qemu internals.
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> Reviewed-by: Alon Levy <alevy@redhat.com>
> Tested-by: Alon Levy <alevy@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> libcacard/vscclient.c | 70 +++++++++++++++++++++++++++------------------------
> 1 file changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
> index 6693900..ab9b2b8 100644
> --- a/libcacard/vscclient.c
> +++ b/libcacard/vscclient.c
> @@ -12,12 +12,10 @@
>
> #ifndef _WIN32
> #include <netdb.h>
> +#define closesocket(x) close(x)
> #endif
> -#include <glib.h>
>
> #include "qemu-common.h"
> -#include "qemu/thread.h"
> -#include "qemu/sockets.h"
This change breaks the build on FreeBSD:
libcacard/vscclient.c: In function 'send_msg':
libcacard/vscclient.c:111: warning: implicit declaration of function 'htonl'
libcacard/vscclient.c:111: warning: nested extern declaration of 'htonl'
libcacard/vscclient.c: In function 'on_host_init':
libcacard/vscclient.c:248: warning: implicit declaration of function 'ntohl'
libcacard/vscclient.c:248: warning: nested extern declaration of 'ntohl'
libcacard/vscclient.c: In function 'connect_to_qemu':
libcacard/vscclient.c:601: warning: implicit declaration of function 'socket'
libcacard/vscclient.c:601: warning: nested extern declaration of 'socket'
...
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PULL 2/8] vscclient: use glib thread primitives not qemu
2014-06-17 17:10 ` Ed Maste
@ 2014-06-17 17:15 ` Paolo Bonzini
2014-06-17 19:11 ` Ed Maste
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-17 17:15 UTC (permalink / raw)
To: Ed Maste; +Cc: Michael Tokarev, qemu-devel
Il 17/06/2014 19:10, Ed Maste ha scritto:
> This change breaks the build on FreeBSD:
>
> libcacard/vscclient.c: In function 'send_msg':
> libcacard/vscclient.c:111: warning: implicit declaration of function 'htonl'
> libcacard/vscclient.c:111: warning: nested extern declaration of 'htonl'
> libcacard/vscclient.c: In function 'on_host_init':
> libcacard/vscclient.c:248: warning: implicit declaration of function 'ntohl'
> libcacard/vscclient.c:248: warning: nested extern declaration of 'ntohl'
> libcacard/vscclient.c: In function 'connect_to_qemu':
> libcacard/vscclient.c:601: warning: implicit declaration of function 'socket'
> libcacard/vscclient.c:601: warning: nested extern declaration of 'socket'
Can you prepare a patch yourself? I would be guessing the right header
to include, sorry.
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PULL 2/8] vscclient: use glib thread primitives not qemu
2014-06-17 17:15 ` Paolo Bonzini
@ 2014-06-17 19:11 ` Ed Maste
2014-06-18 9:25 ` Paolo Bonzini
0 siblings, 1 reply; 15+ messages in thread
From: Ed Maste @ 2014-06-17 19:11 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Michael Tokarev, qemu-devel
On 17 June 2014 13:15, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 17/06/2014 19:10, Ed Maste ha scritto:
>
>> This change breaks the build on FreeBSD:
>>
>> libcacard/vscclient.c: In function 'send_msg':
>> libcacard/vscclient.c:111: warning: implicit declaration of function
>> 'htonl'
>> ...
>
> Can you prepare a patch yourself? I would be guessing the right header to
> include, sorry.
I can get it to build by either restoring the #include of
qemu/sockets.h, or explicitly #including sys/socket.h and
netinet/in.h. I'm happy to send a signed-off patch for either change,
but it seems there must be more to the original change that I'm
missing. What provides the declarations for socket(), htonl(),
AF_INET etc. in the Linux build?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PULL 2/8] vscclient: use glib thread primitives not qemu
2014-06-17 19:11 ` Ed Maste
@ 2014-06-18 9:25 ` Paolo Bonzini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-18 9:25 UTC (permalink / raw)
To: Ed Maste; +Cc: Michael Tokarev, qemu-devel
Il 17/06/2014 21:11, Ed Maste ha scritto:
> On 17 June 2014 13:15, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 17/06/2014 19:10, Ed Maste ha scritto:
>>
>>> This change breaks the build on FreeBSD:
>>>
>>> libcacard/vscclient.c: In function 'send_msg':
>>> libcacard/vscclient.c:111: warning: implicit declaration of function
>>> 'htonl'
>>> ...
>>
>> Can you prepare a patch yourself? I would be guessing the right header to
>> include, sorry.
>
> I can get it to build by either restoring the #include of
> qemu/sockets.h, or explicitly #including sys/socket.h and
> netinet/in.h.
The latter is better, this patch meant to remove most (though not yet
all) dependencies of libcacard on include/qemu/.
I'm happy to send a signed-off patch for either change,
> but it seems there must be more to the original change that I'm
> missing. What provides the declarations for socket(), htonl(),
> AF_INET etc. in the Linux build?
netdb.h includes netinet/in.h, and netinet/in.h includes sys/socket.h.
glibc is not particularly good at avoiding #includes within headers. :(
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 3/8] libcacard: replace qemu thread primitives with glib ones
2014-06-10 5:56 [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Paolo Bonzini
2014-06-10 5:56 ` [Qemu-devel] [PULL 1/8] glib-compat.h: add new thread API emulation on top of pre-2.31 API Paolo Bonzini
2014-06-10 5:56 ` [Qemu-devel] [PULL 2/8] vscclient: use glib thread primitives not qemu Paolo Bonzini
@ 2014-06-10 5:56 ` Paolo Bonzini
2014-06-10 5:56 ` [Qemu-devel] [PULL 4/8] libcacard: actually use symbols file Paolo Bonzini
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-10 5:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tokarev
From: Michael Tokarev <mjt@tls.msk.ru>
Replace QemuMutex with GMutex and QemuCond with GCond
(with corresponding function changes), to make libcacard
independent of qemu internal functions.
After this step, none of libcacard internals use any
qemu-provided symbols. Maybe it's a good idea to
stop including qemu-common.h internally too.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Alon Levy <alevy@redhat.com>
Tested-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
libcacard/Makefile | 8 +-------
libcacard/event.c | 23 ++++++++++-------------
libcacard/vreader.c | 18 ++++++++----------
3 files changed, 19 insertions(+), 30 deletions(-)
diff --git a/libcacard/Makefile b/libcacard/Makefile
index 881b222..92d3b47 100644
--- a/libcacard/Makefile
+++ b/libcacard/Makefile
@@ -3,13 +3,7 @@ libcacard_includedir=$(includedir)/cacard
TOOLS += vscclient$(EXESUF)
# objects linked into a shared library, built with libtool with -fPIC if required
-libcacard-obj-y = $(stub-obj-y) $(libcacard-y)
-libcacard-obj-y += util/osdep.o util/cutils.o util/qemu-timer-common.o
-libcacard-obj-y += util/error.o util/qemu-error.o
-libcacard-obj-$(CONFIG_WIN32) += util/oslib-win32.o util/qemu-thread-win32.o
-libcacard-obj-$(CONFIG_POSIX) += util/oslib-posix.o util/qemu-thread-posix.o
-libcacard-obj-y += $(filter trace/%, $(util-obj-y))
-
+libcacard-obj-y = $(libcacard-y)
libcacard-lobj-y=$(patsubst %.o,%.lo,$(libcacard-obj-y))
# libtool will build the .o files, too
diff --git a/libcacard/event.c b/libcacard/event.c
index a2e6c7d..4c551e4 100644
--- a/libcacard/event.c
+++ b/libcacard/event.c
@@ -6,7 +6,6 @@
*/
#include "qemu-common.h"
-#include "qemu/thread.h"
#include "vcard.h"
#include "vreader.h"
@@ -43,13 +42,11 @@ vevent_delete(VEvent *vevent)
static VEvent *vevent_queue_head;
static VEvent *vevent_queue_tail;
-static QemuMutex vevent_queue_lock;
-static QemuCond vevent_queue_condition;
+static CompatGMutex vevent_queue_lock;
+static CompatGCond vevent_queue_condition;
void vevent_queue_init(void)
{
- qemu_mutex_init(&vevent_queue_lock);
- qemu_cond_init(&vevent_queue_condition);
vevent_queue_head = vevent_queue_tail = NULL;
}
@@ -57,7 +54,7 @@ void
vevent_queue_vevent(VEvent *vevent)
{
vevent->next = NULL;
- qemu_mutex_lock(&vevent_queue_lock);
+ g_mutex_lock(&vevent_queue_lock);
if (vevent_queue_head) {
assert(vevent_queue_tail);
vevent_queue_tail->next = vevent;
@@ -65,8 +62,8 @@ vevent_queue_vevent(VEvent *vevent)
vevent_queue_head = vevent;
}
vevent_queue_tail = vevent;
- qemu_cond_signal(&vevent_queue_condition);
- qemu_mutex_unlock(&vevent_queue_lock);
+ g_cond_signal(&vevent_queue_condition);
+ g_mutex_unlock(&vevent_queue_lock);
}
/* must have lock */
@@ -86,11 +83,11 @@ VEvent *vevent_wait_next_vevent(void)
{
VEvent *vevent;
- qemu_mutex_lock(&vevent_queue_lock);
+ g_mutex_lock(&vevent_queue_lock);
while ((vevent = vevent_dequeue_vevent()) == NULL) {
- qemu_cond_wait(&vevent_queue_condition, &vevent_queue_lock);
+ g_cond_wait(&vevent_queue_condition, &vevent_queue_lock);
}
- qemu_mutex_unlock(&vevent_queue_lock);
+ g_mutex_unlock(&vevent_queue_lock);
return vevent;
}
@@ -98,9 +95,9 @@ VEvent *vevent_get_next_vevent(void)
{
VEvent *vevent;
- qemu_mutex_lock(&vevent_queue_lock);
+ g_mutex_lock(&vevent_queue_lock);
vevent = vevent_dequeue_vevent();
- qemu_mutex_unlock(&vevent_queue_lock);
+ g_mutex_unlock(&vevent_queue_lock);
return vevent;
}
diff --git a/libcacard/vreader.c b/libcacard/vreader.c
index d2a9b7d..f0c57e6 100644
--- a/libcacard/vreader.c
+++ b/libcacard/vreader.c
@@ -9,10 +9,8 @@
#undef G_LOG_DOMAIN
#endif
#define G_LOG_DOMAIN "libcacard"
-#include <glib.h>
#include "qemu-common.h"
-#include "qemu/thread.h"
#include "vcard.h"
#include "vcard_emul.h"
@@ -28,7 +26,7 @@ struct VReaderStruct {
VCard *card;
char *name;
vreader_id_t id;
- QemuMutex lock;
+ CompatGMutex lock;
VReaderEmul *reader_private;
VReaderEmulFree reader_private_free;
};
@@ -97,13 +95,13 @@ apdu_ins_to_string(int ins)
static inline void
vreader_lock(VReader *reader)
{
- qemu_mutex_lock(&reader->lock);
+ g_mutex_lock(&reader->lock);
}
static inline void
vreader_unlock(VReader *reader)
{
- qemu_mutex_unlock(&reader->lock);
+ g_mutex_unlock(&reader->lock);
}
/*
@@ -116,7 +114,7 @@ vreader_new(const char *name, VReaderEmul *private,
VReader *reader;
reader = g_new(VReader, 1);
- qemu_mutex_init(&reader->lock);
+ g_mutex_init(&reader->lock);
reader->reference_count = 1;
reader->name = g_strdup(name);
reader->card = NULL;
@@ -152,6 +150,7 @@ vreader_free(VReader *reader)
return;
}
vreader_unlock(reader);
+ g_mutex_clear(&reader->lock);
if (reader->card) {
vcard_free(reader->card);
}
@@ -406,25 +405,24 @@ vreader_dequeue(VReaderList *list, VReaderListEntry *entry)
}
static VReaderList *vreader_list;
-static QemuMutex vreader_list_mutex;
+static CompatGMutex vreader_list_mutex;
static void
vreader_list_init(void)
{
vreader_list = vreader_list_new();
- qemu_mutex_init(&vreader_list_mutex);
}
static void
vreader_list_lock(void)
{
- qemu_mutex_lock(&vreader_list_mutex);
+ g_mutex_lock(&vreader_list_mutex);
}
static void
vreader_list_unlock(void)
{
- qemu_mutex_unlock(&vreader_list_mutex);
+ g_mutex_unlock(&vreader_list_mutex);
}
static VReaderList *
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 4/8] libcacard: actually use symbols file
2014-06-10 5:56 [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Paolo Bonzini
` (2 preceding siblings ...)
2014-06-10 5:56 ` [Qemu-devel] [PULL 3/8] libcacard: replace qemu thread primitives with glib ones Paolo Bonzini
@ 2014-06-10 5:56 ` Paolo Bonzini
2014-06-10 5:56 ` [Qemu-devel] [PULL 5/8] libcacard: improve documentation Paolo Bonzini
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-10 5:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tokarev
From: Michael Tokarev <mjt@tls.msk.ru>
libtool has an argument for .syms file, which is -export-symbols.
There's no argument `-export-syms', and it looks like at least on
linux, -export-syms is just ignored. Use the correct argument,
-export-symbols, to actually get the right export list.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Alon Levy <alevy@redhat.com>
Tested-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
libcacard/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcacard/Makefile b/libcacard/Makefile
index 92d3b47..0e7903f 100644
--- a/libcacard/Makefile
+++ b/libcacard/Makefile
@@ -18,7 +18,7 @@ vscclient$(EXESUF): libcacard/vscclient.o libcacard.la
# Rules for building libcacard standalone library
libcacard.la: LDFLAGS += -rpath $(libdir) -no-undefined \
- -export-syms $(SRC_PATH)/libcacard/libcacard.syms
+ -export-symbols $(SRC_PATH)/libcacard/libcacard.syms
libcacard.la: $(libcacard-lobj-y)
$(call LINK,$^)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 5/8] libcacard: improve documentation
2014-06-10 5:56 [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Paolo Bonzini
` (3 preceding siblings ...)
2014-06-10 5:56 ` [Qemu-devel] [PULL 4/8] libcacard: actually use symbols file Paolo Bonzini
@ 2014-06-10 5:56 ` Paolo Bonzini
2014-06-10 5:56 ` [Qemu-devel] [PULL 6/8] rules.mak: Rewrite unnest-vars Paolo Bonzini
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-10 5:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Ray Strode
Using the file-backed smartcard backend is black magic, but it can
be useful if your only smartcard bricks itself if it is accessed
the wrong way too many times.
Complete the documentation to include the art of creating certificates
and using them with QEMU, based on Ray Strode's useful tutorial at
https://blogs.gnome.org/halfline/2013/09/08/another-smartcard-post/
but with ccid-card-emulated or vscclient instead of SPICE.
Cc: Ray Strode <rstrode@redhat.com>
Reviewed-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/ccid.txt | 80 ++++++++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 63 insertions(+), 17 deletions(-)
diff --git a/docs/ccid.txt b/docs/ccid.txt
index 83c174d..c7fda6d 100644
--- a/docs/ccid.txt
+++ b/docs/ccid.txt
@@ -47,6 +47,7 @@ In ubuntu/debian:
Configuring and building:
./configure --enable-smartcard && make
+
3. Using ccid-card-emulated with hardware
Assuming you have a working smartcard on the host with the current
@@ -54,19 +55,55 @@ user, using NSS, qemu acts as another NSS client using ccid-card-emulated:
qemu -usb -device usb-ccid -device ccid-card-emulated
-4. Using ccid-card-emulated with certificates
-You must create the certificates. This is a one time process. We use NSS
-certificates:
+4. Using ccid-card-emulated with certificates stored in files
+
+You must create the CA and card certificates. This is a one time process.
+We use NSS certificates:
- certutil -d /etc/pki/nssdb -x -t "CT,CT,CT" -S -s "CN=cert1" -n cert1
+ mkdir fake-smartcard
+ cd fake-smartcard
+ certutil -N -d sql:$PWD
+ certutil -S -d sql:$PWD -s "CN=Fake Smart Card CA" -x -t TC,TC,TC -n fake-smartcard-ca
+ certutil -S -d sql:$PWD -t ,, -s "CN=John Doe" -n id-cert -c fake-smartcard-ca
+ certutil -S -d sql:$PWD -t ,, -s "CN=John Doe (signing)" --nsCertType smime -n signing-cert -c fake-smartcard-ca
+ certutil -S -d sql:$PWD -t ,, -s "CN=John Doe (encryption)" --nsCertType sslClient -n encryption-cert -c fake-smartcard-ca
Note: you must have exactly three certificates.
-Assuming the current user can access the certificates (use certutil -L to
-verify), you can use the emulated card type with the certificates backend:
+You can use the emulated card type with the certificates backend:
+
+ qemu -usb -device usb-ccid -device ccid-card-emulated,backend=certificates,db=sql:$PWD,cert1=id-cert,cert2=signing-cert,cert3=encryption-cert
+
+To use the certificates in the guest, export the CA certificate:
+
+ certutil -L -r -d sql:$PWD -o fake-smartcard-ca.cer -n fake-smartcard-ca
+
+and import it in the guest:
+
+ certutil -A -d /etc/pki/nssdb -i fake-smartcard-ca.cer -t TC,TC,TC -n fake-smartcard-ca
+
+In a Linux guest you can then use the CoolKey PKCS #11 module to access
+the card:
+
+ certutil -d /etc/pki/nssdb -L -h all
+
+It will prompt you for the PIN (which is the password you assigned to the
+certificate database early on), and then show you all three certificates
+together with the manually imported CA cert:
+
+ Certificate Nickname Trust Attributes
+ fake-smartcard-ca CT,C,C
+ John Doe:CAC ID Certificate u,u,u
+ John Doe:CAC Email Signature Certificate u,u,u
+ John Doe:CAC Email Encryption Certificate u,u,u
+
+If this does not happen, CoolKey is not installed or not registered with
+NSS. Registration can be done from Firefox or the command line:
+
+ modutil -dbdir /etc/pki/nssdb -add "CAC Module" -libfile /usr/lib64/pkcs11/libcoolkeypk11.so
+ modutil -dbdir /etc/pki/nssdb -list
- qemu -usb -device usb-ccid -device ccid-card-emulated,backend=certificates,cert1=cert1,cert2=cert2,cert3=cert3
5. Using ccid-card-passthru with client side hardware
@@ -74,15 +111,23 @@ on the host specify the ccid-card-passthru device with a suitable chardev:
qemu -chardev socket,server,host=0.0.0.0,port=2001,id=ccid,nowait -usb -device usb-ccid -device ccid-card-passthru,chardev=ccid
-on the client run vscclient, built when you built the libcacard library:
- libcacard/vscclient <qemu-host> 2001
+on the client run vscclient, built when you built QEMU:
+
+ vscclient <qemu-host> 2001
+
6. Using ccid-card-passthru with client side certificates
-Run qemu as per #5, and run vscclient as follows:
-(Note: vscclient command line interface is in a state of change)
+This case is not particularly useful, but you can use it to debug
+your setup if #4 works but #5 does not.
+
+Follow instructions as per #4, except run QEMU and vscclient as follows:
+Run qemu as per #5, and run vscclient from the "fake-smartcard"
+directory as follows:
+
+ qemu -chardev socket,server,host=0.0.0.0,port=2001,id=ccid,nowait -usb -device usb-ccid -device ccid-card-passthru,chardev=ccid
+ vscclient -e "db=\"sql:$PWD\" use_hw=no soft=(,Test,CAC,,id-cert,signing-cert,encryption-cert)" <qemu-host> 2001
- libcacard/vscclient -e "db=\"/etc/pki/nssdb\" use_hw=no soft=(,Test,CAC,,cert1,cert2,cert3)" <qemu-host> 2001
7. Passthrough protocol scenario
@@ -126,10 +171,11 @@ kill/quit | | | |
8. libcacard
-ccid-card-passthru and vscclient use libcacard as the card emulator.
-libcacard implements a completely virtual CAC (DoD standard for smart cards)
-compliant card and uses NSS to actually retrive certificates and do any
-encryption using the backend (real reader + card or file backed certificates).
+Both ccid-card-emulated and vscclient use libcacard as the card emulator.
+libcacard implements a completely virtual CAC (DoD standard for smart
+cards) compliant card and uses NSS to retrieve certificates and do
+any encryption. The backend can then be a real reader and card, or
+certificates stored in files.
-For documentation of cac_card see README in libcacard subdirectory.
+For documentation of the library see docs/libcacard.txt.
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 6/8] rules.mak: Rewrite unnest-vars
2014-06-10 5:56 [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Paolo Bonzini
` (4 preceding siblings ...)
2014-06-10 5:56 ` [Qemu-devel] [PULL 5/8] libcacard: improve documentation Paolo Bonzini
@ 2014-06-10 5:56 ` Paolo Bonzini
2014-06-10 10:40 ` Paolo Bonzini
2014-06-10 5:56 ` [Qemu-devel] [PULL 7/8] configure: duplicate/incorrect order of -lrt Paolo Bonzini
` (2 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-10 5:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng
From: Fam Zheng <famz@redhat.com>
The macro unnest-vars is the most important, complicated but hard to
track magic in QEMU's build system.
Rewrite it in a (hopefully) clearer way, with more comments, to make it
easier to understand and maintain.
Remove DSO_CFLAGS and module-objs-m that are not used.
A bonus fix of this version is, per object variables are properly
protected in save-objs and load-objs, before including sub-dir
Makefile.objs, just as nested variables are. So the occasional same
object name from different directory levels won't step on each other's
foot.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rules.mak | 227 ++++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 156 insertions(+), 71 deletions(-)
diff --git a/rules.mak b/rules.mak
index b12d312..4eea5bb 100644
--- a/rules.mak
+++ b/rules.mak
@@ -78,13 +78,16 @@ endif
%.o: %.dtrace
$(call quiet-command,dtrace -o $@ -G -s $<, " GEN $(TARGET_DIR)$@")
-DSO_CFLAGS := -fPIC -DBUILD_DSO
+%$(DSOSUF): CFLAGS += -fPIC -DBUILD_DSO
%$(DSOSUF): LDFLAGS += $(LDFLAGS_SHARED)
-%$(DSOSUF): %.mo libqemustub.a
+%$(DSOSUF): libqemustub.a module-common.o
$(call LINK,$^)
@# Copy to build root so modules can be loaded when program started without install
$(if $(findstring /,$@),$(call quiet-command,cp $@ $(subst /,-,$@), " CP $(subst /,-,$@)"))
+%.mo:
+ $(call quiet-command,touch $@," GEN $(TARGET_DIR)$@")
+
.PHONY: modules
modules:
@@ -161,82 +164,164 @@ clean: clean-timestamp
# will delete the target of a rule if commands exit with a nonzero exit status
.DELETE_ON_ERROR:
-# magic to descend into other directories
-
-define push-var
-$(eval save-$2-$1 = $(value $1))
-$(eval $1 :=)
-endef
-
-define pop-var
-$(eval subdir-$2-$1 := $(if $(filter $2,$(save-$2-$1)),$(addprefix $2,$($1))))
-$(eval $1 = $(value save-$2-$1) $$(subdir-$2-$1))
-$(eval save-$2-$1 :=)
-endef
-
-define fix-obj-vars
-$(if $2, $(foreach v,$($1), \
- $(if $($v-cflags), \
- $(eval $2/$v-cflags := $($v-cflags)) \
- $(eval $v-cflags := )) \
- $(if $($v-libs), \
- $(eval $2/$v-libs := $($v-libs)) \
- $(eval $v-libs := )) \
- $(if $($v-objs), \
- $(eval $2/$v-objs := $(addprefix $2/,$($v-objs))) \
- $(eval $v-objs := ))))
+# save-vars
+# Usage: $(call save-vars, vars)
+# Save each variable $v in $vars as save-vars-$v, save their object's
+# variables, then clear $v.
+define save-vars
+ $(foreach v,$1,
+ $(eval save-vars-$v := $(value $v))
+ $(foreach o,$($v),
+ $(foreach k,cflags libs objs,
+ $(if $($o-$k),
+ $(eval save-vars-$o-$k := $($o-$k))
+ $(eval $o-$k := ))))
+ $(eval $v := ))
endef
-define unnest-dir
-$(foreach var,$(nested-vars),$(call push-var,$(var),$1/))
-$(eval obj-parent-$1 := $(obj))
-$(eval obj := $(if $(obj),$(obj)/$1,$1))
-$(eval include $(SRC_PATH)/$1/Makefile.objs)
-$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(obj)))
-$(eval obj := $(obj-parent-$1))
-$(eval obj-parent-$1 := )
-$(foreach var,$(nested-vars),$(call pop-var,$(var),$1/))
+# load-vars
+# Usage: $(call load-vars, vars, add_var)
+# Load the saved value for each variable in @vars, and the per object
+# variables.
+# Append @add_var's current value to the loaded value.
+define load-vars
+ $(eval $2-new-value := $(value $2))
+ $(foreach v,$1,
+ $(eval $v := $(value save-vars-$v))
+ $(foreach o,$($v),
+ $(foreach k,cflags libs objs,
+ $(if $(save-vars-$o-$k),
+ $(eval $o-$k := $(save-vars-$o-$k))
+ $(eval save-vars-$o-$k := ))))
+ $(eval save-vars-$v := ))
+ $(eval $2 := $(value $2) $($2-new-value))
endef
-define unnest-vars-1
-$(eval nested-dirs := $(filter-out \
- $(old-nested-dirs), \
- $(sort $(foreach var,$(nested-vars), $(filter %/, $($(var)))))))
-$(if $(nested-dirs),
- $(foreach dir,$(nested-dirs),$(call unnest-dir,$(patsubst %/,%,$(dir))))
- $(eval old-nested-dirs := $(old-nested-dirs) $(nested-dirs))
- $(call unnest-vars-1))
+# fix-paths
+# Usage: $(call fix-paths, obj_path, src_path, vars)
+# Add prefix @obj_path to all objects in @vars, and add prefix @src_path to all
+# directories in @vars.
+define fix-paths
+ $(foreach v,$3,
+ $(foreach o,$($v),
+ $(if $($o-libs),
+ $(eval $1$o-libs := $($o-libs)))
+ $(if $($o-cflags),
+ $(eval $1$o-cflags := $($o-cflags)))
+ $(if $($o-objs),
+ $(eval $1$o-objs := $(addprefix $1,$($o-objs)))))
+ $(eval $v := $(addprefix $1,$(filter-out %/,$($v))) \
+ $(addprefix $2,$(filter %/,$($v)))))
endef
-define process-modules
-$(foreach o,$(filter %.o,$($1)),
- $(eval $(patsubst %.o,%.mo,$o): $o) \
- $(eval $(patsubst %.o,%.mo,$o)-objs := $o))
-$(foreach o,$(filter-out $(modules-m), $(patsubst %.o,%.mo,$($1))), \
- $(eval $o-objs += module-common.o)
- $(eval $o: $($o-objs))
- $(eval modules-objs-m += $($o-objs))
- $(eval modules-m += $o)
- $(eval $o:; $$(call quiet-command,touch $$@," GEN $$(TARGET_DIR)$$@"))
- $(if $(CONFIG_MODULES),$(eval modules: $(patsubst %.mo,%$(DSOSUF),$o)))) \
-$(eval modules-objs-m := $(sort $(modules-objs-m)))
-$(foreach o,$(modules-objs-m), \
- $(if $(CONFIG_MODULES),$(eval $o-cflags := $(call maybe-add, $(DSO_CFLAGS), $($o-cflags)))))
-$(eval $(patsubst %-m,%-$(call lnot,$(CONFIG_MODULES)),$1) += $($1))
+# unnest-var-recursive
+# Usage: $(call unnest-var-recursive, obj_prefix, vars, var)
+#
+# Unnest @var by including subdir Makefile.objs, while protect others in @vars
+# unchanged.
+#
+# @obj_prefix is the starting point of object path prefix.
+#
+define unnest-var-recursive
+ $(eval dirs := $(sort $(filter %/,$($3))))
+ $(eval $3 := $(filter-out %/,$($3)))
+ $(foreach d,$(dirs:%/=%),
+ $(call save-vars,$2)
+ $(eval obj := $(if $1,$1/)$d)
+ $(eval -include $(SRC_PATH)/$d/Makefile.objs)
+ $(call fix-paths,$(if $1,$1/)$d/,$d/,$2)
+ $(call load-vars,$2,$3)
+ $(call unnest-var-recursive,$1,$2,$3))
endef
+# unnest-vars
+# Usage: $(call unnest-vars, obj_prefix, vars)
+#
+# @obj_prefix: object path prefix, can be empty, or '..', etc. Don't include
+# ending '/'.
+#
+# @vars: the list of variable names to unnest.
+#
+# This macro will scan subdirectories's Makefile.objs, include them, to build
+# up each variable listed in @vars.
+#
+# Per object and per module cflags and libs are saved with relative path fixed
+# as well, those variables include -libs, -cflags and -objs. Items in -objs are
+# also fixed to relative path against SRC_PATH plus the prefix @obj_prefix.
+#
+# All nested variables postfixed by -m in names are treated as DSO variables,
+# and will be built as modules, if enabled.
+#
+# A simple example of the unnest:
+#
+# obj_prefix = ..
+# vars = hot cold
+# hot = fire.o sun.o season/
+# cold = snow.o water/ season/
+#
+# Unnest through a faked source directory structure:
+#
+# SRC_PATH
+# ├── water
+# │ └── Makefile.objs──────────────────┐
+# │ │ hot += steam.o │
+# │ │ cold += ice.mo │
+# │ │ ice.mo-libs := -licemaker │
+# │ │ ice.mo-objs := ice1.o ice2.o │
+# │ └──────────────────────────────┘
+# │
+# └── season
+# └── Makefile.objs──────┐
+# │ hot += summer.o │
+# │ cold += winter.o │
+# └──────────────────┘
+#
+# In the end, the result will be:
+#
+# hot = ../fire.o ../sun.o ../season/summer.o
+# cold = ../snow.o ../water/ice.mo ../season/winter.o
+# ../water/ice.mo-libs = -licemaker
+# ../water/ice.mo-objs = ../water/ice1.o ../water/ice2.o
+#
+# Note that 'hot' didn't include 'season/' in the input, so 'summer.o' is not
+# included.
+#
define unnest-vars
-$(eval obj := $1)
-$(eval nested-vars := $2)
-$(foreach v,$(nested-vars),$(call fix-obj-vars,$v,$(obj)))
-$(eval old-nested-dirs := )
-$(call unnest-vars-1)
-$(if $1,$(foreach v,$(nested-vars),$(eval \
- $v := $(addprefix $1/,$($v)))))
-$(foreach var,$(nested-vars),$(eval $(var) := $(filter-out %/, $($(var)))))
-$(shell mkdir -p $(sort $(foreach var,$(nested-vars),$(dir $($(var))))))
-$(foreach var,$(nested-vars), $(eval \
- -include $(addsuffix *.d, $(sort $(dir $($(var)))))))
-$(foreach v,$(filter %-m,$(nested-vars)), \
- $(call process-modules,$v))
+ # In the case of target build (i.e. $1 == ..), fix path for top level
+ # Makefile.objs objects
+ $(if $1,$(call fix-paths,$1/,,$2))
+
+ # Descend and include every subdir Makefile.objs
+ $(foreach v, $2, $(call unnest-var-recursive,$1,$2,$v))
+
+ $(foreach v,$(filter %-m,$2),
+ # All .o found in *-m variables are single object modules, create .mo
+ # for them
+ $(foreach o,$(filter %.o,$($v)),
+ $(eval $(o:%.o=%.mo)-objs := $o))
+ # Now unify .o in -m variable to .mo
+ $(eval $v := $($v:%.o=%.mo))
+
+ $(eval modules: $($v:%.mo=%$(DSOSUF)))
+ $(eval modules-m += $($v))
+
+ # For non-module build, add -m to -y
+ $(if $(CONFIG_MODULES),,$(eval $(patsubst %-m,%-y,$v) += $($v))))
+
+ # Post-process all the unnested vars
+ $(foreach v,$2,
+ $(foreach o, $(filter %.mo,$($v)),
+ # Find all the .mo objects in variables and add dependency rules
+ # according to .mo-objs. Report error if not set
+ $(if $($o-objs),
+ $(eval $(o:%.mo=%$(DSOSUF)): $($o-objs)),
+ $(error $o added in $v but $o-objs is not set))
+ # Pass the .mo-cflags along to member objects
+ $(if $($o-cflags),
+ $(foreach p,$($o-objs),
+ $(eval $p-cflags := $($o-cflags)))))
+ $(shell mkdir -p ./ $(sort $(dir $($v))))
+ # Include all the .d files
+ $(eval -include $(addsuffix *.d, $(sort $(dir $($v)))))
+ $(eval $v := $(filter-out %/,$($v))))
endef
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PULL 6/8] rules.mak: Rewrite unnest-vars
2014-06-10 5:56 ` [Qemu-devel] [PULL 6/8] rules.mak: Rewrite unnest-vars Paolo Bonzini
@ 2014-06-10 10:40 ` Paolo Bonzini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-10 10:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Fam Zheng
Il 10/06/2014 07:56, Paolo Bonzini ha scritto:
> + $(foreach v,$(filter %-m,$2),
> + # All .o found in *-m variables are single object modules, create .mo
> + # for them
> + $(foreach o,$(filter %.o,$($v)),
> + $(eval $(o:%.o=%.mo)-objs := $o))
> + # Now unify .o in -m variable to .mo
> + $(eval $v := $($v:%.o=%.mo))
> +
> + $(eval modules: $($v:%.mo=%$(DSOSUF)))
> + $(eval modules-m += $($v))
> +
> + # For non-module build, add -m to -y
> + $(if $(CONFIG_MODULES),,$(eval $(patsubst %-m,%-y,$v) += $($v))))
Peter spotted a few problems here. This needs to be
+ $(if $(CONFIG_MODULES),
+ $(eval modules: $($v:%.mo=%$(DSOSUF)))
+ $(eval $(patsubst %-m,%-y,$v) += $(call expand-objs, $($v)))))
There are two changes:
1) You must not build modules if they are disabled
2) Because the .mo file has no dependencies, you need expand-objs. (In
fact we should just stop using .mo files except as an artificial construct
in Makefile.objs).
> +
> + # Post-process all the unnested vars
> + $(foreach v,$2,
> + $(foreach o, $(filter %.mo,$($v)),
> + # Find all the .mo objects in variables and add dependency rules
> + # according to .mo-objs. Report error if not set
> + $(if $($o-objs),
> + $(eval $(o:%.mo=%$(DSOSUF)): $($o-objs)),
> + $(error $o added in $v but $o-objs is not set))
> + # Pass the .mo-cflags along to member objects
> + $(if $($o-cflags),
> + $(foreach p,$($o-objs),
> + $(eval $p-cflags := $($o-cflags)))))
Finally, this should be a +=.
I'll send an updated pull request.
Paolo
> + $(shell mkdir -p ./ $(sort $(dir $($v))))
> + # Include all the .d files
> + $(eval -include $(addsuffix *.d, $(sort $(dir $($v)))))
> + $(eval $v := $(filter-out %/,$($v))))
> endef
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 7/8] configure: duplicate/incorrect order of -lrt
2014-06-10 5:56 [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Paolo Bonzini
` (5 preceding siblings ...)
2014-06-10 5:56 ` [Qemu-devel] [PULL 6/8] rules.mak: Rewrite unnest-vars Paolo Bonzini
@ 2014-06-10 5:56 ` Paolo Bonzini
2014-06-10 5:56 ` [Qemu-devel] [PULL 8/8] configure: unset interfering variables Paolo Bonzini
2014-06-10 10:00 ` [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Peter Maydell
8 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-10 5:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Rick Liu
From: Rick Liu <yrliu.ca@gmail.com>
'-lrt' flag duplication/incorrect order
would cause 'undefined reference to clock_gettime' error during compilation time.
Before fix:
... -o qemu-bridge-helper qemu-bridge-helper.o -lrt -pthread -lgthread-2.0 -lrt -lglib-2.0
After fix:
... -o qemu-bridge-helper qemu-bridge-helper.o -pthread -lgthread-2.0 -lrt -lglib-2.0
Reference:
http://hi.baidu.com/sanitywolf/item/7a8b69c1e76dd220a0b50ab1
Signed-off-by: Rick Liu <yrliu.ca@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
configure | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index a994f41..96f91ad 100755
--- a/configure
+++ b/configure
@@ -3452,9 +3452,9 @@ EOF
if compile_prog "" "" ; then
:
# we need pthread for static linking. use previous pthread test result
-elif compile_prog "" "-lrt $pthread_lib" ; then
- LIBS="-lrt $LIBS"
- libs_qga="-lrt $libs_qga"
+elif compile_prog "" "$pthread_lib -lrt" ; then
+ LIBS="$LIBS -lrt"
+ libs_qga="$libs_qga -lrt"
fi
if test "$darwin" != "yes" -a "$mingw32" != "yes" -a "$solaris" != yes -a \
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 8/8] configure: unset interfering variables
2014-06-10 5:56 [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Paolo Bonzini
` (6 preceding siblings ...)
2014-06-10 5:56 ` [Qemu-devel] [PULL 7/8] configure: duplicate/incorrect order of -lrt Paolo Bonzini
@ 2014-06-10 5:56 ` Paolo Bonzini
2014-06-10 10:00 ` [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Peter Maydell
8 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-06-10 5:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Cornelia Huck
From: Cornelia Huck <cornelia.huck@de.ibm.com>
The check for big or little endianness relies on grep reporting
match/non-match on the generated binary. If the user specified
--binary-files=without-match in their GREP_OPTIONS, this will fail.
Let's follow what autoconf does and unset GREP_OPTIONS and CLICOLOR_FORCE
at the beginning of the script.
Reported-by: Eugene (jno) Dvurechenski <jno@linux.vnet.ibm.com>
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
configure | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/configure b/configure
index 96f91ad..b93cce8 100755
--- a/configure
+++ b/configure
@@ -3,6 +3,11 @@
# qemu configure script (c) 2003 Fabrice Bellard
#
+# Unset some variables known to interfere with behavior of common tools,
+# just as autoconf does.
+CLICOLOR_FORCE= GREP_OPTIONS=
+unset CLICOLOR_FORCE GREP_OPTIONS
+
# Temporary directory used for files created while
# configure runs. Since it is in the build directory
# we can safely blow away any previous version of it
--
1.8.3.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10
2014-06-10 5:56 [Qemu-devel] [PULL 0/8] build system and libcacard changes for 2014-06-10 Paolo Bonzini
` (7 preceding siblings ...)
2014-06-10 5:56 ` [Qemu-devel] [PULL 8/8] configure: unset interfering variables Paolo Bonzini
@ 2014-06-10 10:00 ` Peter Maydell
8 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2014-06-10 10:00 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU Developers
On 10 June 2014 06:56, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 7721a3044234c46cd6f5f899e7467dc9351f3c8d:
>
> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20140609-1' into staging (2014-06-09 17:04:13 +0100)
>
> are available in the git repository at:
>
>
> git://github.com/bonzini/qemu.git configure
>
> for you to fetch changes up to 2e77e6ee829ede0c20e968c71c99c2fbd9eedf9b:
>
> configure: unset interfering variables (2014-06-10 07:51:42 +0200)
There seem to be problems with the rules.mak rewrite patch
(as discussed on irc) so I'm not applying this one.
[sample repro: http://pastebin.ubuntu.com/7622677/ ]
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread