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

[Peter Krempa]

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 ...

> ---
>  src/storage/storage_backend.c                       |  2 +-
>  src/storage/storage_driver.c                        |  3 ++-
>  .../qcow2-nocapacity-convert-prealloc.argv          |  4 ++++
>  tests/storagevolxml2argvtest.c                      |  9 ++++++++-
>  tests/storagevolxml2xmlin/vol-qcow2-nocapacity.xml  | 19 +++++++++++++++++++
>  tests/storagevolxml2xmlout/vol-qcow2-nocapacity.xml | 21 +++++++++++++++++++++
>  tests/storagevolxml2xmltest.c                       |  2 ++
>  7 files changed, 57 insertions(+), 3 deletions(-)
>  create mode 100644 tests/storagevolxml2argvdata/qcow2-nocapacity-convert-prealloc.argv
>  create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-nocapacity.xml
>  create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-nocapacity.xml
> 
> 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.

>  
> 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.

Peter
-------------- 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/1db448ef/attachment-0001.sig>