From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58735) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aam2b-0001m6-Vf for qemu-devel@nongnu.org; Tue, 01 Mar 2016 10:16:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aam2X-0001vf-Rb for qemu-devel@nongnu.org; Tue, 01 Mar 2016 10:16:49 -0500 Date: Tue, 1 Mar 2016 16:16:39 +0100 From: Kevin Wolf Message-ID: <20160301151639.GF4250@noname.str.redhat.com> References: <3BAC5050-04A8-44DA-B65F-424CFE5615DA@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3BAC5050-04A8-44DA-B65F-424CFE5615DA@gmail.com> Subject: Re: [Qemu-devel] ping [PATCH v14] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Programmingkid Cc: qemu-devel qemu-devel , Qemu-block 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 > > --- > 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 > >