On Tue, Mar 01, 2016 at 04:16:39PM +0100, Kevin Wolf wrote: > 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 <programmingk...@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 > > > > >
In addition to the above, the patch is still using quoted-printable encoding. With Mutt at least, this makes applying patches cumbersome. Whether or not this is an issue for Kevin's workflow, I don't know. But if you are re-spinning a new patch, I recommend taking care of that (git send-email will do the right thing). Jeff