[libvirt] [PATCH] storage: allow metadata preallocation when creating qcow2 images

[Daniel P. Berrange]

On Thu, Nov 08, 2012 at 04:24:51PM +0100, Ján Tomko wrote:
> Currently the 'allocation' element is not used when creating new images
> with qemu-img. This patch interprets a non-zero value as a request to
> preallocate metadata when a qcow2 image is created.
> 
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=684793
> ---
>  src/storage/storage_backend.c |   19 +++++++++++++------
>  1 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 41a19a1..bf6f7cc 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -669,10 +669,15 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>      int imgformat = -1;
>      virCommandPtr cmd = NULL;
>      bool do_encryption = (vol->target.encryption != NULL);
> +    bool preallocate = false;
>      unsigned long long int size_arg;
>  
>      virCheckFlags(0, -1);
>  
> +    /* qcow2: preallocate metadata if requested allocation is non-zero */
> +    if (vol->allocation > 0 && vol->target.format == VIR_STORAGE_FILE_QCOW2)
> +        preallocate = true;
> +
>      const char *type = virStorageFileFormatTypeToString(vol->target.format);
>      const char *backingType = vol->backingStore.path ?
>          virStorageFileFormatTypeToString(vol->backingStore.format) : NULL;> @@ -842,12 +847,14 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>                               vol->target.path, NULL);
>          virCommandAddArgFormat(cmd, "%lluK", size_arg);
>  
> -        if (do_encryption) {
> -            if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) {
> -                virCommandAddArgList(cmd, "-o", "encryption=on", NULL);
> -            } else {
> -                virCommandAddArg(cmd, "-e");
> -            }
> +        if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS &&
> +            (do_encryption || preallocate)) {
> +            virCommandAddArg(cmd, "-o");
> +            virCommandAddArgFormat(cmd, "%s%s%s", do_encryption ? "encryption=on" : "",
> +                                   (do_encryption && preallocate) ? "," : "",
> +                                   preallocate ? "preallocation=metadata" : "");
> +        } else if (do_encryption){
> +            virCommandAddArg(cmd, "-e");
>          }
>      }

I've thought about doing it this way before, but I always felt it was
a bit of a gross hack. It would be nice to keep the 'allocation' element
as purely describing the data sector allocation. I was recently wondering
if we should just add a flag to the virStorageVolCreate like

  VIR_STORAGE_VOL_PREALLOC_METADATA


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|