[libvirt] [PATCH 4/9] Allow cloning volumes with no capacity specified

Ján Tomko jtomko at redhat.com
Wed Feb 25 12:33:34 UTC 2015


On Wed, Feb 25, 2015 at 10:05:35AM +0100, Peter Krempa wrote:
> On Thu, Feb 19, 2015 at 15:59:12 +0100, Ján Tomko wrote:
> > We can get the capacity from the input volume.
> 
> I had to trace the code to understand what this patch was about ...
> 

I have added a commit message:
    In virStorageVolCreateXML, add VIR_VOL_XML_PARSE_NO_CAPACITY
    to the call parsing the XML of the new volume to make the capacity
    optional.

    If the capacity is omitted, use the capacity of the old volume.
    We already do that for values that are less than the original
    volume capacity.

> > diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> > index a67d50c..dd33436 100644
> > --- a/src/storage/storage_backend.c
> > +++ b/src/storage/storage_backend.c
> > @@ -1055,7 +1055,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
> >      if (convert)
> >          virCommandAddArg(cmd, inputPath);
> >      virCommandAddArg(cmd, vol->target.path);
> > -    if (!convert)
> > +    if (!convert && size_arg)
> 
> Note that this change is on the "!convert" (create) path ... [1]
> 
> >          virCommandAddArgFormat(cmd, "%lluK", size_arg);
> >  
> >      return cmd;
> > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> > index bc16e87..409b486 100644
> > --- a/src/storage/storage_driver.c
> > +++ b/src/storage/storage_driver.c
> > @@ -1810,7 +1810,8 @@ storageVolCreateXMLFrom(virStoragePoolPtr obj,
> >          goto cleanup;
> >      }
> >  
> > -    newvol = virStorageVolDefParseString(pool->def, xmldesc, 0);
> > +    newvol = virStorageVolDefParseString(pool->def, xmldesc,
> > +                                         VIR_VOL_XML_PARSE_NO_CAPACITY);
> >      if (newvol == NULL)
> >          goto cleanup;
> 
> Few lines below this change there's the following hunk:
> 
>     /* Is there ever a valid case for this? */
>     if (newvol->target.capacity < origvol->target.capacity)
>         newvol->target.capacity = origvol->target.capacity;
> 
> Which sets the capacity to the capacity of the original volume. While
> this is probably semantically OK it might be worth tweaking the comment.
> 

tweaked the comment:
    /* Use the original volume's capacity in case the new capacity
     * is less than that, or it was omitted */

> >  
> > diff --git a/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
> > new file mode 100644
> > index 0000000..9073b1b
> > --- /dev/null
> > +++ b/tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
> > @@ -0,0 +1,4 @@
> > +qemu-img convert -f raw -O qcow2 \
> 
> [1] ... while the test you are adding is testing the convert path. I
> think that the hunk [1] belongs to a different patch as the size
> argument was added only on the "create" path anyways.
> 
> > +-o encryption=on,preallocation=metadata \
> > +/var/lib/libvirt/images/sparse.img \
> > +/var/lib/libvirt/images/OtherDemo.img
> 
> ACK after the release with hunk [1] removed.

and removed the unrelated hunk.

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150225/deacfe9d/attachment-0001.sig>


More information about the libvir-list mailing list