6:52 a.m.
This patchset introduces a new macro that allows simple checking for mutually
exclusive arguments in virsh and uses it in many occasions in virs.
Peter Krempa (8):
virsh: Introduce macros to reject mutually exclusive arguments
virsh-snapshot: Refactor virsh snapshot-list
virsh-host: Refactor cmdFreecell
virsh-domain: Fix flag name in error message to match the check
virsh-snapshot: Refactor cmdSnapshotCurrent
virsh-snapshot: Use the mutually exclusive params macro in
cmdSnapshotEdit
virsh-snapshot: Simplify cleanup path in cmdSnapshotEdit
virsh-domain: Simplify usage of --current, --live and --config flags
po/POTFILES.in | 1 +
tools/virsh-domain.c | 262 +++++++++++++++++++++----------------------------
tools/virsh-host.c | 55 +++++------
tools/virsh-snapshot.c | 181 +++++++++++++---------------------
tools/virsh.h | 42 ++++++++
5 files changed, 248 insertions(+), 293 deletions(-)
--
1.8.1.1
6:53 a.m.
New subject: [libvirt] [PATCH 1/8] virsh: Introduce macros to reject mutually exclusive arguments
This patch adds three macros to the virsh source tree that help to
easily check for mutually exclusive parameters.
VSH_EXCLUSIVE_OPTIONS_EXPR has four arguments, two expressions to check
and two names of the parameters to print in the message.
VSH_EXCLUSIVE_OPTIONS is more specific and check the command structure
for the parameters using vshCommandOptBool.
VSH_EXCLUSIVE_OPTIONS_VAR is meant to check boolean variables with the
same name as the parameters.
---
po/POTFILES.in | 1 +
tools/virsh.h | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index bd2c02e..7a2da8f 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -204,6 +204,7 @@ src/xenxs/xen_xm.c
tools/console.c
tools/libvirt-guests.sh.in
tools/virsh.c
+tools/virsh.h
tools/virsh-domain-monitor.c
tools/virsh-domain.c
tools/virsh-edit.c
diff --git a/tools/virsh.h b/tools/virsh.h
index fec9785..410d859 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -375,4 +375,46 @@ char *_vshStrdup(vshControl *ctl, const char *s, const char
*filename,
# define realloc use_vshRealloc_instead_of_realloc
# define strdup use_vshStrdup_instead_of_strdup
+/* Macros to help dealing with mutually exclusive options. */
+
+/* VSH_EXCLUSIVE_OPTIONS_EXPR:
+ *
+ * @NAME1: String containing the name of the option.
+ * @EXPR1: Expression to validate the variable (boolean variable)
+ * @NAME2: String containing the name of the option.
+ * @EXPR2: Expression to validate the variable (boolean variable)
+ *
+ * Reject mutually exclusive command options in virsh. Use the
+ * provided expression to check the variables.
+ */
+# define VSH_EXCLUSIVE_OPTIONS_EXPR(NAME1, EXPR1, NAME2, EXPR2) \
+ if ((EXPR1) && (EXPR2)) { \
+ vshError(ctl, _("Options --%s and --%s are mutually exclusive"), \
+ NAME1, NAME2); \
+ return false; \
+ }
+
+/* VSH_EXCLUSIVE_OPTIONS:
+ *
+ * @NAME1: String containing the name of the option.
+ * @NAME2: String containing the name of the option.
+ *
+ * Reject mutually exclusive command options in virsh. Use the
+ * vshCommandOptBool call to request them.
+ */
+# define VSH_EXCLUSIVE_OPTIONS(NAME1, NAME2) \
+ VSH_EXCLUSIVE_OPTIONS_EXPR(NAME1, vshCommandOptBool(cmd, NAME1), \
+ NAME2, vshCommandOptBool(cmd, NAME2))
+
+/* VSH_EXCLUSIVE_OPTIONS_VAR:
+ *
+ * @VARNAME1: Boolean variable containing the value of the option of same name
+ * @VARNAME2: Boolean variable containing the value of the option of same name
+ *
+ * Reject mutually exclusive command options in virsh. Check in variables that
+ * contain the value and have same name as the option.
+ */
+# define VSH_EXCLUSIVE_OPTIONS_VAR(VARNAME1, VARNAME2) \
+ VSH_EXCLUSIVE_OPTIONS_EXPR(#VARNAME1, VARNAME1, #VARNAME2, VARNAME2)
+
#endif /* VIRSH_H */
--
1.8.1.1
10:03 p.m.
New subject: [libvirt] [PATCH 1/8] virsh: Introduce macros to reject mutually exclusive arguments
On 03/07/2013 05:53 AM, Peter Krempa wrote:
This patch adds three macros to the virsh source tree that help to
easily check for mutually exclusive parameters.
VSH_EXCLUSIVE_OPTIONS_EXPR has four arguments, two expressions to check
and two names of the parameters to print in the message.
VSH_EXCLUSIVE_OPTIONS is more specific and check the command structure
for the parameters using vshCommandOptBool.
VSH_EXCLUSIVE_OPTIONS_VAR is meant to check boolean variables with the
same name as the parameters.
---
po/POTFILES.in | 1 +
tools/virsh.h | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+)
+++ b/tools/virsh.h
@@ -375,4 +375,46 @@ char *_vshStrdup(vshControl *ctl, const char *s, const char
*filename,
# define realloc use_vshRealloc_instead_of_realloc
# define strdup use_vshStrdup_instead_of_strdup
+/* Macros to help dealing with mutually exclusive options. */
+
+/* VSH_EXCLUSIVE_OPTIONS_EXPR:
+ *
+ * @NAME1: String containing the name of the option.
+ * @EXPR1: Expression to validate the variable (boolean variable)
+ * @NAME2: String containing the name of the option.
+ * @EXPR2: Expression to validate the variable (boolean variable)
+ *
+ * Reject mutually exclusive command options in virsh. Use the
+ * provided expression to check the variables.
+ */
+# define VSH_EXCLUSIVE_OPTIONS_EXPR(NAME1, EXPR1, NAME2, EXPR2) \
+ if ((EXPR1) && (EXPR2)) { \
+ vshError(ctl, _("Options --%s and --%s are mutually exclusive"), \
+ NAME1, NAME2); \
+ return false; \
This does an early return, and therefore the macro must be used before
anything that needs cleanup.
At any rate, these look sane to me. ACK if you add a sentence along
those lines.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:53 a.m.
New subject: [libvirt] [PATCH 2/8] virsh-snapshot: Refactor virsh snapshot-list
Simplify error handling and mutually exlusive option checking.
---
tools/virsh-snapshot.c | 121 ++++++++++++++++++-------------------------------
1 file changed, 45 insertions(+), 76 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index ed41014..d4aa4de 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -1571,66 +1571,34 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
virDomainPtr dom = NULL;
bool ret = false;
unsigned int flags = 0;
- bool show_parent = false;
int i;
xmlDocPtr xml = NULL;
xmlXPathContextPtr ctxt = NULL;
char *doc = NULL;
virDomainSnapshotPtr snapshot = NULL;
char *state = NULL;
- char *parent = NULL;
long long creation_longlong;
time_t creation_time_t;
char timestr[100];
struct tm time_info;
bool tree = vshCommandOptBool(cmd, "tree");
bool name = vshCommandOptBool(cmd, "name");
- const char *from = NULL;
+ bool from = vshCommandOptBool(cmd, "from");
+ bool parent = vshCommandOptBool(cmd, "parent");
+ bool roots = vshCommandOptBool(cmd, "roots");
+ bool current = vshCommandOptBool(cmd, "current");
+ const char *from_snap = NULL;
+ char *parent_snap = NULL;
virDomainSnapshotPtr start = NULL;
vshSnapshotListPtr snaplist = NULL;
- if (tree && name) {
- vshError(ctl, "%s",
- _("--tree and --name are mutually exclusive"));
- return false;
- }
-
- dom = vshCommandOptDomain(ctl, cmd, NULL);
- if (dom == NULL)
- goto cleanup;
+ VSH_EXCLUSIVE_OPTIONS_VAR(tree, name);
+ VSH_EXCLUSIVE_OPTIONS_VAR(parent, roots);
+ VSH_EXCLUSIVE_OPTIONS_VAR(parent, tree);
+ VSH_EXCLUSIVE_OPTIONS_VAR(roots, tree);
+ VSH_EXCLUSIVE_OPTIONS_VAR(roots, from);
+ VSH_EXCLUSIVE_OPTIONS_VAR(roots, current);
- if ((vshCommandOptBool(cmd, "from") ||
- vshCommandOptBool(cmd, "current")) &&
- vshLookupSnapshot(ctl, cmd, "from", true, dom, &start, &from)
< 0)
- goto cleanup;
-
- if (vshCommandOptBool(cmd, "parent")) {
- if (vshCommandOptBool(cmd, "roots")) {
- vshError(ctl, "%s",
- _("--parent and --roots are mutually exclusive"));
- goto cleanup;
- }
- if (tree) {
- vshError(ctl, "%s",
- _("--parent and --tree are mutually exclusive"));
- goto cleanup;
- }
- show_parent = true;
- } else if (vshCommandOptBool(cmd, "roots")) {
- if (tree) {
- vshError(ctl, "%s",
- _("--roots and --tree are mutually exclusive"));
- goto cleanup;
- }
- if (from) {
- vshError(ctl, "%s",
- vshCommandOptBool(cmd, "current") ?
- _("--roots and --current are mutually exclusive") :
- _("--roots and --from are mutually exclusive"));
- goto cleanup;
- }
- flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
- }
#define FILTER(option, flag) \
do { \
if (vshCommandOptBool(cmd, option)) { \
@@ -1638,7 +1606,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, \
_("--%s and --tree are mutually exclusive"), \
option); \
- goto cleanup; \
+ return false; \
} \
flags |= VIR_DOMAIN_SNAPSHOT_LIST_ ## flag; \
} \
@@ -1653,28 +1621,36 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
FILTER("external", EXTERNAL);
#undef FILTER
- if (vshCommandOptBool(cmd, "metadata")) {
+ if (roots)
+ flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
+
+ if (vshCommandOptBool(cmd, "metadata"))
flags |= VIR_DOMAIN_SNAPSHOT_LIST_METADATA;
- }
- if (vshCommandOptBool(cmd, "no-metadata")) {
+
+ if (vshCommandOptBool(cmd, "no-metadata"))
flags |= VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA;
- }
if (vshCommandOptBool(cmd, "descendants")) {
- if (!from) {
+ if (!from && !current) {
vshError(ctl, "%s",
_("--descendants requires either --from or --current"));
- goto cleanup;
+ return false;
}
flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS;
}
- if ((snaplist = vshSnapshotListCollect(ctl, dom, start, flags,
- tree)) == NULL)
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ return false;
+
+ if ((from || current) &&
+ vshLookupSnapshot(ctl, cmd, "from", true, dom, &start,
&from_snap) < 0)
+ goto cleanup;
+
+ if (!(snaplist = vshSnapshotListCollect(ctl, dom, start, flags, tree)))
goto cleanup;
if (!tree && !name) {
- if (show_parent)
+ if (parent)
vshPrintExtra(ctl, " %-20s %-25s %-15s %s",
_("Name"), _("Creation Time"),
_("State"),
_("Parent"));
@@ -1682,12 +1658,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, " %-20s %-25s %s",
_("Name"), _("Creation Time"),
_("State"));
vshPrintExtra(ctl, "\n"
-"------------------------------------------------------------\n");
- }
-
- if (!snaplist->nsnaps) {
- ret = true;
- goto cleanup;
+ "------------------------------"
+ "------------------------------\n");
}
if (tree) {
@@ -1705,7 +1677,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
const char *snap_name;
/* free up memory from previous iterations of the loop */
- VIR_FREE(parent);
+ VIR_FREE(parent_snap);
VIR_FREE(state);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
@@ -1716,25 +1688,24 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
assert(snap_name);
if (name) {
+ /* just print the snapshot name */
vshPrint(ctl, "%s\n", snap_name);
continue;
}
- doc = virDomainSnapshotGetXMLDesc(snapshot, 0);
- if (!doc)
+ if (!(doc = virDomainSnapshotGetXMLDesc(snapshot, 0)))
continue;
- xml = virXMLParseStringCtxt(doc, _("(domain_snapshot)"), &ctxt);
- if (!xml)
+ if (!(xml = virXMLParseStringCtxt(doc, _("(domain_snapshot)"),
&ctxt)))
continue;
- if (show_parent)
- parent = virXPathString("string(/domainsnapshot/parent/name)",
- ctxt);
+ if (parent)
+ parent_snap =
virXPathString("string(/domainsnapshot/parent/name)",
+ ctxt);
- state = virXPathString("string(/domainsnapshot/state)", ctxt);
- if (state == NULL)
+ if (!(state = virXPathString("string(/domainsnapshot/state)", ctxt)))
continue;
+
if (virXPathLongLong("string(/domainsnapshot/creationTime)", ctxt,
&creation_longlong) < 0)
continue;
@@ -1749,7 +1720,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
if (parent)
vshPrint(ctl, " %-20s %-25s %-15s %s\n",
- snap_name, timestr, state, parent);
+ snap_name, timestr, state, parent_snap);
else
vshPrint(ctl, " %-20s %-25s %s\n", snap_name, timestr, state);
}
@@ -1759,15 +1730,13 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
cleanup:
/* this frees up memory from the last iteration of the loop */
vshSnapshotListFree(snaplist);
- VIR_FREE(parent);
+ VIR_FREE(parent_snap);
VIR_FREE(state);
- if (start)
- virDomainSnapshotFree(start);
+ virDomainSnapshotFree(start);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
VIR_FREE(doc);
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
return ret;
}
--
1.8.1.1
10:11 p.m.
New subject: [libvirt] [PATCH 2/8] virsh-snapshot: Refactor virsh snapshot-list
On 03/07/2013 05:53 AM, Peter Krempa wrote:
Simplify error handling and mutually exlusive option checking.
s/exlusive/exclusive/
---
tools/virsh-snapshot.c | 121 ++++++++++++++++++-------------------------------
1 file changed, 45 insertions(+), 76 deletions(-)
ACK. It might have been nicer to split into two patches - one to rename
existing variables with no other changes, and the second to use the new
macros; but it's not worth splitting now.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:53 a.m.
New subject: [libvirt] [PATCH 3/8] virsh-host: Refactor cmdFreecell
Use the new helpers to determine mutually exclusive options and touch up
some parts to simplify the code.
---
tools/virsh-host.c | 55 +++++++++++++++++++++++-------------------------------
1 file changed, 23 insertions(+), 32 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 4345839..22bcd6c 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -159,35 +159,29 @@ static const vshCmdOptDef opts_freecell[] = {
static bool
cmdFreecell(vshControl *ctl, const vshCmd *cmd)
{
- bool func_ret = false;
- int ret;
- int cell = -1, cell_given;
- unsigned long long memory;
+ bool ret = false;
+ int cell = -1;
+ unsigned long long memory = 0;
xmlNodePtr *nodes = NULL;
unsigned long nodes_cnt;
unsigned long *nodes_id = NULL;
unsigned long long *nodes_free = NULL;
- int all_given;
+ bool all = vshCommandOptBool(cmd, "all");
+ bool cellno = vshCommandOptBool(cmd, "cellno");
int i;
char *cap_xml = NULL;
xmlDocPtr xml = NULL;
xmlXPathContextPtr ctxt = NULL;
- if ((cell_given = vshCommandOptInt(cmd, "cellno", &cell)) < 0) {
- vshError(ctl, "%s", _("cell number has to be a number"));
- goto cleanup;
- }
- all_given = vshCommandOptBool(cmd, "all");
+ VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);
- if (all_given && cell_given) {
- vshError(ctl, "%s", _("--cellno and --all are mutually exclusive.
"
- "Please choose only one."));
- goto cleanup;
+ if (cellno && vshCommandOptInt(cmd, "cellno", &cell) < 0) {
+ vshError(ctl, "%s", _("cell number has to be a number"));
+ return false;
}
- if (all_given) {
- cap_xml = virConnectGetCapabilities(ctl->conn);
- if (!cap_xml) {
+ if (all) {
+ if (!(cap_xml = virConnectGetCapabilities(ctl->conn))) {
vshError(ctl, "%s", _("unable to get node
capabilities"));
goto cleanup;
}
@@ -197,6 +191,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, "%s", _("unable to get node
capabilities"));
goto cleanup;
}
+
nodes_cnt = virXPathNodeSet("/capabilities/host/topology/cells/cell",
ctxt, &nodes);
@@ -219,15 +214,14 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
}
VIR_FREE(val);
nodes_id[i]=id;
- ret = virNodeGetCellsFreeMemory(ctl->conn, &(nodes_free[i]), id, 1);
- if (ret != 1) {
+ if (virNodeGetCellsFreeMemory(ctl->conn, &(nodes_free[i]),
+ id, 1) != 1) {
vshError(ctl, _("failed to get free memory for NUMA node "
"number: %lu"), id);
goto cleanup;
}
}
- memory = 0;
for (cell = 0; cell < nodes_cnt; cell++) {
vshPrint(ctl, "%5lu: %10llu KiB\n", nodes_id[cell],
(nodes_free[cell]/1024));
@@ -237,23 +231,20 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, "--------------------\n");
vshPrintExtra(ctl, "%5s: %10llu KiB\n", _("Total"),
memory/1024);
} else {
- if (!cell_given) {
- memory = virNodeGetFreeMemory(ctl->conn);
- if (memory == 0)
+ if (cellno) {
+ if (virNodeGetCellsFreeMemory(ctl->conn, &memory, cell, 1) != 1)
goto cleanup;
- } else {
- ret = virNodeGetCellsFreeMemory(ctl->conn, &memory, cell, 1);
- if (ret != 1)
+
+ vshPrint(ctl, "%d: %llu KiB\n", cell, (memory/1024));
+ } else {
+ if ((memory = virNodeGetFreeMemory(ctl->conn)) == 0)
goto cleanup;
- }
- if (cell == -1)
vshPrint(ctl, "%s: %llu KiB\n", _("Total"),
(memory/1024));
- else
- vshPrint(ctl, "%d: %llu KiB\n", cell, (memory/1024));
+ }
}
- func_ret = true;
+ ret = true;
cleanup:
xmlXPathFreeContext(ctxt);
@@ -262,7 +253,7 @@ cleanup:
VIR_FREE(nodes_free);
VIR_FREE(nodes_id);
VIR_FREE(cap_xml);
- return func_ret;
+ return ret;
}
/*
--
1.8.1.1
10:48 p.m.
New subject: [libvirt] [PATCH 3/8] virsh-host: Refactor cmdFreecell
On 03/07/2013 05:53 AM, Peter Krempa wrote:
Use the new helpers to determine mutually exclusive options and touch
up
some parts to simplify the code.
---
tools/virsh-host.c | 55 +++++++++++++++++++++++-------------------------------
1 file changed, 23 insertions(+), 32 deletions(-)
- if ((cell_given = vshCommandOptInt(cmd, "cellno", &cell)) < 0) {
- vshError(ctl, "%s", _("cell number has to be a number"));
- goto cleanup;
- }
- all_given = vshCommandOptBool(cmd, "all");
+ VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);
- if (all_given && cell_given) {
- vshError(ctl, "%s", _("--cellno and --all are mutually exclusive.
"
- "Please choose only one."));
- goto cleanup;
+ if (cellno && vshCommandOptInt(cmd, "cellno", &cell) < 0) {
The 'cellno &&' portion is extra; we didn't need it before, so I
don't
know why you added it here. vshCommandOptInt returns 0 if --cellno was
not provided, since it is not a mandatory option.
@@ -237,23 +231,20 @@ cmdFreecell(vshControl *ctl, const vshCmd
*cmd)
vshPrintExtra(ctl, "--------------------\n");
vshPrintExtra(ctl, "%5s: %10llu KiB\n", _("Total"),
memory/1024);
} else {
- if (!cell_given) {
- memory = virNodeGetFreeMemory(ctl->conn);
- if (memory == 0)
+ if (cellno) {
Indentation is off.
+ if (virNodeGetCellsFreeMemory(ctl->conn, &memory,
cell, 1) != 1)
goto cleanup;
- } else {
- ret = virNodeGetCellsFreeMemory(ctl->conn, &memory, cell, 1);
- if (ret != 1)
+
+ vshPrint(ctl, "%d: %llu KiB\n", cell, (memory/1024));
+ } else {
and again.
ACK if you either explain the added conjunct or remove it, and if you
fix the whitespace.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:03 a.m.
New subject: [libvirt] [PATCH 3/8] virsh-host: Refactor cmdFreecell
On 03/21/13 04:48, Eric Blake wrote:
On 03/07/2013 05:53 AM, Peter Krempa wrote:
> Use the new helpers to determine mutually exclusive options and touch up
> some parts to simplify the code.
> ---
> tools/virsh-host.c | 55 +++++++++++++++++++++++-------------------------------
> 1 file changed, 23 insertions(+), 32 deletions(-)
>
>
> - if ((cell_given = vshCommandOptInt(cmd, "cellno", &cell)) < 0)
{
> - vshError(ctl, "%s", _("cell number has to be a
number"));
> - goto cleanup;
> - }
> - all_given = vshCommandOptBool(cmd, "all");
> + VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);
>
> - if (all_given && cell_given) {
> - vshError(ctl, "%s", _("--cellno and --all are mutually
exclusive. "
> - "Please choose only one."));
> - goto cleanup;
> + if (cellno && vshCommandOptInt(cmd, "cellno", &cell) <
0) {
The 'cellno &&' portion is extra; we didn't need it before, so I
don't
know why you added it here. vshCommandOptInt returns 0 if --cellno was
not provided, since it is not a mandatory option.
I used it there to avoid the call to vshCommandOptInt in case it won't
parse anything as the cellno parameter is missing anyways. It is not
strictly needed but it's a optimization so I'll leave it in.
ACK if you either explain the added conjunct or remove it, and if you
fix the whitespace.
Peter
6:53 a.m.
New subject: [libvirt] [PATCH 4/8] virsh-domain: Fix flag name in error message to match the check
The check is done on the "--paused" flag by the error message stated
"--started"
---
tools/virsh-domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 96dd4fa..8dad85d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3792,7 +3792,7 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd)
* step fails, but the define step will always fail on invalid
* flags, so we reject it up front to avoid looping. */
if (define_flags == (VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED)) {
- vshError(ctl, "%s", _("--running and --saved are mutually
exclusive"));
+ vshError(ctl, "%s", _("--running and --paused are mutually
exclusive"));
return false;
}
--
1.8.1.1
10:50 p.m.
New subject: [libvirt] [PATCH 4/8] virsh-domain: Fix flag name in error message to match the check
On 03/07/2013 05:53 AM, Peter Krempa wrote:
The check is done on the "--paused" flag by the error
message stated
s/by/but/
"--started"
---
tools/virsh-domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK.
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 96dd4fa..8dad85d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3792,7 +3792,7 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd)
* step fails, but the define step will always fail on invalid
* flags, so we reject it up front to avoid looping. */
if (define_flags == (VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED)) {
- vshError(ctl, "%s", _("--running and --saved are mutually
exclusive"));
+ vshError(ctl, "%s", _("--running and --paused are mutually
exclusive"));
return false;
}
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:53 a.m.
New subject: [libvirt] [PATCH 5/8] virsh-snapshot: Refactor cmdSnapshotCurrent
Use the exclusive parameter checker and touch up some parts to simplify
code.
---
tools/virsh-snapshot.c | 46 +++++++++++++++++++++-------------------------
1 file changed, 21 insertions(+), 25 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index d4aa4de..78db178 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -712,9 +712,10 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptBool(cmd, "security-info"))
flags |= VIR_DOMAIN_XML_SECURE;
- dom = vshCommandOptDomain(ctl, cmd, &domname);
- if (dom == NULL)
- goto cleanup;
+ VSH_EXCLUSIVE_OPTIONS("name", "snapshotname");
+
+ if (!(dom = vshCommandOptDomain(ctl, cmd, &domname)))
+ return false;
if (vshCommandOptStringReq(ctl, cmd, "snapshotname", &snapshotname)
< 0)
goto cleanup;
@@ -724,52 +725,48 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
flags = (VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT);
- if (vshCommandOptBool(cmd, "name")) {
- vshError(ctl, "%s",
- _("--name and snapshotname are mutually exclusive"));
- goto cleanup;
- }
- snapshot = virDomainSnapshotLookupByName(dom, snapshotname, 0);
- if (snapshot == NULL)
+ if (!(snapshot = virDomainSnapshotLookupByName(dom, snapshotname, 0)))
goto cleanup;
+
xml = virDomainSnapshotGetXMLDesc(snapshot, VIR_DOMAIN_XML_SECURE);
if (!xml)
goto cleanup;
+
/* strstr is safe here, since xml came from libvirt API and not user */
if (strstr(xml, "<state>disk-snapshot</state>"))
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
- snapshot2 = virDomainSnapshotCreateXML(dom, xml, flags);
- if (snapshot2 == NULL)
+
+ if (!(snapshot2 = virDomainSnapshotCreateXML(dom, xml, flags)))
goto cleanup;
+
virDomainSnapshotFree(snapshot2);
vshPrint(ctl, _("Snapshot %s set as current"), snapshotname);
ret = true;
goto cleanup;
}
- current = virDomainHasCurrentSnapshot(dom, 0);
- if (current < 0) {
+ if ((current = virDomainHasCurrentSnapshot(dom, 0)) < 0)
goto cleanup;
- } else if (!current) {
+
+ if (!current) {
vshError(ctl, _("domain '%s' has no current snapshot"),
domname);
goto cleanup;
} else {
- const char *name = NULL;
-
if (!(snapshot = virDomainSnapshotCurrent(dom, 0)))
goto cleanup;
if (vshCommandOptBool(cmd, "name")) {
- name = virDomainSnapshotGetName(snapshot);
- if (!name)
+ const char *name;
+ if (!(name = virDomainSnapshotGetName(snapshot)))
goto cleanup;
+
+ vshPrint(ctl, "%s", name);
} else {
- xml = virDomainSnapshotGetXMLDesc(snapshot, flags);
- if (!xml)
+ if (!(xml = virDomainSnapshotGetXMLDesc(snapshot, flags)))
goto cleanup;
- }
- vshPrint(ctl, "%s", name ? name : xml);
+ vshPrint(ctl, "%s", xml);
+ }
}
ret = true;
@@ -780,8 +777,7 @@ cleanup:
VIR_FREE(xml);
if (snapshot)
virDomainSnapshotFree(snapshot);
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
return ret;
}
--
1.8.1.1
10:52 p.m.
New subject: [libvirt] [PATCH 5/8] virsh-snapshot: Refactor cmdSnapshotCurrent
On 03/07/2013 05:53 AM, Peter Krempa wrote:
Use the exclusive parameter checker and touch up some parts to
simplify
code.
---
tools/virsh-snapshot.c | 46 +++++++++++++++++++++-------------------------
1 file changed, 21 insertions(+), 25 deletions(-)
ACK
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:53 a.m.
New subject: [libvirt] [PATCH 6/8] virsh-snapshot: Use the mutually exclusive params macro in cmdSnapshotEdit
---
tools/virsh-snapshot.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 78db178..3add7fb 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -584,11 +584,7 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
bool rename_okay = vshCommandOptBool(cmd, "rename");
bool clone_okay = vshCommandOptBool(cmd, "clone");
- if (rename_okay && clone_okay) {
- vshError(ctl, "%s",
- _("--rename and --clone are mutually exclusive"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_EXPR("rename", rename_okay, "clone",
clone_okay)
if (vshCommandOptBool(cmd, "current") &&
vshCommandOptBool(cmd, "snapshotname"))
--
1.8.1.1
10:53 p.m.
New subject: [libvirt] [PATCH 6/8] virsh-snapshot: Use the mutually exclusive params macro in cmdSnapshotEdit
On 03/07/2013 05:53 AM, Peter Krempa wrote:
---
tools/virsh-snapshot.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
ACK.
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 78db178..3add7fb 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -584,11 +584,7 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
bool rename_okay = vshCommandOptBool(cmd, "rename");
bool clone_okay = vshCommandOptBool(cmd, "clone");
- if (rename_okay && clone_okay) {
- vshError(ctl, "%s",
- _("--rename and --clone are mutually exclusive"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_EXPR("rename", rename_okay, "clone",
clone_okay)
if (vshCommandOptBool(cmd, "current") &&
vshCommandOptBool(cmd, "snapshotname"))
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:53 a.m.
New subject: [libvirt] [PATCH 7/8] virsh-snapshot: Simplify cleanup path in cmdSnapshotEdit
---
tools/virsh-snapshot.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 3add7fb..e1f66e3 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -590,9 +590,8 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd)
vshCommandOptBool(cmd, "snapshotname"))
define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT;
- dom = vshCommandOptDomain(ctl, cmd, NULL);
- if (dom == NULL)
- goto cleanup;
+ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
+ return false;
if (vshLookupSnapshot(ctl, cmd, "snapshotname", false, dom,
&snapshot, &name) < 0)
@@ -651,8 +650,7 @@ cleanup:
virDomainSnapshotFree(edited);
if (snapshot)
virDomainSnapshotFree(snapshot);
- if (dom)
- virDomainFree(dom);
+ virDomainFree(dom);
return ret;
}
--
1.8.1.1
10:53 p.m.
New subject: [libvirt] [PATCH 7/8] virsh-snapshot: Simplify cleanup path in cmdSnapshotEdit
On 03/07/2013 05:53 AM, Peter Krempa wrote:
---
tools/virsh-snapshot.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:53 a.m.
New subject: [libvirt] [PATCH 8/8] virsh-domain: Simplify usage of --current, --live and --config flags
This patch uses the new helper to avoid the more complex check for
domain state modification flags.
---
tools/virsh-domain.c | 260 ++++++++++++++++++++++-----------------------------
1 file changed, 111 insertions(+), 149 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8dad85d..4c00db3 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1068,18 +1068,15 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
bool live = vshCommandOptBool(cmd, "live");
bool ret = false;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- }
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
goto cleanup;
@@ -1256,18 +1253,15 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
bool config = vshCommandOptBool(cmd, "config");
bool live = vshCommandOptBool(cmd, "live");
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- }
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -2570,18 +2564,15 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd)
virNetDevBandwidthRate inbound, outbound;
int i;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- }
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
@@ -4146,18 +4137,15 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
bool config = vshCommandOptBool(cmd, "config");
bool live = vshCommandOptBool(cmd, "live");
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- }
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -5618,21 +5606,18 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
bool query = false; /* Query mode if no cpulist */
unsigned int flags = 0;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- /* neither option is specified */
- if (!live && !config)
- flags = -1;
- }
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+ /* neither option is specified */
+ if (!current && !live && !config)
+ flags = -1;
if (vshCommandOptStringReq(ctl, cmd, "cpulist", &cpulist) < 0)
return false;
@@ -5855,21 +5840,18 @@ cmdEmulatorPin(vshControl *ctl, const vshCmd *cmd)
bool query = false; /* Query mode if no cpulist */
unsigned int flags = 0;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- /* neither option is specified */
- if (!live && !config)
- flags = -1;
- }
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+ /* neither option is specified */
+ if (!current && !live && !config)
+ flags = -1;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -6058,21 +6040,18 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
bool current = vshCommandOptBool(cmd, "current");
unsigned int flags = 0;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- /* neither option is specified */
- if (!live && !config && !maximum)
- flags = -1;
- }
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+ /* neither option is specified */
+ if (!current && !live && !config && !maximum)
+ flags = -1;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -6766,18 +6745,13 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
bool ret = false;
unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
- flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -7189,21 +7163,18 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
bool current = vshCommandOptBool(cmd, "current");
unsigned int flags = 0;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- /* neither option is specified */
- if (!live && !config)
- flags = -1;
- }
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+ /* neither option is specified */
+ if (!current && !live && !config)
+ flags = -1;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -7295,20 +7266,18 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
bool current = vshCommandOptBool(cmd, "current");
unsigned int flags = VIR_DOMAIN_MEM_MAXIMUM;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- /* neither option is specified */
- if (!live && !config)
- flags = -1;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
+ flags = VIR_DOMAIN_AFFECT_CURRENT;
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
+ /* neither option is specified */
+ if (!current && !live && !config)
+ flags = -1;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -7619,18 +7588,15 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd)
bool live = vshCommandOptBool(cmd, "live");
const char *mode = NULL;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- }
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
@@ -10318,19 +10284,15 @@ cmdChangeMedia(vshControl *ctl, const vshCmd *cmd)
action = "update";
}
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
- flags = VIR_DOMAIN_AFFECT_CURRENT;
- } else {
- if (config)
- flags |= VIR_DOMAIN_AFFECT_CONFIG;
- if (live)
- flags |= VIR_DOMAIN_AFFECT_LIVE;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+ if (current)
+ flags = VIR_DOMAIN_AFFECT_CURRENT;
+ if (config)
+ flags |= VIR_DOMAIN_AFFECT_CONFIG;
+ if (live)
+ flags |= VIR_DOMAIN_AFFECT_LIVE;
if (force)
flags |= VIR_DOMAIN_DEVICE_MODIFY_FORCE;
--
1.8.1.1
10:59 p.m.
New subject: [libvirt] [PATCH 8/8] virsh-domain: Simplify usage of --current, --live and --config flags
On 03/07/2013 05:53 AM, Peter Krempa wrote:
This patch uses the new helper to avoid the more complex check for
domain state modification flags.
---
tools/virsh-domain.c | 260 ++++++++++++++++++++++-----------------------------
1 file changed, 111 insertions(+), 149 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8dad85d..4c00db3 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1068,18 +1068,15 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
bool live = vshCommandOptBool(cmd, "live");
bool ret = false;
- if (current) {
- if (live || config) {
- vshError(ctl, "%s", _("--current must be specified
exclusively"));
- return false;
- }
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
+ VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
+
+ if (current)
flags = VIR_DOMAIN_AFFECT_CURRENT;
We already know that VIR_DOMAIN_AFFECT_CURRENT == 0; if flags was
pre-initialized to 0, this assignment is redundant. But I guess it
doesn't hurt.
+ /* neither option is specified */
+ if (!current && !live && !config && !maximum)
Generally, 'neither option' implies a choice between 2, but here you are
choosing between 4. A better wording might be 'no options were specified'.
As my complaints don't affect correctness of your patch as-is,
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:46 a.m.
On 03/07/13 13:52, Peter Krempa wrote:
This patchset introduces a new macro that allows simple checking for
mutually
exclusive arguments in virsh and uses it in many occasions in virs.
Peter Krempa (8):
virsh: Introduce macros to reject mutually exclusive arguments
virsh-snapshot: Refactor virsh snapshot-list
virsh-host: Refactor cmdFreecell
virsh-domain: Fix flag name in error message to match the check
virsh-snapshot: Refactor cmdSnapshotCurrent
virsh-snapshot: Use the mutually exclusive params macro in
cmdSnapshotEdit
virsh-snapshot: Simplify cleanup path in cmdSnapshotEdit
virsh-domain: Simplify usage of --current, --live and --config flags
po/POTFILES.in | 1 +
tools/virsh-domain.c | 262 +++++++++++++++++++++----------------------------
tools/virsh-host.c | 55 +++++------
tools/virsh-snapshot.c | 181 +++++++++++++---------------------
tools/virsh.h | 42 ++++++++
5 files changed, 248 insertions(+), 293 deletions(-)
Ping?
4265
days inactive
4279
days old
18 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa