From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:49851) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOnIJ-0006QU-ER for qemu-devel@nongnu.org; Tue, 24 May 2011 04:48:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QOnII-0004TD-6t for qemu-devel@nongnu.org; Tue, 24 May 2011 04:48:51 -0400 Received: from db3ehsobe002.messaging.microsoft.com ([213.199.154.140]:8783 helo=DB3EHSOBE002.bigfish.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOnIH-0004T8-Sk for qemu-devel@nongnu.org; Tue, 24 May 2011 04:48:50 -0400 Message-ID: <4DDB6DFC.2090801@amd.com> Date: Tue, 24 May 2011 10:36:12 +0200 From: Christoph Egger MIME-Version: 1.0 References: <4DDA5442.30801@amd.com> <4DDA6B2A.6000900@redhat.com> In-Reply-To: <4DDA6B2A.6000900@redhat.com> Content-Type: text/plain; charset="ISO-8859-15"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block/raw-posix: use a character device if a block device is given List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: "qemu-devel@nongnu.org" On 05/23/11 16:11, Kevin Wolf wrote: > Am 23.05.2011 14:34, schrieb Christoph Egger: >> >> if given a block device, use the character device instead. >> >> From: Manuel Bouyer >> Signed-off-by: Christoph Egger > > A useful commit message would explain why you're doing that. How about this: On NetBSD, the PV backend has to use the block device; but a userland process is better with the character device interface. In addition, a block device can't be opened twice; if the backend opens it qemu can't and vice-versa. >> >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index 6b72470..d05f373 100644 >> --- a/block/raw-posix.c >> +++ b/block/raw-posix.c >> @@ -136,11 +143,45 @@ static int64_t raw_getlength(BlockDriverState *bs); >> static int cdrom_reopen(BlockDriverState *bs); >> #endif >> >> +#if defined(__NetBSD__) >> +static const char *raw_get_rawdevice(const char *filename) >> +{ >> + static char namebuf[PATH_MAX]; >> + const char *dp = strrchr(filename, '/'); >> + >> + if (dp == NULL) { >> + snprintf(namebuf, PATH_MAX, "r%s", filename); >> + } else { >> + snprintf(namebuf, PATH_MAX, "%.*s/r%s", >> + (int)(dp - filename), filename, dp + 1); >> + } >> + fprintf(stderr, "%s is a block device", filename); >> + filename = namebuf; >> + fprintf(stderr, ", using %s\n", filename); > > Not sure if fprintf is a good idea here, but ok. I want to make it clear what file the qemu process has been using. this is what log files are for, isn't it ? >> + >> + return filename; >> +} >> +#else >> +static const char *raw_get_rawdevice(const char *filename) >> +{ >> + return filename; >> +} >> +#endif >> + >> static int raw_open_common(BlockDriverState *bs, const char *filename, >> int bdrv_flags, int open_flags) >> { >> BDRVRawState *s = bs->opaque; >> int fd, ret; >> + struct stat sb; >> + >> + if (lstat(filename,&sb)< 0) { >> + fprintf(stderr, "%s: stat failed: %s\n", filename, >> strerror(errno)); > > The patch is corrupted by this line wrap. this line fits into 80 columns. must be a mail client or a ms exchange problem. > > Please remove the fprintf, the callers are responsible for sending an > error message to the right destination. >> + return -errno; >> + } >> + >> + if (S_ISBLK(sb.st_mode)) >> + filename = raw_get_rawdevice(filename); > > The qemu coding style requires braces. > > Also, I agree with Christoph that the lstat/S_ISBLK should be moved into > the NetBSD specific code. Ok. Christoph -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632