From patchwork Tue Dec 6 19:28:51 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Bligh X-Patchwork-Id: 129803 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6261E1007D5 for ; Wed, 7 Dec 2011 06:29:12 +1100 (EST) Received: from localhost ([::1]:46991 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RY0hR-0007kF-OQ for incoming@patchwork.ozlabs.org; Tue, 06 Dec 2011 14:29:09 -0500 Received: from eggs.gnu.org ([140.186.70.92]:52326) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RY0hK-0007jc-Pw for qemu-devel@nongnu.org; Tue, 06 Dec 2011 14:29:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RY0hJ-0000yl-Bg for qemu-devel@nongnu.org; Tue, 06 Dec 2011 14:29:02 -0500 Received: from mail.avalus.com ([89.16.176.221]:49290) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RY0hJ-0000ye-1w for qemu-devel@nongnu.org; Tue, 06 Dec 2011 14:29:01 -0500 Received: from [192.168.50.14] (lemondeh-adsl.demon.co.uk [83.105.105.209]) by mail.avalus.com (Postfix) with ESMTPSA id 3A138C560FE; Tue, 6 Dec 2011 19:28:57 +0000 (GMT) Date: Tue, 06 Dec 2011 19:28:51 +0000 From: Alex Bligh To: Christoph Hellwig Message-ID: <5B50D590BB9CDD144E45B0A4@nimrod.local> In-Reply-To: <20111206184226.GA18058@lst.de> References: <958CDBC95A3252B592B5E3C9@nimrod.local> <20111206184226.GA18058@lst.de> X-Mailer: Mulberry/4.0.8 (Mac OS X) MIME-Version: 1.0 Content-Disposition: inline X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 89.16.176.221 Cc: qemu-devel@nongnu.org, Alex Bligh Subject: Re: [Qemu-devel] RFC: raw device support for block device targets X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: Alex Bligh List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org --On 6 December 2011 19:42:26 +0100 Christoph Hellwig wrote: >> Is it therefore worth skipping the ftruncate() if the block device >> is large enough, and at least attempting to proceed further? Something >> like the following (not-even compile tested) patch? > > It probably should share the code with raw_truncate. That is factor > a new raw_truncate_common that is equivalent to raw_truncate but > takes a file descriptor instead of a block driver state and then use > it for both raw_truncate and raw_create. You are of course correct, not least because raw_getlength() illustrates that not every OS supports the method I suggested for length checking. However, this makes things much more intrusive. Essentially you need a raw_ftruncate that takes an fd, and a raw_fgetlength that takes an fd, and that means modifying each of the OS implementations for the latter. I've done them (blind) save for the reference to cdrom_reopen(), sometimes by deleting fd_open(). I'm not quite sure why that's needed there; perhaps doing it in raw_truncate and raw_getlength would be sufficient. Again, this is compile tested only. diff --git a/block/raw-posix.c b/block/raw-posix.c index 2ee5d69..e67efa9 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -138,6 +138,7 @@ typedef struct BDRVRawState { static int fd_open(BlockDriverState *bs); static int64_t raw_getlength(BlockDriverState *bs); +static int64_t raw_fgetlength(int fd); #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) static int cdrom_reopen(BlockDriverState *bs); @@ -379,21 +380,20 @@ static void raw_close(BlockDriverState *bs) } } -static int raw_truncate(BlockDriverState *bs, int64_t offset) +static int raw_ftruncate(int fd, int64_t offset) { - BDRVRawState *s = bs->opaque; struct stat st; - if (fstat(s->fd, &st)) { + if (fstat(fd, &st)) { return -errno; } if (S_ISREG(st.st_mode)) { - if (ftruncate(s->fd, offset) < 0) { + if (ftruncate(fd, offset) < 0) { return -errno; } } else if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) { - if (offset > raw_getlength(bs)) { + if (offset > raw_fgetlength(fd)) { return -EINVAL; } } else { @@ -403,11 +403,16 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset) return 0; } -#ifdef __OpenBSD__ -static int64_t raw_getlength(BlockDriverState *bs) + +static int raw_truncate(BlockDriverState *bs, int64_t offset) { BDRVRawState *s = bs->opaque; - int fd = s->fd; + return raw_ftruncate(s->fd, offset); +} + +#ifdef __OpenBSD__ +static int64_t raw_fgetlength(int fd) +{ struct stat st; if (fstat(fd, &st)) @@ -423,10 +428,8 @@ static int64_t raw_getlength(BlockDriverState *bs) return st.st_size; } #elif defined(__NetBSD__) -static int64_t raw_getlength(BlockDriverState *bs) +static int64_t raw_fgetlength(int fd) { - BDRVRawState *s = bs->opaque; - int fd = s->fd; struct stat st; if (fstat(fd, &st)) @@ -448,21 +451,15 @@ static int64_t raw_getlength(BlockDriverState *bs) return st.st_size; } #elif defined(__sun__) -static int64_t raw_getlength(BlockDriverState *bs) +static int64_t raw_fgetlength(int fd) { - BDRVRawState *s = bs->opaque; struct dk_minfo minfo; int ret; - ret = fd_open(bs); - if (ret < 0) { - return ret; - } - /* * Use the DKIOCGMEDIAINFO ioctl to read the size. */ - ret = ioctl(s->fd, DKIOCGMEDIAINFO, &minfo); + ret = ioctl(fd, DKIOCGMEDIAINFO, &minfo); if (ret != -1) { return minfo.dki_lbsize * minfo.dki_capacity; } @@ -471,13 +468,11 @@ static int64_t raw_getlength(BlockDriverState *bs) * There are reports that lseek on some devices fails, but * irc discussion said that contingency on contingency was overkill. */ - return lseek(s->fd, 0, SEEK_END); + return lseek(fd, 0, SEEK_END); } #elif defined(CONFIG_BSD) -static int64_t raw_getlength(BlockDriverState *bs) +static int64_t raw_fgetlength(int fd) { - BDRVRawState *s = bs->opaque; - int fd = s->fd; int64_t size; struct stat sb; #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__) @@ -485,10 +480,6 @@ static int64_t raw_getlength(BlockDriverState *bs) #endif int ret; - ret = fd_open(bs); - if (ret < 0) - return ret; - #if defined (__FreeBSD__) || defined(__FreeBSD_kernel__) again: #endif @@ -529,19 +520,17 @@ again: return size; } #else +static int64_t raw_fgetlength(int fd) +{ + return lseek(fd, 0, SEEK_END); +} +#endif + static int64_t raw_getlength(BlockDriverState *bs) { BDRVRawState *s = bs->opaque; - int ret; - - ret = fd_open(bs); - if (ret < 0) { - return ret; - } - - return lseek(s->fd, 0, SEEK_END); + return raw_fgetlength(s->fd); } -#endif static int64_t raw_get_allocated_file_size(BlockDriverState *bs) { @@ -573,7 +562,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) if (fd < 0) { result = -errno; } else { - if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) { + if (raw_ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) { result = -errno; } if (close(fd) != 0) {