Patchwork RFC: raw device support for block device targets

login
register
mail settings
Submitter Alex Bligh
Date Dec. 6, 2011, 4:20 p.m.
Message ID <958CDBC95A3252B592B5E3C9@nimrod.local>
Download mbox | patch
Permalink /patch/129726/
State New
Headers show

Comments

Alex Bligh - Dec. 6, 2011, 4:20 p.m.
qemu-img convert appears to support block devices as input, but not
as output. That is irritating, as when using qemu-img convert to
convert qcow to raw on a block partition, an intermediate file has
to be used, which slows things down and pointlessly uses disk space.

The problem is that ftruncate() is being called on the output file
in order ensure it is sufficiently large, and this fails on
block devices.

I appreciate there may be other calls that fail depending on the
input file format, but these will presumably be error checked
at the time.

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?
Christoph Hellwig - Dec. 6, 2011, 6:42 p.m.
On Tue, Dec 06, 2011 at 04:20:56PM +0000, Alex Bligh wrote:
> qemu-img convert appears to support block devices as input, but not
> as output. That is irritating, as when using qemu-img convert to
> convert qcow to raw on a block partition, an intermediate file has
> to be used, which slows things down and pointlessly uses disk space.
>
> The problem is that ftruncate() is being called on the output file
> in order ensure it is sufficiently large, and this fails on
> block devices.
>
> I appreciate there may be other calls that fail depending on the
> input file format, but these will presumably be error checked
> at the time.
>
> 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.
Kevin Wolf - Dec. 8, 2011, 12:40 p.m.
Am 06.12.2011 17:20, schrieb Alex Bligh:
> qemu-img convert appears to support block devices as input, but not
> as output. That is irritating, as when using qemu-img convert to
> convert qcow to raw on a block partition, an intermediate file has
> to be used, which slows things down and pointlessly uses disk space.
> 
> The problem is that ftruncate() is being called on the output file
> in order ensure it is sufficiently large, and this fails on
> block devices.
> 
> I appreciate there may be other calls that fail depending on the
> input file format, but these will presumably be error checked
> at the time.
> 
> 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?

Creating an image on a block device shouldn't even call raw_create(),
but only hdev_create(), which doesn't try to truncate the device, but
just uses lseek to make sure that it's large enough.

Which qemu version are you using and what's your command line?

Kevin
Alex Bligh - Dec. 11, 2011, 10:45 a.m.
--On 8 December 2011 13:40:57 +0100 Kevin Wolf <kwolf@redhat.com> wrote:

>> qemu-img convert appears to support block devices as input, but not
>> as output. That is irritating, as when using qemu-img convert to
>> convert qcow to raw on a block partition, an intermediate file has
>> to be used, which slows things down and pointlessly uses disk space.
>>
>> The problem is that ftruncate() is being called on the output file
>> in order ensure it is sufficiently large, and this fails on
>> block devices.
...
>
> Creating an image on a block device shouldn't even call raw_create(),
> but only hdev_create(), which doesn't try to truncate the device, but
> just uses lseek to make sure that it's large enough.
>
> Which qemu version are you using and what's your command line?

I was testing on:

amb@alex-test:~$ qemu-img --version
qemu-img version 0.12.3, Copyright (c) 2004-2008 Fabrice Bellard

though the patch was against current trunk.

Command line simply:

qemu-img convert -O raw test.qcow /dev/xyz

Fails on ftruncate() as verified with strace.

I must admit I only 'tested' on trunk by reading the source.
Kevin Wolf - Dec. 12, 2011, 9:48 a.m.
Am 11.12.2011 11:45, schrieb Alex Bligh:
> 
> 
> --On 8 December 2011 13:40:57 +0100 Kevin Wolf <kwolf@redhat.com> wrote:
> 
>>> qemu-img convert appears to support block devices as input, but not
>>> as output. That is irritating, as when using qemu-img convert to
>>> convert qcow to raw on a block partition, an intermediate file has
>>> to be used, which slows things down and pointlessly uses disk space.
>>>
>>> The problem is that ftruncate() is being called on the output file
>>> in order ensure it is sufficiently large, and this fails on
>>> block devices.
> ...
>>
>> Creating an image on a block device shouldn't even call raw_create(),
>> but only hdev_create(), which doesn't try to truncate the device, but
>> just uses lseek to make sure that it's large enough.
>>
>> Which qemu version are you using and what's your command line?
> 
> I was testing on:
> 
> amb@alex-test:~$ qemu-img --version
> qemu-img version 0.12.3, Copyright (c) 2004-2008 Fabrice Bellard

That's the problem. It should work since 0.13.

Kevin
Alex Bligh - Dec. 12, 2011, 10:32 a.m.
--On 12 December 2011 10:48:43 +0100 Kevin Wolf <kwolf@redhat.com> wrote:

>> I was testing on:
>>
>> amb@alex-test:~$ qemu-img --version
>> qemu-img version 0.12.3, Copyright (c) 2004-2008 Fabrice Bellard
>
> That's the problem. It should work since 0.13.

Thanks

Patch

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 2ee5d69..be9a371 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -573,9 +573,29 @@  static int raw_create(const char *filename, 
QEMUOptionParameter *options)
     if (fd < 0) {
         result = -errno;
     } else {
+
+        struct stat sb;
+
+        if (-1 == fstat(dest, &sb)) {
+            result = -errno;
+            goto close;
+        }
+
+        /* block devices do not support truncate. If the device is large
+           enough, then it will do */
+
+        if (S_ISBLK(sb.st_mode)) {
+            /* divide to prevent overflow */
+            if (sb.st_size / BDRV_SECTOR_SIZE < total_size)
+                result = -ENOSPC;
+            goto close;
+        }
+
         if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) {
             result = -errno;
         }
+
+    close:
         if (close(fd) != 0) {
             result = -errno;