[Qemu-devel] [PATCH] add bdrv->create function for host_device

[Qemu-devel] [PATCH] add bdrv->create function for host_device

[Nolan]

"qemu-img convert" tries to create the destination image by calling it's
->create function.  host_devices do not currently have one, since host
devices are not created like normal images are.  A reasonable analog of
create for host devices is verifying that they have the appropriate
properties.

This patch adds that verification.

It also fixes a typo that caused qemu-img to print the wrong device type
in the error message informing the user that the destination device did
not support creation.

Signed-off-by: Nolan Leake <nolan <at> sigbus.net>

Index: block-raw-posix.c
===================================================================
--- block-raw-posix.c	(revision 6718)
+++ block-raw-posix.c	(working copy)
@@ -1132,6 +1132,32 @@
 
     return ioctl(s->fd, req, buf);
 }
+
+static int hdev_create(const char *filename, int64_t total_size,
+                       const char *backing_file, int flags)
+{
+    int fd;
+    int ret = 0;
+    struct stat stat_buf;
+
+    if (flags || backing_file)
+        return -ENOTSUP;
+
+    fd = open(filename, O_WRONLY | O_BINARY);
+    if (fd < 0)
+        return -EIO;
+
+    if (fstat(fd, &stat_buf) < 0)
+        ret = -EIO;
+    else if (!S_ISBLK(stat_buf.st_mode))
+        ret = -EIO;
+    else if (lseek(fd, 0, SEEK_END) < total_size * 512)
+        ret = -ENOSPC;
+
+    close(fd);
+    return ret;
+}
+
 #else
 
 static int fd_open(BlockDriverState *bs)
@@ -1163,6 +1189,12 @@
 {
     return -ENOTSUP;
 }
+
+static int hdev_create(const char *filename, int64_t total_size,
+                       const char *backing_file, int flags)
+{
+    return -ENOTSUP;
+}
 #endif /* !linux */
 
 BlockDriver bdrv_host_device = {
@@ -1173,7 +1205,7 @@
     NULL,
     NULL,
     raw_close,
-    NULL,
+    hdev_create,
     raw_flush,
 
 #ifdef CONFIG_AIO
Index: qemu-img.c
===================================================================
--- qemu-img.c	(revision 6718)
+++ qemu-img.c	(working copy)
@@ -477,7 +477,7 @@
     ret = bdrv_create(drv, out_filename, total_sectors, out_baseimg, flags);
     if (ret < 0) {
         if (ret == -ENOTSUP) {
-            error("Formatting not supported for file format '%s'", fmt);
+            error("Formatting not supported for file format '%s'", out_fmt);
         } else {
             error("Error while formatting '%s'", out_filename);
         }

Re: [Qemu-devel] [PATCH] add bdrv->create function for host_device

[Anthony Liguori]

Nolan wrote:
> "qemu-img convert" tries to create the destination image by calling it's
> ->create function.  host_devices do not currently have one, since host
> devices are not created like normal images are.  A reasonable analog of
> create for host devices is verifying that they have the appropriate
> properties.
>
> This patch adds that verification.
>
> It also fixes a typo that caused qemu-img to print the wrong device type
> in the error message informing the user that the destination device did
> not support creation.
>
> Signed-off-by: Nolan Leake <nolan <at> sigbus.net>
>
> Index: block-raw-posix.c
> ===================================================================
> --- block-raw-posix.c	(revision 6718)
> +++ block-raw-posix.c	(working copy)
> @@ -1132,6 +1132,32 @@
>  
>      return ioctl(s->fd, req, buf);
>  }
> +
> +static int hdev_create(const char *filename, int64_t total_size,
> +                       const char *backing_file, int flags)
> +{
> +    int fd;
> +    int ret = 0;
> +    struct stat stat_buf;
> +
> +    if (flags || backing_file)
> +        return -ENOTSUP;
> +
> +    fd = open(filename, O_WRONLY | O_BINARY);
> +    if (fd < 0)
> +        return -EIO;
> +
> +    if (fstat(fd, &stat_buf) < 0)
> +        ret = -EIO;
> +    else if (!S_ISBLK(stat_buf.st_mode))
> +        ret = -EIO;
> +    else if (lseek(fd, 0, SEEK_END) < total_size * 512)
> +        ret = -ENOSPC;
> +
> +    close(fd);
> +    return ret;
> +}
>   

I'm not quite sure how, but I think you need to have BDRV_O_FILE as a 
special case.  I think probably the easiest thing to do is to bail out 
if BDRV_O_FILE is set since a physical device cannot be growable.

Regards,

Anthony Liguori

>
>   

Re: [Qemu-devel] [PATCH] add bdrv->create function for host_device

[Nolan]

Anthony Liguori writes:
> I'm not quite sure how, but I think you need to have BDRV_O_FILE as a 
> special case.  I think probably the easiest thing to do is to bail
> out if BDRV_O_FILE is set since a physical device cannot be growable.

If I understand you correctly, my original patch already does that:

    if (flags || backing_file)
        return -ENOTSUP;

Though I can't say that I'm entirely sure I understand what it means to
pass BDRV_O_FILE to bdrv_create().

The consensus of other block_*.c implementations is to ignore flags
completely or bail if flags != 0, unless they have their own bdrv
specific flags (eg BLOCK_FLAG_COMPAT6 for vmdk, and BLOCK_FLAG_ENCRYPT
for qcow/qcow2) which they specifically honor and ignore any other
flags.