Patchwork qcow2: Allow qcow2 disk images with size zero

login
register
mail settings
Submitter Stefan Weil
Date Oct. 26, 2009, 3:11 p.m.
Message ID <1256569876-13038-1-git-send-email-weil@mail.berlios.de>
Download mbox | patch
Permalink /patch/36927/
State New
Headers show

Comments

Stefan Weil - Oct. 26, 2009, 3:11 p.m.
Images with disk size 0 may be used for
VM snapshots, but not to save normal block data.

It is possible to create such images using
qemu-img, but opening them later fails.

So even "qemu-img info image.qcow2" is not
possible for an image created with
"qemu-img create -f qcow2 image.qcow2 0".

This is fixed here.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 block/qcow2-cluster.c |    3 +++
 block/qcow2.c         |   19 ++++++++++---------
 2 files changed, 13 insertions(+), 9 deletions(-)
Anthony Liguori - Oct. 27, 2009, 2:11 p.m.
Stefan Weil wrote:
> Images with disk size 0 may be used for
> VM snapshots, but not to save normal block data.
>
> It is possible to create such images using
> qemu-img, but opening them later fails.
>
> So even "qemu-img info image.qcow2" is not
> possible for an image created with
> "qemu-img create -f qcow2 image.qcow2 0".
>
> This is fixed here.
>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>   

What do you think Kevin?

Regards,

Anthony Liguori
Kevin Wolf - Oct. 27, 2009, 4:44 p.m.
Am 27.10.2009 15:11, schrieb Anthony Liguori:
> Stefan Weil wrote:
>> Images with disk size 0 may be used for
>> VM snapshots, but not to save normal block data.
>>
>> It is possible to create such images using
>> qemu-img, but opening them later fails.
>>
>> So even "qemu-img info image.qcow2" is not
>> possible for an image created with
>> "qemu-img create -f qcow2 image.qcow2 0".
>>
>> This is fixed here.
>>
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>>   
> 
> What do you think Kevin?

That with malloc instead of qemu_malloc the patch would have been a bit
smaller. ;-) I think it can make sense to have such images in some cases
(although a 64k image isn't much worse) and I don't see any obvious
problem with the patch. Not sure how guests like 0 byte disks, but I
assume Stefan has tested that.

I'm just wondering how Stefan did create his test image - with current
qemu-img I can't create such images, so I needed to patch it first. For
this one I'll send a fix.

Kevin
Stefan Weil - Oct. 27, 2009, 5:17 p.m.
Kevin Wolf schrieb:
> Am 27.10.2009 15:11, schrieb Anthony Liguori:
>   
>> Stefan Weil wrote:
>>     
>>> Images with disk size 0 may be used for
>>> VM snapshots, but not to save normal block data.
>>>
>>> It is possible to create such images using
>>> qemu-img, but opening them later fails.
>>>
>>> So even "qemu-img info image.qcow2" is not
>>> possible for an image created with
>>> "qemu-img create -f qcow2 image.qcow2 0".
>>>
>>> This is fixed here.
>>>
>>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>>>   
>>>       
>> What do you think Kevin?
>>     
>
> That with malloc instead of qemu_malloc the patch would have been a bit
> smaller. ;-) I think it can make sense to have such images in some cases
> (although a 64k image isn't much worse) and I don't see any obvious
> problem with the patch. Not sure how guests like 0 byte disks, but I
> assume Stefan has tested that.
>
> I'm just wondering how Stefan did create his test image - with current
> qemu-img I can't create such images, so I needed to patch it first. For
> this one I'll send a fix.
>
> Kevin
>   

Hi Kevin,

obviously, I called an old version of qemu-img to create the test image
(from debian's qemu package).

Regards
Stefan

Patch

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index a7de820..3cfcd50 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -39,6 +39,9 @@  int qcow2_grow_l1_table(BlockDriverState *bs, int min_size)
     new_l1_size = s->l1_size;
     if (min_size <= new_l1_size)
         return 0;
+    if (new_l1_size == 0) {
+        new_l1_size = 1;
+    }
     while (min_size > new_l1_size) {
         new_l1_size = (new_l1_size * 3 + 1) / 2;
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index 189fa73..f696125 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -168,8 +168,7 @@  static int qcow_open(BlockDriverState *bs, const char *filename, int flags)
 
     if (header.magic != QCOW_MAGIC || header.version != QCOW_VERSION)
         goto fail;
-    if (header.size <= 1 ||
-        header.cluster_bits < MIN_CLUSTER_BITS ||
+    if (header.cluster_bits < MIN_CLUSTER_BITS ||
         header.cluster_bits > MAX_CLUSTER_BITS)
         goto fail;
     if (header.crypt_method > QCOW_CRYPT_AES)
@@ -202,13 +201,15 @@  static int qcow_open(BlockDriverState *bs, const char *filename, int flags)
     if (s->l1_size < s->l1_vm_state_index)
         goto fail;
     s->l1_table_offset = header.l1_table_offset;
-    s->l1_table = qemu_mallocz(
-        align_offset(s->l1_size * sizeof(uint64_t), 512));
-    if (bdrv_pread(s->hd, s->l1_table_offset, s->l1_table, s->l1_size * sizeof(uint64_t)) !=
-        s->l1_size * sizeof(uint64_t))
-        goto fail;
-    for(i = 0;i < s->l1_size; i++) {
-        be64_to_cpus(&s->l1_table[i]);
+    if (s->l1_size > 0) {
+        s->l1_table = qemu_mallocz(
+            align_offset(s->l1_size * sizeof(uint64_t), 512));
+        if (bdrv_pread(s->hd, s->l1_table_offset, s->l1_table, s->l1_size * sizeof(uint64_t)) !=
+            s->l1_size * sizeof(uint64_t))
+            goto fail;
+        for(i = 0;i < s->l1_size; i++) {
+            be64_to_cpus(&s->l1_table[i]);
+        }
     }
     /* alloc L2 cache */
     s->l2_cache = qemu_malloc(s->l2_size * L2_CACHE_SIZE * sizeof(uint64_t));