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 <lon@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