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(a)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