Re: [Qemu-devel] RFC: raw device support for block device targets

[Alex Bligh]

--On 6 December 2011 19:42:26 +0100 Christoph Hellwig <address@hidden> 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.

--
Alex Bligh

Signed-Off-By: Alex Bligh <address@hidden>

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) {