Re: [Qemu-devel] [PATCH 03/14] qemu-nbd: Fail earlier for -c/-d on non-linux

[Vladimir Sementsov-Ogievskiy]

01.12.2018 1:03, 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 <address@hidden>
> ---
[...]

> @@ -815,6 +823,7 @@ int main(int argc, char **argv)
>       }
> 
>       if (disconnect) {
> +#if HAVE_NBD_DEVICE
>           int nbdfd = open(argv[optind], O_RDWR);
>           if (nbdfd < 0) {
>               error_report("Cannot open %s: %s", argv[optind],
> @@ -828,6 +837,10 @@ int main(int argc, char **argv)
>           printf("%s disconnected\n", argv[optind]);
> 
>           return 0;
> +#else
> +        error_report("Kernel /dev/nbdN support not available");
> +        exit(EXIT_FAILURE);
> +#endif
>       }
> 
>       if ((device && !verbose) || fork_process) {
> @@ -1006,6 +1019,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 +1027,10 @@ int main(int argc, char **argv)
>               error_report("Failed to create client thread: %s", 
> strerror(ret));
>               exit(EXIT_FAILURE);
>           }
> +#else
> +        error_report("Kernel /dev/nbdN support not available");
> +        exit(EXIT_FAILURE);

hmm, we have some if (device ...) conditions with extra logic above this point.
I think it's better to fail earlier in this case. For example, exactly after
disconnect hunk

> +#endif
>       } else {
>           /* Shut up GCC warnings.  */
>           memset(&client_thread, 0, sizeof(client_thread));
> 


-- 
Best regards,
Vladimir