[libvirt] [PATCH 0/8] virsh: kill blockJobImpl
Move the code to appropriate parts to sanitize control flow. Peter Krempa (8): virsh: blockjob: Extract block job info code into a separate function virsh: cmdBlockJob: Switch to declarative flag interlocking virsh: blockjob: Split out vshBlockJobSetSpeed from blockJobImpl virsh: man: Add missing dashes for blockjob --bandwidth virsh: block job: Support --bytes and scaled integers when setting speed virsh: block job: separate abort from blockJobImpl virsh: Split out block pull implementation from blockJobImpl virsh: Kill blockJobImpl by moving the final impl into cmdBlockCommit tools/virsh-domain.c | 314 +++++++++++++++++++++++++++++---------------------- tools/virsh.pod | 14 ++- 2 files changed, 189 insertions(+), 139 deletions(-) -- 2.3.5
cmdBlockJob will be converted to a hub that will call into the individual executor functions. --- tools/virsh-domain.c | 93 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 56 insertions(+), 37 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4b627e1..01c6b9e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2457,47 +2457,19 @@ vshDomainBlockJobToString(int type) return str ? _(str) : _("Unknown job"); } + static bool -cmdBlockJob(vshControl *ctl, const vshCmd *cmd) +vshBlockJobInfo(vshControl *ctl, + virDomainPtr dom, + const char *path, + bool raw, + bool bytes) { virDomainBlockJobInfo info; - bool ret = false; - int rc = -1; - bool raw = vshCommandOptBool(cmd, "raw"); - bool bytes = vshCommandOptBool(cmd, "bytes"); - bool abortMode = (vshCommandOptBool(cmd, "abort") || - vshCommandOptBool(cmd, "async") || - vshCommandOptBool(cmd, "pivot")); - bool infoMode = vshCommandOptBool(cmd, "info") || raw; - bool bandwidth = vshCommandOptBool(cmd, "bandwidth"); - virDomainPtr dom = NULL; - const char *path; - unsigned int flags = 0; unsigned long long speed; - - if (abortMode + infoMode + bandwidth > 1) { - vshError(ctl, "%s", - _("conflict between abort, info, and bandwidth modes")); - return false; - } - /* XXX also support --bytes with bandwidth mode */ - if (bytes && (abortMode || bandwidth)) { - vshError(ctl, "%s", _("--bytes requires info mode")); - return false; - } - - if (abortMode) - return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); - if (bandwidth) - return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL); - - /* Everything below here is for --info mode */ - if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; - - /* XXX Allow path to be optional to list info on all devices at once */ - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) - goto cleanup; + unsigned int flags = 0; + bool ret = false; + int rc; /* If bytes were requested, or if raw mode is not forcing a MiB/s * query and cache can't prove failure, then query bytes/sec. */ @@ -2562,7 +2534,54 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) } vshPrint(ctl, "\n"); } + ret = true; + + cleanup: + return ret; +} + + +static bool +cmdBlockJob(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + bool raw = vshCommandOptBool(cmd, "raw"); + bool bytes = vshCommandOptBool(cmd, "bytes"); + bool abortMode = (vshCommandOptBool(cmd, "abort") || + vshCommandOptBool(cmd, "async") || + vshCommandOptBool(cmd, "pivot")); + bool infoMode = vshCommandOptBool(cmd, "info") || raw; + bool bandwidth = vshCommandOptBool(cmd, "bandwidth"); + virDomainPtr dom = NULL; + const char *path; + + if (abortMode + infoMode + bandwidth > 1) { + vshError(ctl, "%s", + _("conflict between abort, info, and bandwidth modes")); + return false; + } + /* XXX also support --bytes with bandwidth mode */ + if (bytes && (abortMode || bandwidth)) { + vshError(ctl, "%s", _("--bytes requires info mode")); + return false; + } + + if (abortMode) + return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); + if (bandwidth) + return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL); + + /* Everything below here is for --info mode */ + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + goto cleanup; + + /* XXX Allow path to be optional to list info on all devices at once */ + if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) + goto cleanup; + + ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); + cleanup: if (dom) virDomainFree(dom); -- 2.3.5
On 04/30/2015 10:45 AM, Peter Krempa wrote:
cmdBlockJob will be converted to a hub that will call into the individual executor functions. --- tools/virsh-domain.c | 93 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 56 insertions(+), 37 deletions(-)
This doesn't compile for me... gcc (GCC) 4.9.2 20150212
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4b627e1..01c6b9e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2457,47 +2457,19 @@ vshDomainBlockJobToString(int type) return str ? _(str) : _("Unknown job"); }
+ static bool -cmdBlockJob(vshControl *ctl, const vshCmd *cmd) +vshBlockJobInfo(vshControl *ctl, + virDomainPtr dom, + const char *path, + bool raw, + bool bytes) { virDomainBlockJobInfo info; - bool ret = false; - int rc = -1;
^^^
- bool raw = vshCommandOptBool(cmd, "raw"); - bool bytes = vshCommandOptBool(cmd, "bytes"); - bool abortMode = (vshCommandOptBool(cmd, "abort") || - vshCommandOptBool(cmd, "async") || - vshCommandOptBool(cmd, "pivot")); - bool infoMode = vshCommandOptBool(cmd, "info") || raw; - bool bandwidth = vshCommandOptBool(cmd, "bandwidth"); - virDomainPtr dom = NULL; - const char *path; - unsigned int flags = 0; unsigned long long speed; - - if (abortMode + infoMode + bandwidth > 1) { - vshError(ctl, "%s", - _("conflict between abort, info, and bandwidth modes")); - return false; - } - /* XXX also support --bytes with bandwidth mode */ - if (bytes && (abortMode || bandwidth)) { - vshError(ctl, "%s", _("--bytes requires info mode")); - return false; - } - - if (abortMode) - return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); - if (bandwidth) - return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL); - - /* Everything below here is for --info mode */ - if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; - - /* XXX Allow path to be optional to list info on all devices at once */ - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) - goto cleanup; + unsigned int flags = 0; + bool ret = false; + int rc;
^^^ virsh-domain.c: In function 'cmdBlockJob': virsh-domain.c:2515:8: error: 'rc' may be used uninitialized in this function [-Werror=maybe-uninitialized] if (rc == 0) { ^ virsh-domain.c:2472:9: note: 'rc' was declared here int rc; ^ virsh-domain.c:2532:13: error: 'speed' may be used uninitialized in this function [-Werror=maybe-uninitialized] vshPrint(ctl, _(" Bandwidth limit: %llu bytes/s (%-.3lf %s/s)"), ^ virsh-domain.c:2469:24: note: 'speed' was declared here unsigned long long speed; ^ Initializing rc to -1 resolves the issue ACK with the adjustment. John
/* If bytes were requested, or if raw mode is not forcing a MiB/s * query and cache can't prove failure, then query bytes/sec. */ @@ -2562,7 +2534,54 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) } vshPrint(ctl, "\n"); } + ret = true; + + cleanup: + return ret; +} + + +static bool +cmdBlockJob(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + bool raw = vshCommandOptBool(cmd, "raw"); + bool bytes = vshCommandOptBool(cmd, "bytes"); + bool abortMode = (vshCommandOptBool(cmd, "abort") || + vshCommandOptBool(cmd, "async") || + vshCommandOptBool(cmd, "pivot")); + bool infoMode = vshCommandOptBool(cmd, "info") || raw; + bool bandwidth = vshCommandOptBool(cmd, "bandwidth"); + virDomainPtr dom = NULL; + const char *path; + + if (abortMode + infoMode + bandwidth > 1) { + vshError(ctl, "%s", + _("conflict between abort, info, and bandwidth modes")); + return false; + } + /* XXX also support --bytes with bandwidth mode */ + if (bytes && (abortMode || bandwidth)) { + vshError(ctl, "%s", _("--bytes requires info mode")); + return false; + } + + if (abortMode) + return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); + if (bandwidth) + return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL); + + /* Everything below here is for --info mode */ + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + goto cleanup; + + /* XXX Allow path to be optional to list info on all devices at once */ + if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) + goto cleanup; + + ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); + cleanup: if (dom) virDomainFree(dom);
Use the VSH_EXCLUSIVE_OPTIONS_VAR to interlock incompatible options --- tools/virsh-domain.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 01c6b9e..720bc68 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2548,26 +2548,31 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) bool ret = false; bool raw = vshCommandOptBool(cmd, "raw"); bool bytes = vshCommandOptBool(cmd, "bytes"); - bool abortMode = (vshCommandOptBool(cmd, "abort") || - vshCommandOptBool(cmd, "async") || - vshCommandOptBool(cmd, "pivot")); - bool infoMode = vshCommandOptBool(cmd, "info") || raw; + bool abort = vshCommandOptBool(cmd, "abort"); + bool pivot = vshCommandOptBool(cmd, "pivot"); + bool async = vshCommandOptBool(cmd, "async"); + bool info = vshCommandOptBool(cmd, "info"); bool bandwidth = vshCommandOptBool(cmd, "bandwidth"); virDomainPtr dom = NULL; const char *path; - if (abortMode + infoMode + bandwidth > 1) { - vshError(ctl, "%s", - _("conflict between abort, info, and bandwidth modes")); - return false; - } + VSH_EXCLUSIVE_OPTIONS_VAR(raw, abort); + VSH_EXCLUSIVE_OPTIONS_VAR(raw, pivot); + VSH_EXCLUSIVE_OPTIONS_VAR(raw, async); + VSH_EXCLUSIVE_OPTIONS_VAR(raw, bandwidth); + + VSH_EXCLUSIVE_OPTIONS_VAR(info, abort); + VSH_EXCLUSIVE_OPTIONS_VAR(info, pivot); + VSH_EXCLUSIVE_OPTIONS_VAR(info, async); + VSH_EXCLUSIVE_OPTIONS_VAR(info, bandwidth); + + VSH_EXCLUSIVE_OPTIONS_VAR(bytes, abort); + VSH_EXCLUSIVE_OPTIONS_VAR(bytes, pivot); + VSH_EXCLUSIVE_OPTIONS_VAR(bytes, async); /* XXX also support --bytes with bandwidth mode */ - if (bytes && (abortMode || bandwidth)) { - vshError(ctl, "%s", _("--bytes requires info mode")); - return false; - } + VSH_EXCLUSIVE_OPTIONS_VAR(bytes, bandwidth); - if (abortMode) + if (abort || pivot || async) return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); if (bandwidth) return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL); -- 2.3.5
On 04/30/2015 10:45 AM, Peter Krempa wrote:
Use the VSH_EXCLUSIVE_OPTIONS_VAR to interlock incompatible options --- tools/virsh-domain.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-)
ACK John
--- tools/virsh-domain.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 720bc68..0d0e39e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1667,7 +1667,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) typedef enum { VSH_CMD_BLOCK_JOB_ABORT, - VSH_CMD_BLOCK_JOB_SPEED, VSH_CMD_BLOCK_JOB_PULL, VSH_CMD_BLOCK_JOB_COMMIT, } vshCmdBlockJobMode; @@ -1704,10 +1703,6 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, if (virDomainBlockJobAbort(dom, path, flags) < 0) goto cleanup; break; - case VSH_CMD_BLOCK_JOB_SPEED: - if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) - goto cleanup; - break; case VSH_CMD_BLOCK_JOB_PULL: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) goto cleanup; @@ -2543,6 +2538,26 @@ vshBlockJobInfo(vshControl *ctl, static bool +vshBlockJobSetSpeed(vshControl *ctl, + const vshCmd *cmd, + virDomainPtr dom, + const char *path) +{ + unsigned long bandwidth; + + if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { + vshError(ctl, "%s", _("bandwidth must be a number")); + return false; + } + + if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) + return false; + + return true; +} + + +static bool cmdBlockJob(vshControl *ctl, const vshCmd *cmd) { bool ret = false; @@ -2574,10 +2589,7 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) if (abort || pivot || async) return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); - if (bandwidth) - return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL); - /* Everything below here is for --info mode */ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) goto cleanup; @@ -2585,7 +2597,10 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) goto cleanup; - ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); + if (bandwidth) + ret = vshBlockJobSetSpeed(ctl, cmd, dom, path); + else + ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); cleanup: if (dom) -- 2.3.5
On 04/30/2015 10:45 AM, Peter Krempa wrote:
--- tools/virsh-domain.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 720bc68..0d0e39e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1667,7 +1667,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
typedef enum { VSH_CMD_BLOCK_JOB_ABORT, - VSH_CMD_BLOCK_JOB_SPEED, VSH_CMD_BLOCK_JOB_PULL, VSH_CMD_BLOCK_JOB_COMMIT, } vshCmdBlockJobMode; @@ -1704,10 +1703,6 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, if (virDomainBlockJobAbort(dom, path, flags) < 0) goto cleanup; break; - case VSH_CMD_BLOCK_JOB_SPEED: - if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) - goto cleanup; - break; case VSH_CMD_BLOCK_JOB_PULL: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) goto cleanup; @@ -2543,6 +2538,26 @@ vshBlockJobInfo(vshControl *ctl,
static bool +vshBlockJobSetSpeed(vshControl *ctl, + const vshCmd *cmd, + virDomainPtr dom, + const char *path) +{ + unsigned long bandwidth; + + if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { + vshError(ctl, "%s", _("bandwidth must be a number")); + return false; + } + + if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) + return false; + + return true; +} + + +static bool cmdBlockJob(vshControl *ctl, const vshCmd *cmd) { bool ret = false; @@ -2574,10 +2589,7 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
if (abort || pivot || async) return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); - if (bandwidth) - return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL);
- /* Everything below here is for --info mode */ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) goto cleanup;
@@ -2585,7 +2597,10 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) goto cleanup;
- ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); + if (bandwidth) + ret = vshBlockJobSetSpeed(ctl, cmd, dom, path);
Since we're getting bandwidth a few lines above - why not just pass that? Especially considering what you do in patch 5 which only allows using a scaled bandwidth for SetSpeed, but not Pull and Commit which patch 5 implies to me in the virsh.pod can be used "in general". If you pass the already parsed bandwidth here - then an ACK; otherwise, I'd have to get more explanation... John
+ else + ret = vshBlockJobInfo(ctl, dom, path, raw, bytes);
cleanup: if (dom)
On 05/08/2015 07:25 AM, John Ferlan wrote:
On 04/30/2015 10:45 AM, Peter Krempa wrote:
--- tools/virsh-domain.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 720bc68..0d0e39e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1667,7 +1667,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
typedef enum { VSH_CMD_BLOCK_JOB_ABORT, - VSH_CMD_BLOCK_JOB_SPEED, VSH_CMD_BLOCK_JOB_PULL, VSH_CMD_BLOCK_JOB_COMMIT, } vshCmdBlockJobMode; @@ -1704,10 +1703,6 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, if (virDomainBlockJobAbort(dom, path, flags) < 0) goto cleanup; break; - case VSH_CMD_BLOCK_JOB_SPEED: - if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) - goto cleanup; - break; case VSH_CMD_BLOCK_JOB_PULL: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) goto cleanup; @@ -2543,6 +2538,26 @@ vshBlockJobInfo(vshControl *ctl,
static bool +vshBlockJobSetSpeed(vshControl *ctl, + const vshCmd *cmd, + virDomainPtr dom, + const char *path) +{ + unsigned long bandwidth; + + if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { + vshError(ctl, "%s", _("bandwidth must be a number")); + return false; + } + + if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) + return false; + + return true; +} + + +static bool cmdBlockJob(vshControl *ctl, const vshCmd *cmd) { bool ret = false; @@ -2574,10 +2589,7 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd)
if (abort || pivot || async) return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); - if (bandwidth) - return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_SPEED, NULL);
- /* Everything below here is for --info mode */ if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) goto cleanup;
@@ -2585,7 +2597,10 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) goto cleanup;
- ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); + if (bandwidth) + ret = vshBlockJobSetSpeed(ctl, cmd, dom, path);
Since we're getting bandwidth a few lines above - why not just pass that? Especially considering what you do in patch 5 which only allows using a scaled bandwidth for SetSpeed, but not Pull and Commit which patch 5 implies to me in the virsh.pod can be used "in general".
If you pass the already parsed bandwidth here - then an ACK; otherwise, I'd have to get more explanation...
John
Ug - didn't read and consider 7/8 and 8/8 closely enough <sigh>, so let me retract the if you pass and just ACK John
+ else + ret = vshBlockJobInfo(ctl, dom, path, raw, bytes);
cleanup: if (dom)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
--- tools/virsh.pod | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index f8496f3..f576fb0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1124,10 +1124,10 @@ I<--config> or I<--current> can be specified. If no flag is specified, behavior is different depending on hypervisor. =item B<blockjob> I<domain> I<path> { [I<--abort>] [I<--async>] [I<--pivot>] | -[I<--info>] [I<--raw>] [I<--bytes>] | [I<bandwidth>] } +[I<--info>] [I<--raw>] [I<--bytes>] | [I<--bandwidth>] } Manage active block operations. There are three mutually-exclusive modes: -I<--info>, I<bandwidth>, and I<--abort>. I<--async> and I<--pivot> imply +I<--info>, I<--bandwidth>, and I<--abort>. I<--async> and I<--pivot> imply abort mode; I<--raw> implies info mode; and if no mode was given, I<--info> mode is assumed. @@ -1151,7 +1151,7 @@ not supply bytes/s resolution; when omitting the flag, raw output is listed in MiB/s and human-readable output automatically selects the best resolution supported by the server. -I<bandwidth> can be used to set bandwidth limit for the active job. +I<--bandwidth> can be used to set bandwidth limit for the active job. Specifying a negative value is interpreted as an unsigned long long value or essentially unlimited. The hypervisor can choose whether to reject the value or convert it to the maximum value allowed. -- 2.3.5
On 04/30/2015 08:45 AM, Peter Krempa wrote:
--- tools/virsh.pod | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index f8496f3..f576fb0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1124,10 +1124,10 @@ I<--config> or I<--current> can be specified. If no flag is specified, behavior is different depending on hypervisor.
=item B<blockjob> I<domain> I<path> { [I<--abort>] [I<--async>] [I<--pivot>] | -[I<--info>] [I<--raw>] [I<--bytes>] | [I<bandwidth>] } +[I<--info>] [I<--raw>] [I<--bytes>] | [I<--bandwidth>] }
Actually, --bandwidth takes an argument, so this could probably be something like: | [[I<--bandwidth>] I<bandwidth>] } (look at how freepages lists --cellno for comparison).
Manage active block operations. There are three mutually-exclusive modes: -I<--info>, I<bandwidth>, and I<--abort>. I<--async> and I<--pivot> imply +I<--info>, I<--bandwidth>, and I<--abort>. I<--async> and I<--pivot> imply abort mode; I<--raw> implies info mode; and if no mode was given, I<--info> mode is assumed.
This one is fine.
@@ -1151,7 +1151,7 @@ not supply bytes/s resolution; when omitting the flag, raw output is listed in MiB/s and human-readable output automatically selects the best resolution supported by the server.
-I<bandwidth> can be used to set bandwidth limit for the active job. +I<--bandwidth> can be used to set bandwidth limit for the active job.
Since the --bandwidth option is the only optional parameter that takes an argument, and since we declared the modes to be mutually exclusive, it will be identical behavior whether the user does 'blockjob $dom $path $number' or 'blockjob $dom $path --bandwidth $number'. I'm not sure how best to word that here; maybe: A I<--bandwidth number> can be used to set ...
Specifying a negative value is interpreted as an unsigned long long value or essentially unlimited. The hypervisor can choose whether to reject the value or convert it to the maximum value allowed.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 04/30/2015 10:45 AM, Peter Krempa wrote:
--- tools/virsh.pod | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index f8496f3..f576fb0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1124,10 +1124,10 @@ I<--config> or I<--current> can be specified. If no flag is specified, behavior is different depending on hypervisor.
=item B<blockjob> I<domain> I<path> { [I<--abort>] [I<--async>] [I<--pivot>] | -[I<--info>] [I<--raw>] [I<--bytes>] | [I<bandwidth>] } +[I<--info>] [I<--raw>] [I<--bytes>] | [I<--bandwidth>] }
Manage active block operations. There are three mutually-exclusive modes: -I<--info>, I<bandwidth>, and I<--abort>. I<--async> and I<--pivot> imply +I<--info>, I<--bandwidth>, and I<--abort>. I<--async> and I<--pivot> imply abort mode; I<--raw> implies info mode; and if no mode was given, I<--info> mode is assumed.
@@ -1151,7 +1151,7 @@ not supply bytes/s resolution; when omitting the flag, raw output is listed in MiB/s and human-readable output automatically selects the best resolution supported by the server.
-I<bandwidth> can be used to set bandwidth limit for the active job. +I<--bandwidth> can be used to set bandwidth limit for the active job. Specifying a negative value is interpreted as an unsigned long long value or essentially unlimited. The hypervisor can choose whether to reject the value or convert it to the maximum value allowed.
To add to what Eric already stated - there's multiple places with that have [[I<--option>] {B|I}<argument>], whether you decide to go with I or B is your choice... Also further patches only accept a scaled bandwidth for SetSpeed, but not for Pull, Copy, Commit (hence my 3/8 comment regarding passing bandwidth)... ACK - with adjustments mentioned. John
Allow specifying sizes in bytes or as scaled integers for user convenience. --- tools/virsh-domain.c | 31 ++++++++++++++++++++++++------- tools/virsh.pod | 10 +++++++--- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0d0e39e..fef3918 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2541,16 +2541,35 @@ static bool vshBlockJobSetSpeed(vshControl *ctl, const vshCmd *cmd, virDomainPtr dom, - const char *path) + const char *path, + bool bytes) { unsigned long bandwidth; + unsigned long long bw; + unsigned int flags = 0; + + if (bytes) + flags |= VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES; if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { - vshError(ctl, "%s", _("bandwidth must be a number")); - return false; + /* try to parse the number as scaled size */ + if (vshCommandOptScaledInt(cmd, "bandwidth", &bw, 1, ULLONG_MAX) < 0) { + vshError(ctl, "%s", _("bandwidth must be a number")); + return false; + } + + if (!bytes) + bw /= 1024 * 1024; + + if (bw > ULONG_MAX) { + vshError(ctl, _("bandwidth must be less than %lu"), ULONG_MAX); + return false; + } + + bandwidth = bw; } - if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) + if (virDomainBlockJobSetSpeed(dom, path, bandwidth, flags) < 0) return false; return true; @@ -2584,8 +2603,6 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(bytes, abort); VSH_EXCLUSIVE_OPTIONS_VAR(bytes, pivot); VSH_EXCLUSIVE_OPTIONS_VAR(bytes, async); - /* XXX also support --bytes with bandwidth mode */ - VSH_EXCLUSIVE_OPTIONS_VAR(bytes, bandwidth); if (abort || pivot || async) return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); @@ -2598,7 +2615,7 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (bandwidth) - ret = vshBlockJobSetSpeed(ctl, cmd, dom, path); + ret = vshBlockJobSetSpeed(ctl, cmd, dom, path, bytes); else ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); diff --git a/tools/virsh.pod b/tools/virsh.pod index f576fb0..85743c6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1151,10 +1151,14 @@ not supply bytes/s resolution; when omitting the flag, raw output is listed in MiB/s and human-readable output automatically selects the best resolution supported by the server. -I<--bandwidth> can be used to set bandwidth limit for the active job. -Specifying a negative value is interpreted as an unsigned long long +I<--bandwidth> can be used to set bandwidth limit for the active job in MiB/s. +If I<--bytes> is specified then the bandwidth value is interpreted in bytes/s. +Specifying a negative value is interpreted as an unsigned long value or essentially unlimited. The hypervisor can choose whether to -reject the value or convert it to the maximum value allowed. +reject the value or convert it to the maximum value allowed. Optionally a +scaled positive number may be used as bandwidth (see B<NOTES> above). Using +I<--bytes> with a scaled value allows to use finer granularity. Note that the +I<--bytes> may be unsupported by the hypervisor. =item B<blockresize> I<domain> I<path> I<size> -- 2.3.5
On 04/30/2015 10:45 AM, Peter Krempa wrote:
Allow specifying sizes in bytes or as scaled integers for user convenience. --- tools/virsh-domain.c | 31 ++++++++++++++++++++++++------- tools/virsh.pod | 10 +++++++--- 2 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0d0e39e..fef3918 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2541,16 +2541,35 @@ static bool vshBlockJobSetSpeed(vshControl *ctl, const vshCmd *cmd, virDomainPtr dom, - const char *path) + const char *path, + bool bytes) { unsigned long bandwidth; + unsigned long long bw; + unsigned int flags = 0; + + if (bytes) + flags |= VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES;
if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { - vshError(ctl, "%s", _("bandwidth must be a number")); - return false; + /* try to parse the number as scaled size */ + if (vshCommandOptScaledInt(cmd, "bandwidth", &bw, 1, ULLONG_MAX) < 0) { + vshError(ctl, "%s", _("bandwidth must be a number")); + return false; + } + + if (!bytes) + bw /= 1024 * 1024;
Or like other code with does the shifting by 20 bits...
+ + if (bw > ULONG_MAX) {
Coverity complains CONSTANT_EXPRESSION_RESULT (1) Event result_independent_of_operands: "bw > 18446744073709551615ULL /* 9223372036854775807L * 2UL + 1UL */" is always false regardless of the values of its operands. This occurs as the logical operand of if. However, why even make the check here? Since virsh.pod astutely points out "The hypervisor can choose whether to reject the value or convert it to the maximum value allowed." I see in qemuDomainBlockJobSetSpeed that bandwidth maximum is checked.
+ vshError(ctl, _("bandwidth must be less than %lu"), ULONG_MAX); + return false; + } + + bandwidth = bw; }
- if (virDomainBlockJobSetSpeed(dom, path, bandwidth, 0) < 0) + if (virDomainBlockJobSetSpeed(dom, path, bandwidth, flags) < 0) return false;
return true; @@ -2584,8 +2603,6 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(bytes, abort); VSH_EXCLUSIVE_OPTIONS_VAR(bytes, pivot); VSH_EXCLUSIVE_OPTIONS_VAR(bytes, async); - /* XXX also support --bytes with bandwidth mode */ - VSH_EXCLUSIVE_OPTIONS_VAR(bytes, bandwidth);
if (abort || pivot || async) return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); @@ -2598,7 +2615,7 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) goto cleanup;
if (bandwidth) - ret = vshBlockJobSetSpeed(ctl, cmd, dom, path); + ret = vshBlockJobSetSpeed(ctl, cmd, dom, path, bytes); else ret = vshBlockJobInfo(ctl, dom, path, raw, bytes);
diff --git a/tools/virsh.pod b/tools/virsh.pod index f576fb0..85743c6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1151,10 +1151,14 @@ not supply bytes/s resolution; when omitting the flag, raw output is listed in MiB/s and human-readable output automatically selects the best resolution supported by the server.
-I<--bandwidth> can be used to set bandwidth limit for the active job. -Specifying a negative value is interpreted as an unsigned long long +I<--bandwidth> can be used to set bandwidth limit for the active job in MiB/s. +If I<--bytes> is specified then the bandwidth value is interpreted in bytes/s. +Specifying a negative value is interpreted as an unsigned long value or essentially unlimited. The hypervisor can choose whether to -reject the value or convert it to the maximum value allowed. +reject the value or convert it to the maximum value allowed. Optionally a +scaled positive number may be used as bandwidth (see B<NOTES> above). Using +I<--bytes> with a scaled value allows to use finer granularity. Note that the +I<--bytes> may be unsupported by the hypervisor.
You'll have adjustments here too based on 4/8 changes... Also depending on your response to 3/8 - you may need to change the discussion to only support scaled values for SetSpeed ACK with the ULONG_MAX check removed and I trust there will be an adjustment to the text here based on how bandwidth can be used... John
=item B<blockresize> I<domain> I<path> I<size>
--- tools/virsh-domain.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fef3918..7afa319 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1666,7 +1666,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) } typedef enum { - VSH_CMD_BLOCK_JOB_ABORT, VSH_CMD_BLOCK_JOB_PULL, VSH_CMD_BLOCK_JOB_COMMIT, } vshCmdBlockJobMode; @@ -1695,14 +1694,6 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, } switch (mode) { - case VSH_CMD_BLOCK_JOB_ABORT: - if (vshCommandOptBool(cmd, "async")) - flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC; - if (vshCommandOptBool(cmd, "pivot")) - flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT; - if (virDomainBlockJobAbort(dom, path, flags) < 0) - goto cleanup; - break; case VSH_CMD_BLOCK_JOB_PULL: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) goto cleanup; @@ -2577,6 +2568,26 @@ vshBlockJobSetSpeed(vshControl *ctl, static bool +vshBlockJobAbort(virDomainPtr dom, + const char *path, + bool pivot, + bool async) +{ + unsigned int flags = 0; + + if (async) + flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC; + if (pivot) + flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT; + + if (virDomainBlockJobAbort(dom, path, flags) < 0) + return false; + + return true; +} + + +static bool cmdBlockJob(vshControl *ctl, const vshCmd *cmd) { bool ret = false; @@ -2604,9 +2615,6 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(bytes, pivot); VSH_EXCLUSIVE_OPTIONS_VAR(bytes, async); - if (abort || pivot || async) - return blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_ABORT, NULL); - if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) goto cleanup; @@ -2616,6 +2624,8 @@ cmdBlockJob(vshControl *ctl, const vshCmd *cmd) if (bandwidth) ret = vshBlockJobSetSpeed(ctl, cmd, dom, path, bytes); + else if (abort || pivot || async) + ret = vshBlockJobAbort(dom, path, pivot, async); else ret = vshBlockJobInfo(ctl, dom, path, raw, bytes); -- 2.3.5
--- tools/virsh-domain.c | 50 ++++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 7afa319..081adfd 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1666,7 +1666,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) } typedef enum { - VSH_CMD_BLOCK_JOB_PULL, VSH_CMD_BLOCK_JOB_COMMIT, } vshCmdBlockJobMode; @@ -1694,21 +1693,6 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, } switch (mode) { - case VSH_CMD_BLOCK_JOB_PULL: - if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) - goto cleanup; - if (vshCommandOptBool(cmd, "keep-relative")) - flags |= VIR_DOMAIN_BLOCK_REBASE_RELATIVE; - - if (base || flags) { - if (virDomainBlockRebase(dom, path, base, bandwidth, flags) < 0) - goto cleanup; - } else { - if (virDomainBlockPull(dom, path, bandwidth, 0) < 0) - goto cleanup; - } - - break; case VSH_CMD_BLOCK_JOB_COMMIT: if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0 || vshCommandOptStringReq(ctl, cmd, "top", &top) < 0) @@ -2704,16 +2688,31 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd) struct timeval start; struct timeval curr; const char *path = NULL; + const char *base = NULL; + unsigned long bandwidth = 0; bool quit = false; int abort_flags = 0; int status = -1; int cb_id = -1; + unsigned int flags = 0; + + if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) + return false; + + if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { + vshError(ctl, "%s", _("bandwidth must be a number")); + return false; + } + + if (vshCommandOptBool(cmd, "keep-relative")) + flags |= VIR_DOMAIN_BLOCK_REBASE_RELATIVE; if (blocking) { if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) - return false; if (vshCommandOptBool(cmd, "async")) abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC; @@ -2733,6 +2732,9 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd) return false; } + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + virConnectDomainEventGenericCallback cb = VIR_DOMAIN_EVENT_CALLBACK(vshBlockJobStatusHandler); @@ -2744,8 +2746,13 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd) NULL)) < 0) vshResetLibvirtError(); - if (!blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_PULL, &dom)) - goto cleanup; + if (base || flags) { + if (virDomainBlockRebase(dom, path, base, bandwidth, flags) < 0) + goto cleanup; + } else { + if (virDomainBlockPull(dom, path, bandwidth, 0) < 0) + goto cleanup; + } if (!blocking) { vshPrint(ctl, "%s", _("Block Pull started")); @@ -2803,8 +2810,7 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: - if (dom) - virDomainFree(dom); + virDomainFree(dom); if (blocking) sigaction(SIGINT, &old_sig_action, NULL); if (cb_id >= 0) -- 2.3.5
On 04/30/2015 10:45 AM, Peter Krempa wrote:
--- tools/virsh-domain.c | 50 ++++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 22 deletions(-)
ACK, although should/could bandwidth be a scaled int too? As a followup? Hence my confusion back around patch 3-5... John
Final cleanup to get rid of the hub function. --- tools/virsh-domain.c | 96 +++++++++++++++++++--------------------------------- 1 file changed, 35 insertions(+), 61 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 081adfd..e2a38e7 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1665,62 +1665,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) goto cleanup; } -typedef enum { - VSH_CMD_BLOCK_JOB_COMMIT, -} vshCmdBlockJobMode; - -static bool -blockJobImpl(vshControl *ctl, const vshCmd *cmd, - vshCmdBlockJobMode mode, virDomainPtr *pdom) -{ - virDomainPtr dom = NULL; - const char *path; - unsigned long bandwidth = 0; - bool ret = false; - const char *base = NULL; - const char *top = NULL; - unsigned int flags = 0; - - if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) - goto cleanup; - - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) - goto cleanup; - - if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { - vshError(ctl, "%s", _("bandwidth must be a number")); - goto cleanup; - } - - switch (mode) { - case VSH_CMD_BLOCK_JOB_COMMIT: - if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0 || - vshCommandOptStringReq(ctl, cmd, "top", &top) < 0) - goto cleanup; - if (vshCommandOptBool(cmd, "shallow")) - flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW; - if (vshCommandOptBool(cmd, "delete")) - flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE; - if (vshCommandOptBool(cmd, "active") || - vshCommandOptBool(cmd, "pivot") || - vshCommandOptBool(cmd, "keep-overlay")) - flags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE; - if (vshCommandOptBool(cmd, "keep-relative")) - flags |= VIR_DOMAIN_BLOCK_COMMIT_RELATIVE; - if (virDomainBlockCommit(dom, path, base, top, bandwidth, flags) < 0) - goto cleanup; - break; - } - - ret = true; - - cleanup: - if (pdom && ret) - *pdom = dom; - else if (dom) - virDomainFree(dom); - return ret; -} static void vshPrintJobProgress(const char *label, unsigned long long remaining, @@ -1867,10 +1811,40 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) struct timeval start; struct timeval curr; const char *path = NULL; + const char *base = NULL; + const char *top = NULL; bool quit = false; int abort_flags = 0; int status = -1; int cb_id = -1; + unsigned int flags = 0; + unsigned long bandwidth = 0; + + if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "base", &base) < 0) + return false; + + if (vshCommandOptStringReq(ctl, cmd, "top", &top) < 0) + return false; + + if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { + vshError(ctl, "%s", _("bandwidth must be a number")); + return false; + } + + if (vshCommandOptBool(cmd, "shallow")) + flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW; + + if (vshCommandOptBool(cmd, "delete")) + flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE; + + if (active) + flags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE; + + if (vshCommandOptBool(cmd, "keep-relative")) + flags |= VIR_DOMAIN_BLOCK_COMMIT_RELATIVE; blocking |= vshCommandOptBool(cmd, "timeout") || pivot || finish; if (blocking) { @@ -1880,8 +1854,6 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) } if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) return false; - if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) - return false; if (vshCommandOptBool(cmd, "async")) abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC; @@ -1900,6 +1872,9 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) return false; } + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + virConnectDomainEventGenericCallback cb = VIR_DOMAIN_EVENT_CALLBACK(vshBlockJobStatusHandler); @@ -1911,7 +1886,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) NULL)) < 0) vshResetLibvirtError(); - if (!blockJobImpl(ctl, cmd, VSH_CMD_BLOCK_JOB_COMMIT, &dom)) + if (virDomainBlockCommit(dom, path, base, top, bandwidth, flags) < 0) goto cleanup; if (!blocking) { @@ -1993,8 +1968,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: - if (dom) - virDomainFree(dom); + virDomainFree(dom); if (blocking) sigaction(SIGINT, &old_sig_action, NULL); if (cb_id >= 0) -- 2.3.5
On 04/30/2015 10:45 AM, Peter Krempa wrote:
Final cleanup to get rid of the hub function. --- tools/virsh-domain.c | 96 +++++++++++++++++++--------------------------------- 1 file changed, 35 insertions(+), 61 deletions(-)
ACK - similar bandwidth comment from 7/8 John
participants (3)
-
Eric Blake -
John Ferlan -
Peter Krempa