From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOUsJ-0002ia-5g for qemu-devel@nongnu.org; Mon, 23 May 2011 09:08:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QOUsH-0008H6-48 for qemu-devel@nongnu.org; Mon, 23 May 2011 09:08:47 -0400 Received: from va3ehsobe005.messaging.microsoft.com ([216.32.180.31]:21242 helo=VA3EHSOBE005.bigfish.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QOUsG-0008Gu-Vn for qemu-devel@nongnu.org; Mon, 23 May 2011 09:08:45 -0400 Message-ID: <4DDA5287.4050407@amd.com> Date: Mon, 23 May 2011 14:26:47 +0200 From: Christoph Egger MIME-Version: 1.0 References: <4DDA3153.4070003@amd.com> <4DDA383F.7080908@amd.com> <20110523110615.GA15957@lst.de> In-Reply-To: <20110523110615.GA15957@lst.de> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block/raw-posix: get right size of partition size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christoph Hellwig Cc: "qemu-devel@nongnu.org" On 05/23/11 13:06, Christoph Hellwig wrote: > On Mon, May 23, 2011 at 12:34:39PM +0200, Christoph Egger wrote: >> >> This does 2 things: >> - use the correct way to get the size of a disk device or partition >> (from haad@NetBSD.org) >> - if given a block device, use the character device instead. >> (from bouyer@NetBSD.org) > > Please split that into two independent patches. ok. > >> + if (S_ISLNK(sb.st_mode)) { >> + fprintf(stderr, "%s: symbolic link not supported\n", filename); >> + return -EINVAL; >> + } > > Why not, it's a pretty clear regression from current code, and will > break various Linux setups where there are lots of symlinks under /dev, > as well as users using symlinks for their image files. there's no easy way to find the real target of a symlink, to find the corresponding character device (this could be refinded, because this restriction only applies if the target is a block device). > >> +#if defined(__NetBSD__) >> + if (S_ISBLK(sb.st_mode)) { >> + 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); >> + } >> +#endif > > Please split this out into a helper, which has a no-op version > for other operating systems. It probably should also be enabled > for other operating systems having char and block nodes for disk > devices. That's at least OpenBSD and Solaris, not sure about > Darwin. Yes, but don't have a test machine right now. >> -#ifdef __OpenBSD__ >> +#if defined(__OpenBSD__) || defined(__NetBSD__) >> static int64_t raw_getlength(BlockDriverState *bs) >> { >> BDRVRawState *s = bs->opaque; >> @@ -613,12 +645,22 @@ static int64_t raw_getlength(BlockDriverState *bs) >> if (fstat(fd,&st)) >> return -1; >> if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) { >> - struct disklabel dl; >> +#if defined(__NetBSD__) >> + struct dkwedge_info dkw; >> >> - if (ioctl(fd, DIOCGDINFO,&dl)) >> - return -1; >> - return (uint64_t)dl.d_secsize * >> - dl.d_partitions[DISKPART(st.st_rdev)].p_size; >> + if (ioctl(fd, DIOCGWEDGEINFO,&dkw) != -1) { >> + return dkw.dkw_size * 512; >> + } else { >> +#endif > > Please provide a completely separate raw_getlength for NetBSD instead > of creating this ifdef mess for almost no shared code. Ok. -- ---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