Re: [PATCH v3 2/2] virsh: allow metadata preallocation when creating volumes

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 05.12.2012 11:48, Ján Tomko wrote:
> Add --prealloc-metadata flag to these commands:
> vol-clone
> vol-create
> vol-create-as
> vol-create-from
> ---
>  tools/virsh-volume.c |   25 +++++++++++++++++++++----
>  tools/virsh.pod      |   23 ++++++++++++++++++++---
>  2 files changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
> index f219de9..1ba0188 100644
> --- a/tools/virsh-volume.c
> +++ b/tools/virsh-volume.c
> @@ -124,6 +124,7 @@ static const vshCmdOptDef opts_vol_create_as[] = {
>       N_("the backing volume if taking a snapshot")},
>      {"backing-vol-format", VSH_OT_STRING, 0,
>       N_("format of backing volume if taking a snapshot")},
> +    {"prealloc-metadata", VSH_OT_BOOL, 0, N_("preallocate metadata (for qcow2 instead of full allocation)")},
>      {NULL, 0, 0, NULL}
>  };
>  
> @@ -146,7 +147,10 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
>      const char *snapshotStrVol = NULL, *snapshotStrFormat = NULL;
>      unsigned long long capacity, allocation = 0;
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    unsigned long flags = 0;
>  
> +    if (vshCommandOptBool(cmd, "prealloc-metadata"))
> +        flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;

While this works for now, I'd rather do it as:

  flags |= VIR_STORAGE_..._METADATA;

since this maybe easily overlooked, and reduces a future patch size if
somebody invents a new flag and add its conditional setting in the front
of yours.

Otherwise looking good. ACK with this squashed in:

diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 1ba0188..145953b 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -150,7 +150,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd)
     unsigned long flags = 0;

     if (vshCommandOptBool(cmd, "prealloc-metadata"))
-        flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
+        flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
     if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL,
                                      VSH_BYNAME)))
         return false;
@@ -306,7 +306,7 @@ cmdVolCreate(vshControl *ctl, const vshCmd *cmd)
     char *buffer;

     if (vshCommandOptBool(cmd, "prealloc-metadata"))
-        flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
+        flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
     if (!(pool = vshCommandOptPoolBy(ctl, cmd, "pool", NULL,
                                            VSH_BYNAME)))
         return false;
@@ -369,7 +369,7 @@ cmdVolCreateFrom(vshControl *ctl, const vshCmd *cmd)
         goto cleanup;

     if (vshCommandOptBool(cmd, "prealloc-metadata"))
-        flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
+        flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
     if (vshCommandOptString(cmd, "file", &from) <= 0) {
         goto cleanup;
     }
@@ -465,7 +465,7 @@ cmdVolClone(vshControl *ctl, const vshCmd *cmd)
         goto cleanup;

     if (vshCommandOptBool(cmd, "prealloc-metadata"))
-        flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;
+        flags |= VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA;

     origpool = virStoragePoolLookupByVolume(origvol);
     if (!origpool) {


Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list