Re: [Qemu-devel] ping [PATCH v14] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Kevin Wolf <kwolf@redhat.com>]

Am 29.02.2016 um 16:17 hat Programmingkid geschrieben:
> I do think this patch is ready to be added to QEMU. I have listened to what you said and implemented your changes. 
> 
> https://patchwork.ozlabs.org/patch/579325/
> 
> Mac OS X can be picky when it comes to allowing the user
> to use physical devices in QEMU. Most mounted volumes
> appear to be off limits to QEMU. If an issue is detected,
> a message is displayed showing the user how to unmount a
> volume. Now QEMU uses both CD and DVD media.
> 
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> 
> ---
> Changed filename variable to const char * type.
> Removed snprintf call for filename variable.
> filename is set to bsd_path if using a physical device that isn't a DVD or CD.

> @@ -2112,33 +2166,57 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
>  
>  #if defined(__APPLE__) && defined(__MACH__)
>      const char *filename = qdict_get_str(options, "filename");
> +    char bsd_path[MAXPATHLEN] = "";
> +    bool error_occurred = false;
> +    

This line adds trailing whitespace.

> @@ -2147,7 +2225,16 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
>          if (local_err) {
>              error_propagate(errp, local_err);
>          }
> -        return ret;
> +#if defined(__APPLE__) && defined(__MACH__)
> +        if (*bsd_path) {
> +            filename = bsd_path;
> +        }
> +        /* if a physical device experienced an error while being opened */
> +        if (strncmp(filename, "/dev/", 5) == 0) {
> +            print_unmounting_directions(filename);
> +            return -1;

Please use a negative errno number instead of -1.

But more importantly: What happened with the return that you removed
above? Even in the non-Apple case, we don't return an error now, but
continue in this function. This looks certainly wrong. Did you intend to
move it to below the #ifdef block?

Kevin

> +        }
> +#endif /* defined(__APPLE__) && defined(__MACH__) */
>      }
>  
>      /* Since this does ioctl the device must be already opened */
> -- 
> 1.7.5.4
> 
>