On 09/19/2011 04:32 PM, Lon Hohberger wrote:
> On 09/13/2011 04:44 AM, Kazunori INOUE wrote:
> 
>> I attached the patch for this problem.
>>
>> * Modified to send a SIGHUP to the primary thread from the secondary
>>     thread detects the start of the VM, and
>>     the primary thread is modified so as to select() the requests from
>>     the VM which started.
> 
> That's certainly one way of doing it.  I might have used
> 'pthread_cond_singal'.

I am sorry for the delay.

I have a fix based on yours that uses a pipe to wake up select() rather
than pthread_kill; I'll attach it here.  This ends up being a bit lower
impact, as well; it doesn't require changing the main program - only the
serial/vmchannel plugin.

Apart from that, the patch here is largely the same.

Let me know what you think at your convenience.

-- Lon
From 5a9431e16e23ea24f5d86a17a83e4edc3f112afd Mon Sep 17 00:00:00 2001
From: Lon Hohberger <l...@users.sourceforge.net>
Date: Tue, 25 Oct 2011 19:29:24 -0400
Subject: [PATCH] Fix serial domain handling

---
 server/serial.c      |   19 ++++++++++++-
 server/serial.h      |    2 +-
 server/virt-serial.c |   71 ++++++++++++++++++++++++-------------------------
 3 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/server/serial.c b/server/serial.c
index 1e7bdbc..42ba389 100644
--- a/server/serial.c
+++ b/server/serial.c
@@ -76,6 +76,7 @@ typedef struct _serial_info {
        history_info_t *history;
        map_object_t *maps;
        int mode;
+       int wake_fd;
 } serial_info;
 
 
@@ -262,12 +263,16 @@ serial_dispatch(listener_context_t c, struct timeval 
*timeout)
        struct timeval tv;
        int max;
        int n, x, ret;
+       char tmp_buf[32];
 
        info = (serial_info *)c;
        VALIDATE(info);
 
        FD_ZERO(&rfds);
        domain_sock_fdset(&rfds, &max);
+       FD_SET(info->wake_fd, &rfds);
+       if (info->wake_fd > max)
+               max = info->wake_fd;
 
        n = select(max+1, &rfds, NULL, NULL, timeout);
        if (n < 0) {
@@ -275,6 +280,18 @@ serial_dispatch(listener_context_t c, struct timeval 
*timeout)
                return n;
        }
 
+       /*
+        * See if the goal was just to be woken up in order to refill our
+        * file descriptor set.  For example, if multiple domains were 
+        * created simultaneously, we would have to refill our fd_set
+        */
+       if (FD_ISSET(info->wake_fd, &rfds)) {
+               tv.tv_sec = 0;
+               tv.tv_usec = 10000;
+               _read_retry(info->wake_fd, &c, 1, &tv);
+               return 0;
+       }
+
        /* 
         * If no requests, we're done 
         */
@@ -392,7 +409,7 @@ serial_init(listener_context_t *c, const fence_callbacks_t 
*cb,
        info->magic = SERIAL_PLUG_MAGIC;
        info->history = history_init(check_history, 10, sizeof(fence_req_t));
        *c = (listener_context_t)info;
-       start_event_listener(info->uri, info->path, info->mode);
+       start_event_listener(info->uri, info->path, info->mode, &info->wake_fd);
        sleep(1);
 
        return 0;
diff --git a/server/serial.h b/server/serial.h
index 0b3e480..fb8e575 100644
--- a/server/serial.h
+++ b/server/serial.h
@@ -13,7 +13,7 @@ int domain_sock_name(int fd, char *outbuf, size_t buflen);
 int domain_sock_cleanup(void);
 
 /* virt-serial.c - event thread control functions */
-int start_event_listener(const char *uri, const char *path, int mode);
+int start_event_listener(const char *uri, const char *path, int mode, int 
*wake_fd);
 int stop_event_listener(void);
 
 
diff --git a/server/virt-serial.c b/server/virt-serial.c
index 065f846..039bacd 100644
--- a/server/virt-serial.c
+++ b/server/virt-serial.c
@@ -69,25 +69,6 @@ struct domain_info {
 };
 
 
-int
-myDomainEventCallback1(virConnectPtr conn,
-                      virDomainPtr dom, int event, int detail, void *opaque)
-{
-       struct domain_info *dinfo = (struct domain_info *) opaque;
-
-       if (event == VIR_DOMAIN_EVENT_STARTED ||
-           event == VIR_DOMAIN_EVENT_STOPPED) {
-               virDomainRef(dom);
-               dinfo->dom = dom;
-               dinfo->event = event;
-       } else {
-               dinfo->event = VIR_DOMAIN_EVENT_UNDEFINED;
-       }
-
-       return 0;
-}
-
-
 /* EventImpl Functions */
 int
 myEventHandleTypeToPollEvent(virEventHandleType events)
@@ -413,15 +394,38 @@ struct event_args {
        char *uri;
        char *path;
        int mode;
+       int wake_fd;
 };
 
 
+int
+myDomainEventCallback1(virConnectPtr conn,
+                      virDomainPtr dom, int event, int detail, void *opaque)
+{
+       struct event_args *args = (struct event_args *)opaque;
+
+       if (event == VIR_DOMAIN_EVENT_STARTED ||
+           event == VIR_DOMAIN_EVENT_STOPPED) {
+               virDomainRef(dom);
+               if (event == VIR_DOMAIN_EVENT_STARTED) {
+                       domainStarted(dom, args->path, args->mode);
+                       virDomainFree(dom);
+                       write(args->wake_fd, "x", 1);
+               } else if (event == VIR_DOMAIN_EVENT_STOPPED) {
+                       domainStopped(dom);
+                       virDomainFree(dom);
+               }
+       }
+
+       return 0;
+}
+
+
 static void *
 event_thread(void *arg)
 {
        struct event_args *args = (struct event_args *)arg;
        virConnectPtr dconn = NULL;
-       struct domain_info dinfo;
        int callback1ret = -1;
        int sts;
 
@@ -450,11 +454,9 @@ event_thread(void *arg)
        registerExisting(dconn, args->path, args->mode);
 
        /* Add 2 callbacks to prove this works with more than just one */
-       memset(&dinfo, 0, sizeof (dinfo));
-       dinfo.event = VIR_DOMAIN_EVENT_UNDEFINED;
        callback1ret =
            virConnectDomainEventRegister(dconn, myDomainEventCallback1,
-                                         &dinfo, NULL);
+                                         arg, NULL);
 
        if ((callback1ret == 0)) {
                while (run) {
@@ -469,18 +471,6 @@ event_thread(void *arg)
                                t_cb(t_timeout, t_opaque);
                        }
 
-                       if (dinfo.event == VIR_DOMAIN_EVENT_STARTED) {
-                               domainStarted(dinfo.dom, args->path, 
args->mode);
-                               virDomainFree(dinfo.dom);
-                               dinfo.dom = NULL;
-                               dinfo.event = VIR_DOMAIN_EVENT_UNDEFINED;
-                       } else if (dinfo.event == VIR_DOMAIN_EVENT_STOPPED) {
-                               domainStopped(dinfo.dom);
-                               virDomainFree(dinfo.dom);
-                               dinfo.dom = NULL;
-                               dinfo.event = VIR_DOMAIN_EVENT_UNDEFINED;
-                       }
-
                        if (sts == 0) {
                                /* DEBUG0("Poll timeout"); */
                                continue;
@@ -522,9 +512,10 @@ out:
 
 
 int
-start_event_listener(const char *uri, const char *path, int mode)
+start_event_listener(const char *uri, const char *path, int mode, int *wake_fd)
 {
        struct event_args *args = NULL;
+       int wake_pipe[2];
 
        virInitialize();
 
@@ -533,6 +524,10 @@ start_event_listener(const char *uri, const char *path, 
int mode)
                return -1;
        memset(args, 0, sizeof(*args));
        
+       if (pipe2(wake_pipe, O_CLOEXEC) < 0) {
+               goto out_fail;
+       }
+
        if (uri) {
               args->uri = strdup(uri);
               if (args->uri == NULL)
@@ -546,6 +541,10 @@ start_event_listener(const char *uri, const char *path, 
int mode)
        }
 
        args->mode = mode;
+       //args->p_tid = pthread_self();
+       *wake_fd = wake_pipe[0];
+       args->wake_fd = wake_pipe[1];
+
        run = 1;
 
        return pthread_create(&event_tid, NULL, event_thread, args);
-- 
1.7.3.4

Reply via email to