Re: [Qemu-devel] [PATCH 1/2] Introducing qcow2 extensions + keep backing file format (v4)

[Uri Lublin]

Anthony Liguori wrote:
> Uri Lublin wrote:
> > Qcow2 extensions are build of magic (id) len (in bytes) and data.
> > They reside between the end of the header and the filename.
> >
> > We can keep the backing file format in a such a qcow2 extension, to
> > 1. Provide a way to know the backing file format without probing
> >    it (setting the format at creation time).
> > 2. Enable using qcow2 format over host block devices.
> >    (only if the user specifically asks for it, by providing the format
> >    at creation time).
> >
> > I've added bdrv_create2 and drv->bdrv_create2 (implemented only
> > by block-qcow2 currently) to pass the backing-format to create.
> >
> > Based on a work done by Shahar Frank.
> >
> > Also fixes a security flaw found by Daniel P. Berrange on [1]
> > which summarizes: "Autoprobing: just say no."
> >
> > [1] http://lists.gnu.org/archive/html/qemu-devel/2008-12/msg01083.html
> >
> > Signed-off-by: Uri Lublin <address@hidden>
> >   
>
> This made it through my regression testing but...
>
> >  int bdrv_create(BlockDriver *drv,
> >                  const char *filename, int64_t size_in_sectors,
> >                  const char *backing_file, int flags)
> > @@ -348,6 +362,9 @@ int bdrv_open2(BlockDriverState *bs, const char 
> > *filename, int flags,
> >  
> >          /* if there is a backing file, use it */
> >          bs1 = bdrv_new("");
> > +/*         if (drv) */
> > +/*             pstrcpy(bs1->backing_format, 
> > sizeof(bs->backing_format), */
> > +/*                     drv->format_name); */
> >   
>
> I don't know if I missed this before or it was added since v3 but I'm 
> curious why this is here and why it's commented out.

It's an old code, which should have been deleted.

The purpose was to make sure the temporary-snapshot-file backing-file-format
is known.
This is now done with bdrv_create2, that I've added.

You can delete it yourself or apply the attached patch (or squash patches)
Alternatively I can send it to you again as a single patch (v5)

Also I've noticed that a small fix is needed for BDRV_O_SNAPSHOT. e.g we should 
use bdrv_open2 (so we would not probe the image if the format is known), so I'm 
attaching a patch to fix that too. Again if you want I can resend it as a single 
patch (squashed with v4) or you can apply(squash) this patch yourself on top of 
my previous patches.

Thanks,
    Uri.

>From 5003cc362793c10f276ab186026b3f25b6b3ac9e Mon Sep 17 00:00:00 2001
From: Uri Lublin <address@hidden>
Date: Sun, 1 Mar 2009 12:43:45 +0200
Subject: [PATCH] block.c: remove dead commented out code

Signed-off-by: Uri Lublin <address@hidden>
---
 block.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 31d6d5b..1626e0c 100644
--- a/block.c
+++ b/block.c
@@ -362,9 +362,6 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
 
         /* if there is a backing file, use it */
         bs1 = bdrv_new("");
-/*         if (drv) */
-/*             pstrcpy(bs1->backing_format, sizeof(bs->backing_format), */
-/*                     drv->format_name); */
         if (!bs1) {
             return -ENOMEM;
         }
-- 
1.6.0.6

>From bc79eae6c3226d21bd3fcbc370a86d7b24657aad Mon Sep 17 00:00:00 2001
From: Uri Lublin <address@hidden>
Date: Sun, 1 Mar 2009 12:44:38 +0200
Subject: [PATCH] block.c: If image format is known, use it for BDRV_O_SNAPSHOT 
flag as well

Otherwise we'd
1. Unnecessarily probe the image to find out its format
2  Get it wrong for host-devices and non-raw images, which
   may affect created image size.

Also, force the temporary snapshot image to be of format bdrv_qcow2

Signed-off-by: Uri Lublin <address@hidden>
---
 block.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index 1626e0c..0939cad 100644
--- a/block.c
+++ b/block.c
@@ -365,7 +365,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
         if (!bs1) {
             return -ENOMEM;
         }
-        if (bdrv_open(bs1, filename, 0) < 0) {
+        if (bdrv_open2(bs1, filename, 0, drv) < 0) {
             bdrv_delete(bs1);
             return -1;
         }
@@ -392,6 +392,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
             return -1;
         }
         filename = tmp_filename;
+        drv = &bdrv_qcow2;
         bs->is_temporary = 1;
     }
 
-- 
1.6.0.6