On 11/15/2011 02:02 AM, Lei Li wrote:
> Support virsh command blkdeviotune. Can set or query a block disk
> I/O throttle setting.
>
> Signed-off-by: Lei Li <lilei@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
> ---
> tools/virsh.c | 240 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/virsh.pod | 23 +++++
> 2 files changed, 263 insertions(+), 0 deletions(-)
>
>
> /*
> + * "blkdeviotune" command
> + */
> +static const vshCmdInfo info_blkdeviotune[] = {
> + {"help", N_("Set or query a block disk I/O throttle setting.")},
> + {"desc", N_("Set or query a block disk I/O throttle setting.\n" \
> + " To query the block disk I/O throttle setting use the following" \
> + " command: \n\n" \
> + " virsh # blkdeviotune <domain> <device>")},
That's a bit long; most of our help text is one line, with the long
version left for the man page (virsh.pod).
> + {NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_blkdeviotune[] = {
> + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
> + {"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("block device")},
> + {"total_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("total throughput limit in bytes per second")},
I line wrapped these.
> +
> + for (i = 0; i < nparams; i++) {
> + switch(params[i].type) {
> + case VIR_TYPED_PARAM_INT:
> + vshPrint(ctl, "%-15s: %d\n", params[i].field,
> + params[i].value.i);
> + break;
This feels like code duplication that we should factor out in a future
patch, but it doesn't matter for this one.
> +
> + virDomainFree(dom);
> + return true;
Memory leak - you need to free params.
> + } else {
> + /* Set the block I/O throttle, match by opt since parameters can be 0 */
> + params = vshCalloc(ctl, nparams, sizeof(*params));
> + i = 0;
> +
> + if ((virDomainSetBlockIoTune(dom, disk, params, nparams, flags)) != 0) {
> + vshError(ctl, "%s",
> + _("Unable to change block I/O throttle"));
> + goto out;
> + } else {
> + virDomainFree(dom);
> + return true;
Same leak.
> @@ -14023,6 +14262,7 @@ static const vshCmdDef domManagementCmds[] = {
> {"blkiotune", cmdBlkiotune, opts_blkiotune, info_blkiotune, 0},
> {"blockpull", cmdBlockPull, opts_block_pull, info_block_pull, 0},
> {"blockjob", cmdBlockJob, opts_block_job, info_block_job, 0},
> + {"blkdeviotune", cmdBlkdeviotune, opts_blkdeviotune, info_blkdeviotune, 0},
Sorting.
> +++ b/tools/virsh.pod
> @@ -572,6 +572,29 @@ operation can be checked with B<blockjob>.
> I<path> specifies fully-qualified path of the disk.
> I<bandwidth> specifies copying bandwidth limit in Mbps.
>
> +=item B<blkdeviotune> I<domain> I<device> [[I<--total_bytes_sec> B<total_bytes_sec>]
> +| [[I<--read_bytes_sec> B<read_bytes_sec>] [I<--write_bytes_sec> B<write_bytes_sec>]]
> +[[I<--total_iops_sec> B<total_iops_sec>] | [[I<--read_iops_sec> B<read_iops_sec>]
> +[I<--write_iops_sec> B<write_iops_sec>]] [[I<--config>] [I<--live>] | [I<--current>]]
This reads long; I shortened it to I<op> instead of I<--op> B<op>.
> +
> +Set or query the block disk io limits settting.
s/settting/setting/
> +I<path> specifies block disk name.
I copied text from <domblkstat> here.
> +I<--total_bytes_sec> specifies total throughput limit in bytes per second.
> +I<--read_bytes_sec> specifies read throughput limit in bytes per second.
> +I<--write_bytes_sec> specifies write throughput limit in bytes per second.
> +I<--total_iops_sec> specifies total I/O operations limit per second.
> +I<--read_iops_sec> specifies read I/O operations limit per second.
> +I<--write_iops_sec> specifies write I/O operations limit per second.
Needs to mention the restriction on total vs. read/write, also on the
handling of 0: it is necessary to specify at least one flag as explicit
0 to clear limits, otherwise you are just querying limits; and per my
comments in 4/8, the current behavior clears the remaining 5 limits when
one limit is set, although we may find it desirable to instead leave
limits unchanged if they are not specified.
> +
> +If I<--live> is specified, affect a running guest.
> +If I<--config> is specified, affect the next boot of a persistent guest.
> +If I<--current> is specified, affect the current guest state.
> +Both I<--live> and I<--current> flags may be given, but I<--current> is
> +exclusive. If no flag is specified, behavior is different depending
> +on hypervisor.
> +
> +If no limit is specified, it will query current I/O limits setting.
This line belongs earlier.
Here's what I'm squashing:
diff --git i/tools/virsh.c w/tools/virsh.c
index eb8c0b6..ea5a267 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -6079,23 +6079,26 @@ cmdNetworkAutostart(vshControl *ctl, const
vshCmd *cmd)
* "blkdeviotune" command
*/
static const vshCmdInfo info_blkdeviotune[] = {
- {"help", N_("Set or query a block disk I/O throttle setting.")},
- {"desc", N_("Set or query a block disk I/O throttle setting.\n" \
- " To query the block disk I/O throttle setting use
the following" \
- " command: \n\n" \
- " virsh # blkdeviotune <domain> <device>")},
+ {"help", N_("Set or query a block device I/O tuning parameters.")},
+ {"desc", N_("Set or query disk I/O parameters such as block
throttling.")},
{NULL, NULL}
};
static const vshCmdOptDef opts_blkdeviotune[] = {
{"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
{"device", VSH_OT_DATA, VSH_OFLAG_REQ, N_("block device")},
- {"total_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("total
throughput limit in bytes per second")},
- {"read_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("read throughput
limit in bytes per second")},
- {"write_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("write
throughput limit in bytes per second")},
- {"total_iops_sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("total I/O
operations limit per second")},
- {"read_iops_sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("read I/O
operations limit per second")},
- {"write_iops_sec", VSH_OT_INT, VSH_OFLAG_NONE, N_("write I/O
operations limit per second")},
+ {"total_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE,
+ N_("total throughput limit in bytes per second")},
+ {"read_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE,
+ N_("read throughput limit in bytes per second")},
+ {"write_bytes_sec", VSH_OT_INT, VSH_OFLAG_NONE,
+ N_("write throughput limit in bytes per second")},
+ {"total_iops_sec", VSH_OT_INT, VSH_OFLAG_NONE,
+ N_("total I/O operations limit per second")},
+ {"read_iops_sec", VSH_OT_INT, VSH_OFLAG_NONE,
+ N_("read I/O operations limit per second")},
+ {"write_iops_sec", VSH_OT_INT, VSH_OFLAG_NONE,
+ N_("write I/O operations limit per second")},
{"config", VSH_OT_BOOL, 0, N_("affect next boot")},
{"live", VSH_OT_BOOL, 0, N_("affect running domain")},
{"current", VSH_OT_BOOL, 0, N_("affect current domain")},
@@ -6116,6 +6119,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
int current = vshCommandOptBool(cmd, "current");
int config = vshCommandOptBool(cmd, "config");
int live = vshCommandOptBool(cmd, "live");
+ bool ret = false;
if (current) {
if (live || config) {
@@ -6131,18 +6135,18 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if (!vshConnectionUsability(ctl, ctl->conn))
- goto out;
+ goto cleanup;
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
- goto out;
+ goto cleanup;
if (vshCommandOptString(cmd, "device", &disk) < 0)
- goto out;
+ goto cleanup;
if ((rv = vshCommandOptULongLong(cmd, "total_bytes_sec",
&total_bytes_sec)) < 0) {
vshError(ctl, "%s",
_("Unable to parse integer parameter"));
- goto out;
+ goto cleanup;
} else if (rv > 0) {
nparams++;
}
@@ -6150,7 +6154,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if ((rv = vshCommandOptULongLong(cmd, "read_bytes_sec",
&read_bytes_sec)) < 0) {
vshError(ctl, "%s",
_("Unable to parse integer parameter"));
- goto out;
+ goto cleanup;
} else if (rv > 0) {
nparams++;
}
@@ -6158,7 +6162,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if ((rv = vshCommandOptULongLong(cmd, "write_bytes_sec",
&write_bytes_sec)) < 0) {
vshError(ctl, "%s",
_("Unable to parse integer parameter"));
- goto out;
+ goto cleanup;
} else if (rv > 0) {
nparams++;
}
@@ -6166,7 +6170,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if ((rv = vshCommandOptULongLong(cmd, "total_iops_sec",
&total_iops_sec)) < 0) {
vshError(ctl, "%s",
_("Unable to parse integer parameter"));
- goto out;
+ goto cleanup;
} else if (rv > 0) {
nparams++;
}
@@ -6174,7 +6178,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if ((rv = vshCommandOptULongLong(cmd, "read_iops_sec",
&read_iops_sec)) < 0) {
vshError(ctl, "%s",
_("Unable to parse integer parameter"));
- goto out;
+ goto cleanup;
} else if (rv > 0) {
nparams++;
}
@@ -6182,22 +6186,22 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if ((rv = vshCommandOptULongLong(cmd, "write_iops_sec",
&write_iops_sec)) < 0) {
vshError(ctl, "%s",
_("Unable to parse integer parameter"));
- goto out;
+ goto cleanup;
} else if (rv > 0) {
nparams++;
}
if (nparams == 0) {
- if ((virDomainGetBlockIoTune(dom, disk, NULL, &nparams, flags))
!= 0) {
+ if ((virDomainGetBlockIoTune(dom, NULL, NULL, &nparams, flags))
!= 0) {
vshError(ctl, "%s",
_("Unable to get number of block I/O throttle
parameters"));
- goto out;
+ goto cleanup;
}
if (nparams == 0) {
- virDomainFree(dom);
- return true;
+ ret = true;
+ goto cleanup;
}
params = vshCalloc(ctl, nparams, sizeof(*params));
@@ -6205,7 +6209,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if ((virDomainGetBlockIoTune(dom, disk, params, &nparams,
flags)) != 0) {
vshError(ctl, "%s",
_("Unable to get block I/O throttle parameters"));
- goto out;
+ goto cleanup;
}
for (i = 0; i < nparams; i++) {
@@ -6299,19 +6303,19 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
temp->value.ul = write_iops_sec;
}
- if ((virDomainSetBlockIoTune(dom, disk, params, nparams,
flags)) != 0) {
+ if ((virDomainSetBlockIoTune(dom, disk, params, nparams,
flags)) < 0) {
vshError(ctl, "%s",
_("Unable to change block I/O throttle"));
- goto out;
- } else {
- virDomainFree(dom);
- return true;
+ goto cleanup;
}
}
-out:
+ ret = true;
+
+cleanup:
+ VIR_FREE(params);
virDomainFree(dom);
- return false;
+ return ret;
}
/*
@@ -14928,10 +14932,10 @@ static const vshCmdDef domManagementCmds[] = {
{"attach-interface", cmdAttachInterface, opts_attach_interface,
info_attach_interface, 0},
{"autostart", cmdAutostart, opts_autostart, info_autostart, 0},
+ {"blkdeviotune", cmdBlkdeviotune, opts_blkdeviotune,
info_blkdeviotune, 0},
{"blkiotune", cmdBlkiotune, opts_blkiotune, info_blkiotune, 0},
{"blockpull", cmdBlockPull, opts_block_pull, info_block_pull, 0},
{"blockjob", cmdBlockJob, opts_block_job, info_block_job, 0},
- {"blkdeviotune", cmdBlkdeviotune, opts_blkdeviotune,
info_blkdeviotune, 0},
#ifndef WIN32
{"console", cmdConsole, opts_console, info_console, 0},
#endif
diff --git i/tools/virsh.pod w/tools/virsh.pod
index bea4288..8737cac 100644
--- i/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -572,13 +572,18 @@ operation can be checked with B<blockjob>.
I<path> specifies fully-qualified path of the disk.
I<bandwidth> specifies copying bandwidth limit in Mbps.
-=item B<blkdeviotune> I<domain> I<device> [[I<--total_bytes_sec>
B<total_bytes_sec>]
-| [[I<--read_bytes_sec> B<read_bytes_sec>] [I<--write_bytes_sec>
B<write_bytes_sec>]]
-[[I<--total_iops_sec> B<total_iops_sec>] | [[I<--read_iops_sec>
B<read_iops_sec>]
-[I<--write_iops_sec> B<write_iops_sec>]] [[I<--config>] [I<--live>] |
[I<--current>]]
+=item B<blkdeviotune> I<domain> I<device>
+[[I<--config>] [I<--live>] | [I<--current>]]
+[[I<total_bytes_sec>] | [I<read_bytes_sec>] [I<write_bytes_sec>]]
+[[I<total_iops_sec>] | [I<read_iops_sec>] [I<write_iops_sec>]]
-Set or query the block disk io limits settting.
-I<path> specifies block disk name.
+Set or query the block disk io parameters for a block device of I<domain>.
+I<device> specifies a unique target name (<target dev='name'/>) or source
+file (<source file='name'/>) for one of the disk devices attached to
+I<domain> (see also B<domblklist> for listing these names).
+
+If no limit is specified, it will query current I/O limits setting.
+Otherwise, alter the limits with these flags:
I<--total_bytes_sec> specifies total throughput limit in bytes per second.
I<--read_bytes_sec> specifies read throughput limit in bytes per second.
I<--write_bytes_sec> specifies write throughput limit in bytes per second.
@@ -586,6 +591,10 @@ I<--total_iops_sec> specifies total I/O operations
limit per second.
I<--read_iops_sec> specifies read I/O operations limit per second.
I<--write_iops_sec> specifies write I/O operations limit per second.
+When setting any value, all remaining values are reset to unlimited,
+an explicit 0 also clears any limit. A non-zero value for a given total
+cannot be mixed with non-zero values for read or write.
+
If I<--live> is specified, affect a running guest.
If I<--config> is specified, affect the next boot of a persistent guest.
If I<--current> is specified, affect the current guest state.
@@ -593,8 +602,6 @@ Both I<--live> and I<--current> flags may be given,
but I<--current> is
exclusive. If no flag is specified, behavior is different depending
on hypervisor.
-If no limit is specified, it will query current I/O limits setting.
-
=item B<blockjob> I<domain> I<path> [I<--abort>] [I<--info>] [I<bandwidth>]
Manage active block operations.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list