From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43414) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQeYv-0001Lt-Qy for qemu-devel@nongnu.org; Tue, 02 Feb 2016 12:16:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQeYp-0003Zp-VH for qemu-devel@nongnu.org; Tue, 02 Feb 2016 12:16:21 -0500 Date: Tue, 2 Feb 2016 17:16:07 +0000 From: "Daniel P. Berrange" Message-ID: <20160202171606.GM18461@redhat.com> References: <1CF37786-9AAA-4E5E-B571-DE20E7A463EB@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1CF37786-9AAA-4E5E-B571-DE20E7A463EB@gmail.com> Subject: Re: [Qemu-devel] ping: [PATCH v13] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Programmingkid Cc: Kevin Wolf , qemu-devel qemu-devel , Qemu-block On Tue, Feb 02, 2016 at 12:08:31PM -0500, Programmingkid wrote: > https://patchwork.ozlabs.org/patch/570128/ > > 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 > > --- > Changed filename variable to a character array. > Changed how filename was set to bsd_path's value by using snprintf(). Whats the rationale here ? Using pre-allocated fixed length arrays is pretty bad practice in general, but especially so for filenames > @@ -2119,34 +2173,59 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags, > int ret; > > #if defined(__APPLE__) && defined(__MACH__) > - const char *filename = qdict_get_str(options, "filename"); [snip] > + char filename[MAXPATHLEN]; > + bool error_occurred = false; > + snprintf(filename, MAXPATHLEN, "%s", qdict_get_str(options, "filename")); This is a step backwards compared to the original code IMHO. There's no good reason to use a stack allocated fixed array here - the code that follows would be quite happy with the original 'const char *'. > + /* If using a real cdrom */ > + if (strcmp(filename, "/dev/cdrom") == 0) { > + char bsd_path[MAXPATHLEN]; > + char *mediaType = NULL; > + kern_return_t ret_val; > + io_iterator_t mediaIterator = 0; > + > + mediaType = FindEjectableOpticalMedia(&mediaIterator); > + if (mediaType == NULL) { > + error_setg(errp, "Please make sure your CD/DVD is in the optical" > + " drive"); > + error_occurred = true; > + goto hdev_open_Mac_error; > + } > + > + ret_val = GetBSDPath(mediaIterator, bsd_path, sizeof(bsd_path), flags); > + if (ret_val != KERN_SUCCESS) { > + error_setg(errp, "Could not get BSD path for optical drive"); > + error_occurred = true; > + goto hdev_open_Mac_error; > + } > + > + /* If a real optical drive was not found */ > + if (bsd_path[0] == '\0') { > + error_setg(errp, "Failed to obtain bsd path for optical drive"); > + error_occurred = true; > + goto hdev_open_Mac_error; > + } > + > + /* If using a cdrom disc and finding a partition on the disc failed */ > + if (strncmp(mediaType, kIOCDMediaClass, 9) == 0 && > + setup_cdrom(bsd_path, errp) == false) { > + print_unmounting_directions(bsd_path); > + error_occurred = true; > + goto hdev_open_Mac_error; > } > > - if ( mediaIterator ) > - IOObjectRelease( mediaIterator ); > + snprintf(filename, MAXPATHLEN, "%s", bsd_path); > + qdict_put(options, "filename", qstring_from_str(filename)); > + > +hdev_open_Mac_error: > + g_free(mediaType); > + if (mediaIterator) { > + IOObjectRelease(mediaIterator); > + } > + if (error_occurred) { > + return -1; > + } > } > -#endif > +#endif /* defined(__APPLE__) && defined(__MACH__) */ Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|