From patchwork Tue Dec  6 19:28:51 2011
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: RFC: raw device support for block device targets
Date: Tue, 06 Dec 2011 09:28:51 -0000
From: Alex Bligh <alex@alex.org.uk>
X-Patchwork-Id: 129803
Message-Id: <5B50D590BB9CDD144E45B0A4@nimrod.local>
To: Christoph Hellwig <hch@lst.de>
Cc: qemu-devel@nongnu.org, Alex Bligh <alex@alex.org.uk>

--On 6 December 2011 19:42:26 +0100 Christoph Hellwig <hch@lst.de> 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) {