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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list