On Sat, Dec 15, 2018 at 07:53:08AM -0600, Eric Blake wrote: > Connecting to a /dev/nbdN device is a Linux-specific action. > We were already masking -c and -d from 'qemu-nbd --help' on > non-linux. However, while -d fails with a sensible error > message, it took hunting through a couple of files to prove > that. What's more, the code for -c doesn't fail until after > it has created a pthread and tried to open a device - possibly > even printing an error message with %m on a non-Linux platform > in spite of the comment that %m is glibc-specific. Make the > failure happen sooner, then get rid of stubs that are no > longer needed because of the early exits. > > While at it: tweak the blank newlines in --help output to be > consistent, whether or not built on Linux. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v2: Hoist -c error message to share with -d message [Vladimir] > Bonus: gets rid of a stray TAB in nbd/client.c > --- > nbd/client.c | 18 +----------------- > qemu-nbd.c | 21 +++++++++++++++++++-- > 2 files changed, 20 insertions(+), 19 deletions(-) > > diff --git a/nbd/client.c b/nbd/client.c > index 5d59d5ba78a..3d9086af39d 100644 > --- a/nbd/client.c > +++ b/nbd/client.c > @@ -1029,23 +1029,7 @@ int nbd_disconnect(int fd) > return 0; > } > > -#else > -int nbd_init(int fd, QIOChannelSocket *ioc, NBDExportInfo *info, > - Error **errp) > -{ > - error_setg(errp, "nbd_init is only supported on Linux"); > - return -ENOTSUP; > -} > - > -int nbd_client(int fd) > -{ > - return -ENOTSUP; > -} > -int nbd_disconnect(int fd) > -{ > - return -ENOTSUP; > -} > -#endif > +#endif /* __linux__ */ > > int nbd_send_request(QIOChannel *ioc, NBDRequest *request) > { > diff --git a/qemu-nbd.c b/qemu-nbd.c > index e169b839ece..2807e132396 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -43,6 +43,12 @@ > #include "trace/control.h" > #include "qemu-version.h" > > +#ifdef __linux__ > +#define HAVE_NBD_DEVICE 1 > +#else > +#define HAVE_NBD_DEVICE 0 > +#endif > + > #define SOCKET_PATH "/var/lock/qemu-nbd-%s" > #define QEMU_NBD_OPT_CACHE 256 > #define QEMU_NBD_OPT_AIO 257 > @@ -98,11 +104,11 @@ static void usage(const char *name) > " specify tracing options\n" > " --fork fork off the server process and exit the > parent\n" > " once the server is running\n" > -#ifdef __linux__ > +#if HAVE_NBD_DEVICE > +"\n" > "Kernel NBD client support:\n" > " -c, --connect=DEV connect FILE to the local NBD device DEV\n" > " -d, --disconnect disconnect the specified device\n" > -"\n" > #endif > "\n" > "Block device options:\n" > @@ -236,6 +242,7 @@ static void termsig_handler(int signum) > } > > > +#if HAVE_NBD_DEVICE > static void *show_parts(void *arg) > { > char *device = arg; > @@ -321,6 +328,7 @@ out: > kill(getpid(), SIGTERM); > return (void *) EXIT_FAILURE; > } > +#endif /* HAVE_NBD_DEVICE */ > > static int nbd_can_accept(void) > { > @@ -814,6 +822,12 @@ int main(int argc, char **argv) > } > } > > +#if !HAVE_NBD_DEVICE > + if (disconnect || device) { > + error_report("Kernel /dev/nbdN support not available"); > + exit(EXIT_FAILURE); > + } > +#else /* HAVE_NBD_DEVICE */ > if (disconnect) { > int nbdfd = open(argv[optind], O_RDWR); > if (nbdfd < 0) { > @@ -829,6 +843,7 @@ int main(int argc, char **argv) > > return 0; > } > +#endif > > if ((device && !verbose) || fork_process) { > int stderr_fd[2]; > @@ -1006,6 +1021,7 @@ int main(int argc, char **argv) > nbd_export_set_description(exp, export_description); > > if (device) { > +#if HAVE_NBD_DEVICE > int ret; > > ret = pthread_create(&client_thread, NULL, nbd_client_thread, > device); > @@ -1013,6 +1029,7 @@ int main(int argc, char **argv) > error_report("Failed to create client thread: %s", > strerror(ret)); > exit(EXIT_FAILURE); > } > +#endif > } else { > /* Shut up GCC warnings. */ > memset(&client_thread, 0, sizeof(client_thread)); > --
An obvious simplification of the last version, so: Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top