[PATCH v2 0/3] nbd: build qemu-nbd on Windows

[PATCH v2 0/3] nbd: build qemu-nbd on Windows

[Daniel P. Berrangé]

We are already building the NBD client and server on Windows when it is
used via the main system emulator binaries. This demonstrates there is
no fundamental blocker to buildig the qemu-nbd binary too.

Changed in v2:

 - Split second patch into two parts
 - Use  HAVE_NBD_DEVICE condition to disable SIGTERM handler not WIN32

Daniel P. Berrangé (3):
  block: add missing socket_init() calls to tools
  nbd: skip SIGTERM handler if NBD device support is not built
  nbd: disable signals and forking on Windows builds

 meson.build |  7 ++-----
 qemu-img.c  |  2 ++
 qemu-io.c   |  2 ++
 qemu-nbd.c  | 11 ++++++++++-
 4 files changed, 16 insertions(+), 6 deletions(-)

-- 
2.26.2

[PATCH v2 1/3] block: add missing socket_init() calls to tools

[Daniel P. Berrangé]

Any tool that uses sockets needs to call socket_init() in order to work
on the Windows platform.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qemu-img.c | 2 ++
 qemu-io.c  | 2 ++
 qemu-nbd.c | 1 +
 3 files changed, 5 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index 5308773811..eb2fc1f862 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -41,6 +41,7 @@
 #include "qemu/log.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
+#include "qemu/sockets.h"
 #include "qemu/units.h"
 #include "qom/object_interfaces.h"
 #include "sysemu/block-backend.h"
@@ -5410,6 +5411,7 @@ int main(int argc, char **argv)
     signal(SIGPIPE, SIG_IGN);
 #endif
 
+    socket_init();
     error_init(argv[0]);
     module_call_init(MODULE_INIT_TRACE);
     qemu_init_exec_dir(argv[0]);
diff --git a/qemu-io.c b/qemu-io.c
index 3adc5a7d0d..7cc832b3d6 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -25,6 +25,7 @@
 #include "qemu/config-file.h"
 #include "qemu/readline.h"
 #include "qemu/log.h"
+#include "qemu/sockets.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qdict.h"
 #include "qom/object_interfaces.h"
@@ -542,6 +543,7 @@ int main(int argc, char **argv)
     signal(SIGPIPE, SIG_IGN);
 #endif
 
+    socket_init();
     error_init(argv[0]);
     module_call_init(MODULE_INIT_TRACE);
     qemu_init_exec_dir(argv[0]);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index d2657b8db5..b102874f0f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -599,6 +599,7 @@ int main(int argc, char **argv)
     signal(SIGPIPE, SIG_IGN);
 #endif
 
+    socket_init();
     error_init(argv[0]);
     module_call_init(MODULE_INIT_TRACE);
     qcrypto_init(&error_fatal);
-- 
2.26.2

[PATCH v2 2/3] nbd: skip SIGTERM handler if NBD device support is not built

[Daniel P. Berrangé]

The termsig_handler function is used by the client thread handling the
host NBD device connection to do a graceful shutdown. IOW, if we have
disabled NBD device support at compile time, we don't need the SIGTERM
handler. This fixes a build issue for Windows.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qemu-nbd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index b102874f0f..dc6ef089af 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -155,12 +155,13 @@ QEMU_COPYRIGHT "\n"
     , name);
 }
 
+#if HAVE_NBD_DEVICE
 static void termsig_handler(int signum)
 {
     atomic_cmpxchg(&state, RUNNING, TERMINATE);
     qemu_notify_event();
 }
-
+#endif /* HAVE_NBD_DEVICE */
 
 static int qemu_nbd_client_list(SocketAddress *saddr, QCryptoTLSCreds *tls,
                                 const char *hostname)
@@ -587,6 +588,7 @@ int main(int argc, char **argv)
     unsigned socket_activation;
     const char *pid_file_name = NULL;
 
+#if HAVE_NBD_DEVICE
     /* The client thread uses SIGTERM to interrupt the server.  A signal
      * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
      */
@@ -594,6 +596,7 @@ int main(int argc, char **argv)
     memset(&sa_sigterm, 0, sizeof(sa_sigterm));
     sa_sigterm.sa_handler = termsig_handler;
     sigaction(SIGTERM, &sa_sigterm, NULL);
+#endif /* HAVE_NBD_DEVICE */
 
 #ifdef CONFIG_POSIX
     signal(SIGPIPE, SIG_IGN);
-- 
2.26.2

[PATCH v2 3/3] nbd: disable signals and forking on Windows builds

[Daniel P. Berrangé]

Disabling these parts are sufficient to get the qemu-nbd program
compiling in a Windows build.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 meson.build | 7 ++-----
 qemu-nbd.c  | 5 +++++
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index df5bf728b5..1071871605 100644
--- a/meson.build
+++ b/meson.build
@@ -1074,12 +1074,9 @@ if have_tools
              dependencies: [authz, block, crypto, io, qom, qemuutil], install: true)
   qemu_io = executable('qemu-io', files('qemu-io.c'),
              dependencies: [block, qemuutil], install: true)
-  qemu_block_tools += [qemu_img, qemu_io]
-  if targetos == 'linux' or targetos == 'sunos' or targetos.endswith('bsd')
-    qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
+  qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
                dependencies: [block, qemuutil], install: true)
-    qemu_block_tools += [qemu_nbd]
-  endif
+  qemu_block_tools += [qemu_img, qemu_io, qemu_nbd]
 
   subdir('storage-daemon')
   subdir('contrib/rdmacm-mux')
diff --git a/qemu-nbd.c b/qemu-nbd.c
index dc6ef089af..33476a1000 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -899,6 +899,7 @@ int main(int argc, char **argv)
 #endif
 
     if ((device && !verbose) || fork_process) {
+#ifndef WIN32
         int stderr_fd[2];
         pid_t pid;
         int ret;
@@ -962,6 +963,10 @@ int main(int argc, char **argv)
              */
             exit(errors);
         }
+#else /* WIN32 */
+        error_report("Unable to fork into background on Windows hosts");
+        exit(EXIT_FAILURE);
+#endif /* WIN32 */
     }
 
     if (device != NULL && sockpath == NULL) {
-- 
2.26.2

Re: [PATCH v2 2/3] nbd: skip SIGTERM handler if NBD device support is not built

[Eric Blake]

On 8/25/20 5:38 AM, Daniel P. Berrangé wrote:
> The termsig_handler function is used by the client thread handling the
> host NBD device connection to do a graceful shutdown. IOW, if we have
> disabled NBD device support at compile time, we don't need the SIGTERM
> handler. This fixes a build issue for Windows.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   qemu-nbd.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 3/3] nbd: disable signals and forking on Windows builds

[Eric Blake]

On 8/25/20 5:38 AM, Daniel P. Berrangé wrote:
> Disabling these parts are sufficient to get the qemu-nbd program
> compiling in a Windows build.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   meson.build | 7 ++-----
>   qemu-nbd.c  | 5 +++++
>   2 files changed, 7 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

Queuing through my NBD tree.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 3/3] nbd: disable signals and forking on Windows builds

[罗勇刚(Yonggang Luo)]

[-- Attachment #1: Type: text/plain, Size: 2069 bytes --]

On Tue, Aug 25, 2020 at 6:40 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> Disabling these parts are sufficient to get the qemu-nbd program
> compiling in a Windows build.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  meson.build | 7 ++-----
>  qemu-nbd.c  | 5 +++++
>  2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index df5bf728b5..1071871605 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1074,12 +1074,9 @@ if have_tools
>               dependencies: [authz, block, crypto, io, qom, qemuutil],
> install: true)
>    qemu_io = executable('qemu-io', files('qemu-io.c'),
>               dependencies: [block, qemuutil], install: true)
> -  qemu_block_tools += [qemu_img, qemu_io]
> -  if targetos == 'linux' or targetos == 'sunos' or
> targetos.endswith('bsd')
> -    qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
> +  qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
>                 dependencies: [block, qemuutil], install: true)
> -    qemu_block_tools += [qemu_nbd]
> -  endif
> +  qemu_block_tools += [qemu_img, qemu_io, qemu_nbd]
>
>    subdir('storage-daemon')
>    subdir('contrib/rdmacm-mux')
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index dc6ef089af..33476a1000 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -899,6 +899,7 @@ int main(int argc, char **argv)
>  #endif
>
>      if ((device && !verbose) || fork_process) {
> +#ifndef WIN32
>          int stderr_fd[2];
>          pid_t pid;
>          int ret;
> @@ -962,6 +963,10 @@ int main(int argc, char **argv)
>               */
>              exit(errors);
>          }
> +#else /* WIN32 */
> +        error_report("Unable to fork into background on Windows hosts");
> +        exit(EXIT_FAILURE);
> +#endif /* WIN32 */
>      }
>
May us replace fork with alternative such as spawn?

>
>      if (device != NULL && sockpath == NULL) {
> --
> 2.26.2
>
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[-- Attachment #2: Type: text/html, Size: 2986 bytes --]

Re: [PATCH v2 3/3] nbd: disable signals and forking on Windows builds

[Eric Blake]

On 9/2/20 5:07 PM, 罗勇刚(Yonggang Luo) wrote:
> On Tue, Aug 25, 2020 at 6:40 PM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
>> Disabling these parts are sufficient to get the qemu-nbd program
>> compiling in a Windows build.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   meson.build | 7 ++-----
>>   qemu-nbd.c  | 5 +++++
>>   2 files changed, 7 insertions(+), 5 deletions(-)

>> +++ b/qemu-nbd.c
>> @@ -899,6 +899,7 @@ int main(int argc, char **argv)
>>   #endif
>>
>>       if ((device && !verbose) || fork_process) {
>> +#ifndef WIN32
>>           int stderr_fd[2];
>>           pid_t pid;
>>           int ret;
>> @@ -962,6 +963,10 @@ int main(int argc, char **argv)
>>                */
>>               exit(errors);
>>           }
>> +#else /* WIN32 */
>> +        error_report("Unable to fork into background on Windows hosts");
>> +        exit(EXIT_FAILURE);
>> +#endif /* WIN32 */
>>       }
>>
> May us replace fork with alternative such as spawn?

You're certainly welcome to propose a patch along those lines, if 
spawning a task is a common Windows counterpart to the Unix notion of 
forking off a daemon.  But even requiring qemu-nbd to run in the 
foreground is already an improvement over what we had previously, so any 
change to use spawn will be a separate series, and will not hold up this 
one.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 3/3] nbd: disable signals and forking on Windows builds

[罗勇刚(Yonggang Luo)]

[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]

On Thu, Sep 3, 2020 at 7:29 AM Eric Blake <eblake@redhat.com> wrote:

> On 9/2/20 5:07 PM, 罗勇刚(Yonggang Luo) wrote:
> > On Tue, Aug 25, 2020 at 6:40 PM Daniel P. Berrangé <berrange@redhat.com>
> > wrote:
> >
> >> Disabling these parts are sufficient to get the qemu-nbd program
> >> compiling in a Windows build.
> >>
> >> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> >> ---
> >>   meson.build | 7 ++-----
> >>   qemu-nbd.c  | 5 +++++
> >>   2 files changed, 7 insertions(+), 5 deletions(-)
>
> >> +++ b/qemu-nbd.c
> >> @@ -899,6 +899,7 @@ int main(int argc, char **argv)
> >>   #endif
> >>
> >>       if ((device && !verbose) || fork_process) {
> >> +#ifndef WIN32
> >>           int stderr_fd[2];
> >>           pid_t pid;
> >>           int ret;
> >> @@ -962,6 +963,10 @@ int main(int argc, char **argv)
> >>                */
> >>               exit(errors);
> >>           }
> >> +#else /* WIN32 */
> >> +        error_report("Unable to fork into background on Windows
> hosts");
> >> +        exit(EXIT_FAILURE);
> >> +#endif /* WIN32 */
> >>       }
> >>
> > May us replace fork with alternative such as spawn?
>
> You're certainly welcome to propose a patch along those lines, if
> spawning a task is a common Windows counterpart to the Unix notion of
> forking off a daemon.  But even requiring qemu-nbd to run in the
> foreground is already an improvement over what we had previously, so any
> change to use spawn will be a separate series, and will not hold up this
>
Yes, of cause.

> one.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
>
>

-- 
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[-- Attachment #2: Type: text/html, Size: 2948 bytes --]