[libvirt] [PATCH 6/6] virsh: Add build flags to pool-create[-as] and pool-start

[John Ferlan]

On 12/16/2015 08:42 AM, Peter Krempa wrote:
> On Wed, Nov 25, 2015 at 14:11:08 -0500, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=830056
>>
>> Utilize recently added VIR_STORAGE_POOL_CREATE_WITH_BUILD* flags in
>> order to pass the flags along to the virStoragePoolCreateXML and
>> virStoragePoolCreate API's.
>>
>> This affects the 'virsh pool-create', 'virsh pool-create-as', and
>> 'virsh pool-start' commands.  While it could be argued that pool-start
>> doesn't need the flags, they could prove useful for someone trying to
>> do one command build --overwrite and start command processing or
>> essentially starting with a clean slate.
>>
>> NB:
>> This patch is loosely based upon code originally authored by Osier
>> Yang that were not reviewed and pushed, see:
>>
>> https://www.redhat.com/archives/libvir-list/2012-July/msg00497.html
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  tools/virsh-pool.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>>  tools/virsh.pod    | 25 ++++++++++++++++++-
>>  2 files changed, 94 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
>> index cb91cd3..1bb987d 100644
>> --- a/tools/virsh-pool.c
>> +++ b/tools/virsh-pool.c
>> @@ -47,6 +47,13 @@
>>       .help = N_("file containing an XML pool description")    \
>>      },                                                        \
>>  
>> +#define OPT_BUILD_COMMON                                      \
>> +    {.name = "build",                                         \
>> +     .type = VSH_OT_BOOL,                                     \
>> +     .flags = 0,                                              \
>> +     .help = N_("build the pool as normal")                   \
>> +    },                                                        \
>> +
>>  #define OPT_NO_OVERWRITE_COMMON                                   \
>>      {.name = "no-overwrite",                                      \
>>       .type = VSH_OT_BOOL,                                         \
>> @@ -235,6 +242,9 @@ static const vshCmdInfo info_pool_create[] = {
>>  
>>  static const vshCmdOptDef opts_pool_create[] = {
>>      OPT_FILE_COMMON
>> +    OPT_BUILD_COMMON
>> +    OPT_NO_OVERWRITE_COMMON
>> +    OPT_OVERWRITE_COMMON
>>
> 
> These look terrible without commas between entries.
> 

OK - so all fixed and the names are now:

VSH_POOL_OPT_COMMON
VSH_POOL_FILE_OPT_COMMON
VSH_POOL_BUILD_OPT_COMMON
VSH_POOL_NO_OVERWRITE_OPT_COMMON
VSH_POOL_OVERWRITE_OPT_COMMON
VSH_POOL_X_AS_OPT_COMMON

Each would require a comma when used within an opts_pool* struct

>>      {.name = NULL}
>>  };
>> @@ -246,15 +256,32 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
>>      const char *from = NULL;
>>      bool ret = true;
>>      char *buffer;
>> +    bool build;
>> +    bool overwrite;
>> +    bool no_overwrite;
>> +    unsigned int flags = 0;
>>      virshControlPtr priv = ctl->privData;
>>  
>>      if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
>>          return false;
>>  
>> +    build = vshCommandOptBool(cmd, "build");
>> +    overwrite = vshCommandOptBool(cmd, "overwrite");
>> +    no_overwrite = vshCommandOptBool(cmd, "no-overwrite");
>> +
>> +    VSH_EXCLUSIVE_OPTIONS_VAR(overwrite, no_overwrite);
> 
> This should be used only if the name of the variable matches the name of
> the option flag since the variable name is used in the error message in
> place of the option flag name.
> 

Oh yeah - I remember reading that.. I think at one time I had used the
VSH_EXCLUSIVE_OPTIONS_EXPR instead, but I cannot remember what happened
to cause me to use this one.

Both instances now changed to:

   VSH_EXCLUSIVE_OPTIONS_EXPR("overwrite", overwrite,
                               "no-overwrite", no_overwrite);

Is it preferable to see a v2 or are the edits as I've described sufficient?

John
>> +
>> +    if (build)
>> +        flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD;
>> +    if (overwrite)
>> +        flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE;
>> +    if (no_overwrite)
>> +        flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE;
>> +
>>      if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
>>          return false;
>>  
>> -    pool = virStoragePoolCreateXML(priv->conn, buffer, 0);
>> +    pool = virStoragePoolCreateXML(priv->conn, buffer, flags);
>>      VIR_FREE(buffer);
>>  
>>      if (pool != NULL) {
>> @@ -386,6 +413,9 @@ static const vshCmdInfo info_pool_create_as[] = {
>>  
>>  static const vshCmdOptDef opts_pool_create_as[] = {
>>      OPTS_POOL_COMMON_X_AS
>> +    OPT_BUILD_COMMON
>> +    OPT_NO_OVERWRITE_COMMON
>> +    OPT_OVERWRITE_COMMON
>>  
>>      {.name = NULL}
>>  };
>> @@ -397,8 +427,25 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
>>      const char *name;
>>      char *xml;
>>      bool printXML = vshCommandOptBool(cmd, "print-xml");
>> +    bool build;
>> +    bool overwrite;
>> +    bool no_overwrite;
>> +    unsigned int flags = 0;
>>      virshControlPtr priv = ctl->privData;
>>  
>> +    build = vshCommandOptBool(cmd, "build");
>> +    overwrite = vshCommandOptBool(cmd, "overwrite");
>> +    no_overwrite = vshCommandOptBool(cmd, "no-overwrite");
>> +
>> +    VSH_EXCLUSIVE_OPTIONS_VAR(overwrite, no_overwrite);
> 
> See above.
> 
>> +
>> +    if (build)
>> +        flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD;
>> +    if (overwrite)
>> +        flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE;
>> +    if (no_overwrite)
>> +        flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE;
>> +
>>      if (!virshBuildPoolXML(ctl, cmd, &name, &xml))
>>          return false;
>>
> 
> Peter
>