Patchwork [03/11] qemu-sockets: make wait_for_connect be invoked in qemu_aio_wait

login
register
mail settings
Submitter MORITA Kazutaka
Date July 23, 2013, 8:30 a.m.
Message ID <1374568221-23147-4-git-send-email-morita.kazutaka@lab.ntt.co.jp>
Download mbox | patch
Permalink /patch/260976/
State New
Headers show

Comments

MORITA Kazutaka - July 23, 2013, 8:30 a.m.
This allows us to use inet_nonblocking_connect() and
unix_nonblocking_connect() in block drivers.

qemu-ga needs to link block-obj to resolve dependencies of
qemu_aio_set_fd_handler().

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 Makefile            |  4 ++--
 util/qemu-sockets.c | 15 ++++++++++-----
 2 files changed, 12 insertions(+), 7 deletions(-)
Paolo Bonzini - July 23, 2013, 11:36 a.m.
Il 23/07/2013 10:30, MORITA Kazutaka ha scritto:
> This allows us to use inet_nonblocking_connect() and
> unix_nonblocking_connect() in block drivers.
> 
> qemu-ga needs to link block-obj to resolve dependencies of
> qemu_aio_set_fd_handler().
> 
> Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>

I'm not sure this is safe.  You could have e.g. migration start during
qemu_aio_wait().

Paolo

> ---
>  Makefile            |  4 ++--
>  util/qemu-sockets.c | 15 ++++++++++-----
>  2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c06bfab..5fe2e0f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -197,7 +197,7 @@ fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
>  qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
>  	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $@")
>  
> -qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
> +qemu-ga$(EXESUF): LIBS = $(LIBS_QGA) $(LIBS_TOOLS)
>  qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
>  
>  gen-out-type = $(subst .,-,$(suffix $@))
> @@ -227,7 +227,7 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
>  QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
>  $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
>  
> -qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
> +qemu-ga$(EXESUF): $(qga-obj-y) $(block-obj-y) libqemuutil.a libqemustub.a
>  	$(call LINK, $^)
>  
>  clean:
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 095716e..8b21fd1 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -218,6 +218,11 @@ typedef struct ConnectState {
>  static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>                               ConnectState *connect_state, Error **errp);
>  
> +static int return_true(void *opaque)
> +{
> +    return 1;
> +}
> +
>  static void wait_for_connect(void *opaque)
>  {
>      ConnectState *s = opaque;
> @@ -225,7 +230,7 @@ static void wait_for_connect(void *opaque)
>      socklen_t valsize = sizeof(val);
>      bool in_progress;
>  
> -    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
> +    qemu_aio_set_fd_handler(s->fd, NULL, NULL, NULL, NULL);
>  
>      do {
>          rc = qemu_getsockopt(s->fd, SOL_SOCKET, SO_ERROR, &val, &valsize);
> @@ -288,8 +293,8 @@ static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
>  
>      if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
>          connect_state->fd = sock;
> -        qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
> -                             connect_state);
> +        qemu_aio_set_fd_handler(sock, NULL, wait_for_connect, return_true,
> +                                connect_state);
>          *in_progress = true;
>      } else if (rc < 0) {
>          error_set_errno(errp, errno, QERR_SOCKET_CONNECT_FAILED);
> @@ -749,8 +754,8 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
>  
>      if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
>          connect_state->fd = sock;
> -        qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
> -                             connect_state);
> +        qemu_aio_set_fd_handler(sock, NULL, wait_for_connect, return_true,
> +                                connect_state);
>          return sock;
>      } else if (rc >= 0) {
>          /* non blocking socket immediate success, call callback */
>
MORITA Kazutaka - July 24, 2013, 7:41 a.m.
At Tue, 23 Jul 2013 13:36:08 +0200,
Paolo Bonzini wrote:
> 
> Il 23/07/2013 10:30, MORITA Kazutaka ha scritto:
> > This allows us to use inet_nonblocking_connect() and
> > unix_nonblocking_connect() in block drivers.
> > 
> > qemu-ga needs to link block-obj to resolve dependencies of
> > qemu_aio_set_fd_handler().
> > 
> > Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> 
> I'm not sure this is safe.  You could have e.g. migration start during
> qemu_aio_wait().

I thought that it is safe.  Qemu creates another thread for migration
and it can be started at any time, either way.  However, so as not to
hurt the existing codes, it might be better to create another
nonblocking connect for qemu_aio_wait().

I think of dropping this patch from this series and will leave it for
another day.  Usually, sheepdog users prepare a local sheepdog daemon
to be connected to, and connect() is unlikely to sleep for a long
time.  Using a blocking connect wouldn't be a big problem.

Thanks,

Kazutaka

Patch

diff --git a/Makefile b/Makefile
index c06bfab..5fe2e0f 100644
--- a/Makefile
+++ b/Makefile
@@ -197,7 +197,7 @@  fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $@")
 
-qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
+qemu-ga$(EXESUF): LIBS = $(LIBS_QGA) $(LIBS_TOOLS)
 qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
 
 gen-out-type = $(subst .,-,$(suffix $@))
@@ -227,7 +227,7 @@  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 
-qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
+qemu-ga$(EXESUF): $(qga-obj-y) $(block-obj-y) libqemuutil.a libqemustub.a
 	$(call LINK, $^)
 
 clean:
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 095716e..8b21fd1 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -218,6 +218,11 @@  typedef struct ConnectState {
 static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
                              ConnectState *connect_state, Error **errp);
 
+static int return_true(void *opaque)
+{
+    return 1;
+}
+
 static void wait_for_connect(void *opaque)
 {
     ConnectState *s = opaque;
@@ -225,7 +230,7 @@  static void wait_for_connect(void *opaque)
     socklen_t valsize = sizeof(val);
     bool in_progress;
 
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    qemu_aio_set_fd_handler(s->fd, NULL, NULL, NULL, NULL);
 
     do {
         rc = qemu_getsockopt(s->fd, SOL_SOCKET, SO_ERROR, &val, &valsize);
@@ -288,8 +293,8 @@  static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
 
     if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
         connect_state->fd = sock;
-        qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
-                             connect_state);
+        qemu_aio_set_fd_handler(sock, NULL, wait_for_connect, return_true,
+                                connect_state);
         *in_progress = true;
     } else if (rc < 0) {
         error_set_errno(errp, errno, QERR_SOCKET_CONNECT_FAILED);
@@ -749,8 +754,8 @@  int unix_connect_opts(QemuOpts *opts, Error **errp,
 
     if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
         connect_state->fd = sock;
-        qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
-                             connect_state);
+        qemu_aio_set_fd_handler(sock, NULL, wait_for_connect, return_true,
+                                connect_state);
         return sock;
     } else if (rc >= 0) {
         /* non blocking socket immediate success, call callback */