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@xxxxxxxxxx> >> --- >> 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 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list