On Wed, Mar 04, 2015 at 17:17:05 +0100, Pavel Hrdina wrote:
Commit message is too sparse.
Also I doubt that this on itself resolves this bug.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
tools/virsh-domain.c | 62 +++++++++++++++++-----------------------------------
1 file changed, 20 insertions(+), 42 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 55c269c..773f9f1 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8276,7 +8276,7 @@ vshMemtuneGetSize(const vshCmd *cmd, const char *name, long long
*value)
if (virScaleInteger(&tmp, end, 1024, LLONG_MAX) < 0)
return -1;
*value = VIR_DIV_UP(tmp, 1024);
- return 0;
+ return 1;
Now that the return values from this function actually make sense it
would be worth adding a comment what they mean.
}
static bool
@@ -8294,6 +8294,7 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
bool current = vshCommandOptBool(cmd, "current");
bool config = vshCommandOptBool(cmd, "config");
bool live = vshCommandOptBool(cmd, "live");
+ int rc = 0;
rc doesn't need to be initialized as the macro below sets and tests it.
VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
@@ -8306,50 +8307,26 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshMemtuneGetSize(cmd, "hard-limit", &hard_limit) < 0 ||
- vshMemtuneGetSize(cmd, "soft-limit", &soft_limit) < 0 ||
- vshMemtuneGetSize(cmd, "swap-hard-limit", &swap_hard_limit) < 0
||
- vshMemtuneGetSize(cmd, "min-guarantee", &min_guarantee) < 0) {
- vshError(ctl, "%s",
- _("Unable to parse integer parameter"));
- goto cleanup;
- }
+#define PARSE_MEMTUNE_PARAM(NAME, VALUE, FIELD) \
+ if ((rc = vshMemtuneGetSize(cmd, NAME, &VALUE)) < 0) { \
+ vshError(ctl, "%s", _("Unable to parse integer parameter
'NAME'")); \
+ goto cleanup; \
+ } \
+ if (rc == 1) { \
+ if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
\
+ FIELD, VALUE) < 0) \
+ goto save_error; \
+ } \
- if (hard_limit) {
- if (hard_limit == -1)
- hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_MEMORY_HARD_LIMIT,
- hard_limit) < 0)
- goto save_error;
- }
- if (soft_limit) {
- if (soft_limit == -1)
- soft_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_MEMORY_SOFT_LIMIT,
- soft_limit) < 0)
- goto save_error;
- }
-
- if (swap_hard_limit) {
- if (swap_hard_limit == -1)
- swap_hard_limit = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT,
- swap_hard_limit) < 0)
- goto save_error;
- }
+ PARSE_MEMTUNE_PARAM("hard-limit", hard_limit,
VIR_DOMAIN_MEMORY_HARD_LIMIT);
+ PARSE_MEMTUNE_PARAM("soft-limit", soft_limit,
VIR_DOMAIN_MEMORY_SOFT_LIMIT);
+ PARSE_MEMTUNE_PARAM("swap-hard-limit", swap_hard_limit,
+ VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT);
+ PARSE_MEMTUNE_PARAM("min-guarantee", min_guarantee,
+ VIR_DOMAIN_MEMORY_MIN_GUARANTEE);
- if (min_guarantee) {
- if (min_guarantee == -1)
- min_guarantee = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_MEMORY_MIN_GUARANTEE,
- min_guarantee) < 0)
- goto save_error;
- }
+#undef PARSE_MEMTUNE_PARAM
Um ...
if (nparams == 0) {
/* get the number of memory parameters */
@@ -8390,6 +8367,7 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
+#undef PARSE_MEMTUNE_PARAM
... why are you undefing it twice?
virTypedParamsFree(params, nparams);
virDomainFree(dom);
return ret;
Also the virsh man page would benefit from adding a statement that -1
should be used as unlimited.
Peter