[libvirt] [PATCH 0/9] Add throttle blkio cgroup support for libvirt

Right now, libvirt only supports the cfq based blkio cgorup, this means if the block devices doesn't use cfq scheduler, the blkio cgroup will loss effect. This patchset adds the throttle blkio cgroup support for libvirt, intoduces four elements for domain configuration and extend the virsh command blkiotune. This patchset is a new version of Guan Qiang's patchset ://www.redhat.com/archives/libvir-list/2013-October/msg01066.html Change form Guan Qiang's patchset: 1, split to 8 patches, make logic more clear 2, change the type of read/write iops form unsigned long long to unsigned int, trying to set read/write iops to the value which bigger than max number of unsigned int will fail. 3, fix some logic shortage. Gao feng (9): rename virDomainBlkioDeviceWeightParseXML to virDomainBlkioDeviceParseXML rename virBlkioDeviceWeightArrayClear to virBlkioDeviceArrayClear rename virBlkioDeviceWeightPtr to virBlkioDevicePtr domain: introduce xml elements for throttle blkio cgroup blkio: Setting throttle blkio cgroup for domain qemu: allow to setup throttle blkio cgroup through virsh virsh: add virsh manual for setting throttle blkio cgroup lxc: allow to setup throttle blkio cgroup through virsh qemu: add new throttle blkio cgroup elements to the test xml docs/schemas/domaincommon.rng | 28 +- include/libvirt/libvirt.h.in | 45 ++ src/conf/domain_conf.c | 113 +++- src/conf/domain_conf.h | 16 +- src/libvirt_private.syms | 5 +- src/lxc/lxc_cgroup.c | 12 +- src/lxc/lxc_driver.c | 649 ++++++++++++++++++++- src/qemu/qemu_cgroup.c | 13 +- src/qemu/qemu_driver.c | 432 ++++++++++++-- src/util/vircgroup.c | 170 +++++- src/util/vircgroup.h | 18 + .../qemuxml2argv-blkiotune-device.xml | 8 + tools/virsh-domain.c | 64 ++ tools/virsh.pod | 36 +- 14 files changed, 1485 insertions(+), 124 deletions(-) -- 1.8.3.1

virDomainBlkioDeviceWeightParseXML will be used to parse the xml element read_bps, write_bps, read_iops, write_iops. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/conf/domain_conf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 140eb80..5eb0278 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -892,7 +892,7 @@ virBlkioDeviceWeightArrayClear(virBlkioDeviceWeightPtr deviceWeights, } /** - * virDomainBlkioDeviceWeightParseXML + * virDomainBlkioDeviceParseXML * * this function parses a XML node: * @@ -904,8 +904,8 @@ virBlkioDeviceWeightArrayClear(virBlkioDeviceWeightPtr deviceWeights, * and fills a virBlkioDeviceWeight struct. */ static int -virDomainBlkioDeviceWeightParseXML(xmlNodePtr root, - virBlkioDeviceWeightPtr dw) +virDomainBlkioDeviceParseXML(xmlNodePtr root, + virBlkioDeviceWeightPtr dw) { char *c; xmlNodePtr node; @@ -11037,8 +11037,8 @@ virDomainDefParseXML(xmlDocPtr xml, for (i = 0; i < n; i++) { size_t j; - if (virDomainBlkioDeviceWeightParseXML(nodes[i], - &def->blkio.devices[i]) < 0) + if (virDomainBlkioDeviceParseXML(nodes[i], + &def->blkio.devices[i]) < 0) goto error; def->blkio.ndevices++; for (j = 0; j < i; j++) { -- 1.8.3.1

On Mon, Dec 02, 2013 at 02:47:56PM +0800, Gao feng wrote:
virDomainBlkioDeviceWeightParseXML will be used to parse the xml element read_bps, write_bps, read_iops, write_iops.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/conf/domain_conf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_conf.h | 4 ++-- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5eb0278..3cb1187 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -882,8 +882,8 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt) void -virBlkioDeviceWeightArrayClear(virBlkioDeviceWeightPtr deviceWeights, - int ndevices) +virBlkioDeviceArrayClear(virBlkioDeviceWeightPtr deviceWeights, + int ndevices) { size_t i; @@ -1990,8 +1990,8 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->description); VIR_FREE(def->title); - virBlkioDeviceWeightArrayClear(def->blkio.devices, - def->blkio.ndevices); + virBlkioDeviceArrayClear(def->blkio.devices, + def->blkio.ndevices); VIR_FREE(def->blkio.devices); virDomainWatchdogDefFree(def->watchdog); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4561ccc..383eb27 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1910,8 +1910,8 @@ struct _virDomainIdMapDef { }; -void virBlkioDeviceWeightArrayClear(virBlkioDeviceWeightPtr deviceWeights, - int ndevices); +void virBlkioDeviceArrayClear(virBlkioDeviceWeightPtr deviceWeights, + int ndevices); typedef struct _virDomainResourceDef virDomainResourceDef; typedef virDomainResourceDef *virDomainResourceDefPtr; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 50fe00e..e5f7cbd 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -104,7 +104,7 @@ virDomainAuditVcpu; # conf/domain_conf.h -virBlkioDeviceWeightArrayClear; +virBlkioDeviceArrayClear; virDiskNameToBusDeviceIndex; virDiskNameToIndex; virDomainActualNetDefFree; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 47d8a09..8a833c7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7497,7 +7497,7 @@ error: virReportError(VIR_ERR_INVALID_ARG, _("unable to parse device weight '%s'"), deviceWeightStr); cleanup: - virBlkioDeviceWeightArrayClear(result, ndevices); + virBlkioDeviceArrayClear(result, ndevices); VIR_FREE(result); return -1; } @@ -7628,7 +7628,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, &vm->def->blkio.ndevices, devices, ndevices) < 0) ret = -1; - virBlkioDeviceWeightArrayClear(devices, ndevices); + virBlkioDeviceArrayClear(devices, ndevices); VIR_FREE(devices); } } @@ -7665,7 +7665,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, &persistentDef->blkio.ndevices, devices, ndevices) < 0) ret = -1; - virBlkioDeviceWeightArrayClear(devices, ndevices); + virBlkioDeviceArrayClear(devices, ndevices); VIR_FREE(devices); } } -- 1.8.3.1

On Mon, Dec 02, 2013 at 02:47:57PM +0800, Gao feng wrote:
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/domain_conf.h | 4 ++-- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

The throttle blkio cgroup will reuse this struct. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/conf/domain_conf.c | 18 +++++++++--------- src/conf/domain_conf.h | 10 +++++----- src/lxc/lxc_cgroup.c | 6 +++--- src/qemu/qemu_cgroup.c | 8 ++++---- src/qemu/qemu_driver.c | 18 +++++++++--------- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3cb1187..98754e5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -882,13 +882,13 @@ virDomainXMLOptionGetNamespace(virDomainXMLOptionPtr xmlopt) void -virBlkioDeviceArrayClear(virBlkioDeviceWeightPtr deviceWeights, +virBlkioDeviceArrayClear(virBlkioDevicePtr devices, int ndevices) { size_t i; for (i = 0; i < ndevices; i++) - VIR_FREE(deviceWeights[i].path); + VIR_FREE(devices[i].path); } /** @@ -901,11 +901,11 @@ virBlkioDeviceArrayClear(virBlkioDeviceWeightPtr deviceWeights, * <weight>weight</weight> * </device> * - * and fills a virBlkioDeviceWeight struct. + * and fills a virBlkioDeviceTune struct. */ static int virDomainBlkioDeviceParseXML(xmlNodePtr root, - virBlkioDeviceWeightPtr dw) + virBlkioDevicePtr dev) { char *c; xmlNodePtr node; @@ -913,16 +913,16 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root, node = root->children; while (node) { if (node->type == XML_ELEMENT_NODE) { - if (xmlStrEqual(node->name, BAD_CAST "path") && !dw->path) { - dw->path = (char *)xmlNodeGetContent(node); + if (xmlStrEqual(node->name, BAD_CAST "path") && !dev->path) { + dev->path = (char *)xmlNodeGetContent(node); } else if (xmlStrEqual(node->name, BAD_CAST "weight")) { c = (char *)xmlNodeGetContent(node); - if (virStrToLong_ui(c, NULL, 10, &dw->weight) < 0) { + if (virStrToLong_ui(c, NULL, 10, &dev->weight) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("could not parse weight %s"), c); VIR_FREE(c); - VIR_FREE(dw->path); + VIR_FREE(dev->path); return -1; } VIR_FREE(c); @@ -930,7 +930,7 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root, } node = node->next; } - if (!dw->path) { + if (!dev->path) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("missing per-device path")); return -1; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 383eb27..c53084c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1859,9 +1859,9 @@ virDomainVcpuPinDefPtr virDomainVcpuPinFindByVcpu(virDomainVcpuPinDefPtr *def, int nvcpupin, int vcpu); -typedef struct _virBlkioDeviceWeight virBlkioDeviceWeight; -typedef virBlkioDeviceWeight *virBlkioDeviceWeightPtr; -struct _virBlkioDeviceWeight { +typedef struct _virBlkioDevice virBlkioDevice; +typedef virBlkioDevice *virBlkioDevicePtr; +struct _virBlkioDevice { char *path; unsigned int weight; }; @@ -1910,7 +1910,7 @@ struct _virDomainIdMapDef { }; -void virBlkioDeviceArrayClear(virBlkioDeviceWeightPtr deviceWeights, +void virBlkioDeviceArrayClear(virBlkioDevicePtr deviceWeights, int ndevices); typedef struct _virDomainResourceDef virDomainResourceDef; @@ -1939,7 +1939,7 @@ struct _virDomainDef { unsigned int weight; size_t ndevices; - virBlkioDeviceWeightPtr devices; + virBlkioDevicePtr devices; } blkio; struct { diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 275e250..310a476 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -112,10 +112,10 @@ static int virLXCCgroupSetupBlkioTune(virDomainDefPtr def, if (def->blkio.ndevices) { for (i = 0; i < def->blkio.ndevices; i++) { - virBlkioDeviceWeightPtr dw = &def->blkio.devices[i]; - if (!dw->weight) + virBlkioDevicePtr dev = &def->blkio.devices[i]; + if (!dev->weight) continue; - if (virCgroupSetBlkioDeviceWeight(cgroup, dw->path, dw->weight) < 0) + if (virCgroupSetBlkioDeviceWeight(cgroup, dev->path, dev->weight) < 0) return -1; } } diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f0cacd0..a18955e 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -399,11 +399,11 @@ qemuSetupBlkioCgroup(virDomainObjPtr vm) if (vm->def->blkio.ndevices) { for (i = 0; i < vm->def->blkio.ndevices; i++) { - virBlkioDeviceWeightPtr dw = &vm->def->blkio.devices[i]; - if (!dw->weight) + virBlkioDevicePtr dev = &vm->def->blkio.devices[i]; + if (!dev->weight) continue; - if (virCgroupSetBlkioDeviceWeight(priv->cgroup, dw->path, - dw->weight) < 0) + if (virCgroupSetBlkioDeviceWeight(priv->cgroup, dev->path, + dev->weight) < 0) return -1; } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8a833c7..5c0c5e5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7424,15 +7424,15 @@ cleanup: */ static int qemuDomainParseDeviceWeightStr(char *deviceWeightStr, - virBlkioDeviceWeightPtr *dw, size_t *size) + virBlkioDevicePtr *dev, size_t *size) { char *temp; int ndevices = 0; int nsep = 0; size_t i; - virBlkioDeviceWeightPtr result = NULL; + virBlkioDevicePtr result = NULL; - *dw = NULL; + *dev = NULL; *size = 0; if (STREQ(deviceWeightStr, "")) @@ -7488,7 +7488,7 @@ qemuDomainParseDeviceWeightStr(char *deviceWeightStr, if (!i) VIR_FREE(result); - *dw = result; + *dev = result; *size = i; return 0; @@ -7505,13 +7505,13 @@ cleanup: /* Modify dest_array to reflect all device weight changes described in * src_array. */ static int -qemuDomainMergeDeviceWeights(virBlkioDeviceWeightPtr *dest_array, +qemuDomainMergeDeviceWeights(virBlkioDevicePtr *dest_array, size_t *dest_size, - virBlkioDeviceWeightPtr src_array, + virBlkioDevicePtr src_array, size_t src_size) { size_t i, j; - virBlkioDeviceWeightPtr dest, src; + virBlkioDevicePtr dest, src; for (i = 0; i < src_size; i++) { bool found = false; @@ -7606,7 +7606,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, ret = -1; } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { size_t ndevices; - virBlkioDeviceWeightPtr devices = NULL; + virBlkioDevicePtr devices = NULL; size_t j; if (qemuDomainParseDeviceWeightStr(params[i].value.s, @@ -7652,7 +7652,7 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, persistentDef->blkio.weight = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { - virBlkioDeviceWeightPtr devices = NULL; + virBlkioDevicePtr devices = NULL; size_t ndevices; if (qemuDomainParseDeviceWeightStr(params[i].value.s, -- 1.8.3.1

On Mon, Dec 02, 2013 at 02:47:58PM +0800, Gao feng wrote:
The throttle blkio cgroup will reuse this struct.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/conf/domain_conf.c | 18 +++++++++--------- src/conf/domain_conf.h | 10 +++++----- src/lxc/lxc_cgroup.c | 6 +++--- src/qemu/qemu_cgroup.c | 8 ++++---- src/qemu/qemu_driver.c | 18 +++++++++--------- 5 files changed, 30 insertions(+), 30 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This patch introduces new xml elements under <blkiotune>, we use these new elements to setup the throttle blkio cgroup for domain. The new blkiotune node looks like this: <blkiotune> <device> <path>/path/to/block</path> <weight>1000</weight> <read_iops>10000</read_iops> <write_iops>10000</write_iops> <read_bps>10000</read_bps> <write_bps>10000</write_bps> </device> </blkiotune> Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- docs/schemas/domaincommon.rng | 28 +++++++++++++-- src/conf/domain_conf.c | 83 +++++++++++++++++++++++++++++++++++++------ src/conf/domain_conf.h | 4 +++ 3 files changed, 102 insertions(+), 13 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 80848d2..e3e4766 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -621,9 +621,31 @@ <element name="path"> <ref name="absFilePath"/> </element> - <element name="weight"> - <ref name="weight"/> - </element> + <optional> + <element name="weight"> + <ref name="weight"/> + </element> + </optional> + <optional> + <element name="read_iops"> + <data type='unsignedInt'/> + </element> + </optional> + <optional> + <element name="write_iops"> + <data type='unsignedInt'/> + </element> + </optional> + <optional> + <element name="read_bps"> + <data type='unsignedLong'/> + </element> + </optional> + <optional> + <element name="write_bps"> + <data type='unsignedLong'/> + </element> + </optional> </interleave> </element> </zeroOrMore> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 98754e5..9bcc14f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -899,6 +899,10 @@ virBlkioDeviceArrayClear(virBlkioDevicePtr devices, * <device> * <path>/fully/qualified/device/path</path> * <weight>weight</weight> + * <read_bps>bps</read_bps> + * <read_iops>iops</read_iops> + * <write_bps>bps</write_bps> + * <write_iops>iops</write_iops> * </device> * * and fills a virBlkioDeviceTune struct. @@ -907,7 +911,7 @@ static int virDomainBlkioDeviceParseXML(xmlNodePtr root, virBlkioDevicePtr dev) { - char *c; + char *c = NULL; xmlNodePtr node; node = root->children; @@ -921,9 +925,43 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("could not parse weight %s"), c); - VIR_FREE(c); - VIR_FREE(dev->path); - return -1; + goto error; + } + VIR_FREE(c); + } else if (xmlStrEqual(node->name, BAD_CAST "read_bps")) { + c = (char *)xmlNodeGetContent(node); + if (virStrToLong_ull(c, NULL, 10, &dev->rbps) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not parse read bps %s"), + c); + goto error; + } + VIR_FREE(c); + } else if (xmlStrEqual(node->name, BAD_CAST "write_bps")) { + c = (char *)xmlNodeGetContent(node); + if (virStrToLong_ull(c, NULL, 10, &dev->wbps) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not parse write bps %s"), + c); + goto error; + } + VIR_FREE(c); + } else if (xmlStrEqual(node->name, BAD_CAST "read_iops")) { + c = (char *)xmlNodeGetContent(node); + if (virStrToLong_ui(c, NULL, 10, &dev->riops) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not parse read iops %s"), + c); + goto error; + } + VIR_FREE(c); + } else if (xmlStrEqual(node->name, BAD_CAST "write_iops")) { + c = (char *)xmlNodeGetContent(node); + if (virStrToLong_ui(c, NULL, 10, &dev->wiops) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("could not parse write iops %s"), + c); + goto error; } VIR_FREE(c); } @@ -937,6 +975,11 @@ virDomainBlkioDeviceParseXML(xmlNodePtr root, } return 0; + +error: + VIR_FREE(c); + VIR_FREE(dev->path); + return -1; } @@ -11045,7 +11088,7 @@ virDomainDefParseXML(xmlDocPtr xml, if (STREQ(def->blkio.devices[j].path, def->blkio.devices[i].path)) { virReportError(VIR_ERR_XML_ERROR, - _("duplicate device weight path '%s'"), + _("duplicate blkio device path '%s'"), def->blkio.devices[i].path); goto error; } @@ -16524,7 +16567,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, blkio = true; } else { for (n = 0; n < def->blkio.ndevices; n++) { - if (def->blkio.devices[n].weight) { + if (def->blkio.devices[n].weight || + def->blkio.devices[n].riops || + def->blkio.devices[n].wiops || + def->blkio.devices[n].rbps || + def->blkio.devices[n].wbps) { blkio = true; break; } @@ -16539,13 +16586,29 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->blkio.weight); for (n = 0; n < def->blkio.ndevices; n++) { - if (def->blkio.devices[n].weight == 0) + virBlkioDevicePtr dev = &def->blkio.devices[n]; + + if (!dev->weight && !dev->riops && !dev->wiops && + !dev->rbps && !dev->wbps) continue; virBufferAddLit(buf, " <device>\n"); virBufferEscapeString(buf, " <path>%s</path>\n", - def->blkio.devices[n].path); - virBufferAsprintf(buf, " <weight>%u</weight>\n", - def->blkio.devices[n].weight); + dev->path); + if (dev->weight) + virBufferAsprintf(buf, " <weight>%u</weight>\n", + dev->weight); + if (dev->riops) + virBufferAsprintf(buf, " <read_iops>%u</read_iops>\n", + dev->riops); + if (dev->wiops) + virBufferAsprintf(buf, " <write_iops>%u</write_iops>\n", + dev->wiops); + if (dev->rbps) + virBufferAsprintf(buf, " <read_bps>%llu</read_bps>\n", + dev->rbps); + if (dev->wbps) + virBufferAsprintf(buf, " <write_bps>%llu</write_bps>\n", + dev->wbps); virBufferAddLit(buf, " </device>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c53084c..ac8e314 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1864,6 +1864,10 @@ typedef virBlkioDevice *virBlkioDevicePtr; struct _virBlkioDevice { char *path; unsigned int weight; + unsigned int riops; + unsigned int wiops; + unsigned long long rbps; + unsigned long long wbps; }; enum virDomainRNGModel { -- 1.8.3.1

On Mon, Dec 02, 2013 at 02:47:59PM +0800, Gao feng wrote:
This patch introduces new xml elements under <blkiotune>, we use these new elements to setup the throttle blkio cgroup for domain. The new blkiotune node looks like this:
<blkiotune> <device> <path>/path/to/block</path> <weight>1000</weight> <read_iops>10000</read_iops> <write_iops>10000</write_iops> <read_bps>10000</read_bps> <write_bps>10000</write_bps> </device> </blkiotune>
Under the <disk> element we have <iotune> <total_bytes_sec>10000000</total_bytes_sec> <read_iops_sec>400000</read_iops_sec> <write_iops_sec>100000</write_iops_sec> </iotune> Please use a consistent naming convention for these new elements - ie read_iops_sec not read_iops. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 12/10/2013 11:30 PM, Daniel P. Berrange wrote:
On Mon, Dec 02, 2013 at 02:47:59PM +0800, Gao feng wrote:
This patch introduces new xml elements under <blkiotune>, we use these new elements to setup the throttle blkio cgroup for domain. The new blkiotune node looks like this:
<blkiotune> <device> <path>/path/to/block</path> <weight>1000</weight> <read_iops>10000</read_iops> <write_iops>10000</write_iops> <read_bps>10000</read_bps> <write_bps>10000</write_bps> </device> </blkiotune>
Under the <disk> element we have
<iotune> <total_bytes_sec>10000000</total_bytes_sec> <read_iops_sec>400000</read_iops_sec> <write_iops_sec>100000</write_iops_sec> </iotune>
Please use a consistent naming convention for these new elements - ie read_iops_sec not read_iops.
iops means i/o per second. iops_sec looks very strange. since this alreay in and it is an user interface. I will change my patchset. Thanks

This patch extends virCgroupSetBlkioWeightDevice and rename it the virCgroupSetBlkioDevice, now we can use this interface to set up throttle blkio cgroup too. Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/libvirt_private.syms | 3 + src/lxc/lxc_cgroup.c | 10 ++- src/qemu/qemu_cgroup.c | 11 +-- src/qemu/qemu_driver.c | 10 ++- src/util/vircgroup.c | 170 ++++++++++++++++++++++++++++++++++++++++++++--- src/util/vircgroup.h | 18 +++++ 6 files changed, 203 insertions(+), 19 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e5f7cbd..c008e2b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1024,6 +1024,9 @@ virCgroupNewVcpu; virCgroupPathOfController; virCgroupRemove; virCgroupRemoveRecursively; +virCgroupSetBlkioDevice; +virCgroupSetBlkioDeviceBps; +virCgroupSetBlkioDeviceIops; virCgroupSetBlkioDeviceWeight; virCgroupSetBlkioWeight; virCgroupSetCpuCfsPeriod; diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 310a476..1c4bee5 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -113,9 +113,13 @@ static int virLXCCgroupSetupBlkioTune(virDomainDefPtr def, if (def->blkio.ndevices) { for (i = 0; i < def->blkio.ndevices; i++) { virBlkioDevicePtr dev = &def->blkio.devices[i]; - if (!dev->weight) - continue; - if (virCgroupSetBlkioDeviceWeight(cgroup, dev->path, dev->weight) < 0) + if (virCgroupSetBlkioDevice(cgroup, + dev->path, + dev->weight, + dev->riops, + dev->wiops, + dev->rbps, + dev->wbps) < 0) return -1; } } diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index a18955e..3b579ca 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -400,10 +400,13 @@ qemuSetupBlkioCgroup(virDomainObjPtr vm) if (vm->def->blkio.ndevices) { for (i = 0; i < vm->def->blkio.ndevices; i++) { virBlkioDevicePtr dev = &vm->def->blkio.devices[i]; - if (!dev->weight) - continue; - if (virCgroupSetBlkioDeviceWeight(priv->cgroup, dev->path, - dev->weight) < 0) + if (virCgroupSetBlkioDevice(priv->cgroup, + dev->path, + dev->weight, + dev->riops, + dev->wiops, + dev->rbps, + dev->wbps) < 0) return -1; } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5c0c5e5..61dbe7f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7616,9 +7616,13 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, continue; } for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDeviceWeight(priv->cgroup, - devices[j].path, - devices[j].weight) < 0) { + if (virCgroupSetBlkioDevice(priv->cgroup, + devices[j].path, + devices[j].weight, + devices[j].riops, + devices[j].wiops, + devices[j].rbps, + devices[j].wbps) < 0) { ret = -1; break; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 5c43e10..7f9ba49 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1824,18 +1824,110 @@ virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) return ret; } +/** + * virCgroupSetBlkioDeviceIops: + * @group: The cgroup to change block io setting for + * @path: The path of device + * @read: setup the read iops or write iops + * @iops: The new device iops throttle, or 0 to clear + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupSetBlkioDeviceIops(virCgroupPtr group, + const char *path, + bool read, + unsigned int iops) +{ + char *str; + struct stat sb; + int ret; + + if (stat(path, &sb) < 0) { + virReportSystemError(errno, + _("Path '%s' is not accessible"), + path); + return -1; + } + + if (!S_ISBLK(sb.st_mode)) { + virReportSystemError(EINVAL, + _("Path '%s' must be a block device"), + path); + return -1; + } + + if (virAsprintf(&str, "%d:%d %u", major(sb.st_rdev), + minor(sb.st_rdev), iops) < 0) + return -1; + + ret = virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + read ? + "blkio.throttle.read_iops_device" : + "blkio.throttle.write_iops_device", + str); + + VIR_FREE(str); + return ret; +} + /** - * virCgroupSetBlkioDeviceWeight: + * virCgroupSetBlkioDeviceBps: + * @group: The cgroup to change block io setting for + * @path: The path of device + * @read: setup the read bps or write bps + * @bps: The new device bps throttle, or 0 to clear * - * @group: The cgroup to change io device weight device for - * @path: The device with a weight to alter + * Returns: 0 on success, -1 on error + */ +int +virCgroupSetBlkioDeviceBps(virCgroupPtr group, + const char *path, + bool read, + unsigned long long bps) +{ + char *str; + struct stat sb; + int ret; + + if (stat(path, &sb) < 0) { + virReportSystemError(errno, + _("Path '%s' is not accessible"), + path); + return -1; + } + + if (!S_ISBLK(sb.st_mode)) { + virReportSystemError(EINVAL, + _("Path '%s' must be a block device"), + path); + return -1; + } + + if (virAsprintf(&str, "%d:%d %llu", major(sb.st_rdev), + minor(sb.st_rdev), bps) < 0) + return -1; + + ret = virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + read ? + "blkio.throttle.read_bps_device" : + "blkio.throttle.write_bps_device", + str); + + VIR_FREE(str); + return ret; +} + +/** + * virCgroupSetBlkioDeviceWeight: + * @group: The cgroup to change block io setting for + * @path: The path of device * @weight: The new device weight (100-1000), * (10-1000) after kernel 2.6.39, or 0 to clear * - * device_weight is treated as a write-only parameter, so - * there isn't a getter counterpart. - * * Returns: 0 on success, -1 on error */ int @@ -1861,8 +1953,8 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group, return -1; } - if (virAsprintf(&str, "%d:%d %d", major(sb.st_rdev), minor(sb.st_rdev), - weight) < 0) + if (virAsprintf(&str, "%d:%d %u", major(sb.st_rdev), + minor(sb.st_rdev), weight) < 0) return -1; ret = virCgroupSetValueStr(group, @@ -1874,6 +1966,47 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group, } +/** + * virCgroupSetBlkioDevice: + * + * @group: The cgroup to change block io setting for + * @path: The path of device + * @weight: The new device weight (100-1000), + * (10-1000) after kernel 2.6.39 + * @riops: The new device read iops throttle + * @wiops: The new device write iops throttle + * @rbps: The new device read bps throttle + * @wbps: The new device write bps throttle + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupSetBlkioDevice(virCgroupPtr group, + const char *path, + unsigned int weight, + unsigned int riops, + unsigned int wiops, + unsigned long long rbps, + unsigned long long wbps) +{ + if (weight && (virCgroupSetBlkioDeviceWeight(group, path, weight) < 0)) + return -1; + + if (riops && (virCgroupSetBlkioDeviceIops(group, path, 1, riops) < 0)) + return -1; + + if (wiops && (virCgroupSetBlkioDeviceIops(group, path, 0, wiops) < 0)) + return -1; + + if (rbps && (virCgroupSetBlkioDeviceBps(group, path, 1, rbps) < 0)) + return -1; + + if (wbps && (virCgroupSetBlkioDeviceBps(group, path, 0, wbps) < 0)) + return -1; + + return 0; +} + /** * virCgroupSetMemory: @@ -3280,11 +3413,30 @@ virCgroupGetBlkioWeight(virCgroupPtr group ATTRIBUTE_UNUSED, return -1; } - int virCgroupSetBlkioDeviceWeight(virCgroupPtr group ATTRIBUTE_UNUSED, const char *path ATTRIBUTE_UNUSED, unsigned int weight ATTRIBUTE_UNUSED) + +int +virCgroupSetBlkioDeviceIops(virCgroupPtr group ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + bool read ATTRIBUTE_UNUSED, + unsigned long long iops ATTRIBUTE_UNUSED) + +int +virCgroupSetBlkioDeviceBps(virCgroupPtr group ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + bool read ATTRIBUTE_UNUSED, + unsigned long long bps ATTRIBUTE_UNUSED) +int +virCgroupSetBlkioDevice(virCgroupPtr group ATTRIBUTE_UNUSED, + const char *path ATTRIBUTE_UNUSED, + unsigned int weight ATTRIBUTE_UNUSED, + unsigned int riops ATTRIBUTE_UNUSED, + unsigned int wiops ATTRIBUTE_UNUSED, + unsigned long long rbps ATTRIBUTE_UNUSED, + unsigned long long wbps ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Control groups not supported on this platform")); diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 835eb30..99c3a35 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -126,6 +126,24 @@ int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, const char *path, unsigned int weight); +int virCgroupSetBlkioDeviceIops(virCgroupPtr group, + const char *path, + bool read, + unsigned int iops); + +int virCgroupSetBlkioDeviceBps(virCgroupPtr group, + const char *path, + bool read, + unsigned long long bps); + +int virCgroupSetBlkioDevice(virCgroupPtr group, + const char *path, + unsigned int weight, + unsigned int riops, + unsigned int wiops, + unsigned long long rbps, + unsigned long long wbps); + int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb); int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb); -- 1.8.3.1

On Mon, Dec 02, 2013 at 02:48:00PM +0800, Gao feng wrote:
This patch extends virCgroupSetBlkioWeightDevice and rename it the virCgroupSetBlkioDevice, now we can use this interface to set up throttle blkio cgroup too.
Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/libvirt_private.syms | 3 + src/lxc/lxc_cgroup.c | 10 ++- src/qemu/qemu_cgroup.c | 11 +-- src/qemu/qemu_driver.c | 10 ++- src/util/vircgroup.c | 170 ++++++++++++++++++++++++++++++++++++++++++++--- src/util/vircgroup.h | 18 +++++ 6 files changed, 203 insertions(+), 19 deletions(-) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 835eb30..99c3a35 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -126,6 +126,24 @@ int virCgroupSetBlkioDeviceWeight(virCgroupPtr group, const char *path, unsigned int weight);
+int virCgroupSetBlkioDeviceIops(virCgroupPtr group, + const char *path, + bool read, + unsigned int iops); + +int virCgroupSetBlkioDeviceBps(virCgroupPtr group, + const char *path, + bool read, + unsigned long long bps);
I think it is preferrable to have separate methods for read and write, because no one reading the code will ever remember that 'true == read' and 'false == write' in parameters.
+int virCgroupSetBlkioDevice(virCgroupPtr group, + const char *path, + unsigned int weight, + unsigned int riops, + unsigned int wiops, + unsigned long long rbps, + unsigned long long wbps);
I don't think this method serves any real useful purpose - just call the other public APIs directly. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

With this patch, user can setup the throttle blkio cgorup for domain through the virsh cmd, such as: virsh blkiotune domain1 --device-read-bps /dev/sda1,10000,/dev/sda2,20000 --device-write-bps /dev/sda1,10000 --device-read-iops /dev/sda1,10000 --device-write-iops /dev/sda1,10000,/dev/sda2,0 Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- include/libvirt/libvirt.h.in | 45 +++++ src/qemu/qemu_driver.c | 418 +++++++++++++++++++++++++++++++++++++++---- tools/virsh-domain.c | 64 +++++++ 3 files changed, 488 insertions(+), 39 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5aad75c..d054900 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1806,6 +1806,51 @@ char * virDomainGetSchedulerType(virDomainPtr domain, #define VIR_DOMAIN_BLKIO_DEVICE_WEIGHT "device_weight" + +/** + * VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS: + * + * Macro for the blkio tunable throttle.read_iops_device: it represents + * the number of reading the block device per second, as a string. The + * string is parsed as a series of /path/to/device, read_iops elements, + * separated by ','. + */ + +#define VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS "device_read_iops" + + +/** + * VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS: + * + * Macro for the blkio tunable throttle.write_iops_device: it represents + * the number of writing the block device per second, as a string. The + * string is parsed as a series of /path/to/device, write_iops elements, + * separated by ','. + */ +#define VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS "device_write_iops" + + +/** + * VIR_DOMAIN_BLKIO_DEVICE_READ_BPS: + * + * Macro for the blkio tunable throttle.read_iops_device: it represents + * the bytes of reading the block device per second, as a string. The + * string is parsed as a series of /path/to/device, read_bps elements, + * separated by ','. + */ +#define VIR_DOMAIN_BLKIO_DEVICE_READ_BPS "device_read_bps" + + +/** + * VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS: + * + * Macro for the blkio tunable throttle.read_iops_device: it represents + * the number of reading the block device per second, as a string. The + * string is parsed as a series of /path/to/device, read_iops elements, + * separated by ','. + */ +#define VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS "device_write_bps" + /* Set Blkio tunables for the domain*/ int virDomainSetBlkioParameters(virDomainPtr domain, virTypedParameterPtr params, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 61dbe7f..1ad5b94 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -131,7 +131,7 @@ # define KVM_CAP_NR_VCPUS 9 /* returns max vcpus per vm */ #endif -#define QEMU_NB_BLKIO_PARAM 2 +#define QEMU_NB_BLKIO_PARAM 6 #define QEMU_NB_BANDWIDTH_PARAM 6 @@ -7419,12 +7419,12 @@ cleanup: return ret; } -/* deviceWeightStr in the form of /device/path,weight,/device/path,weight +/* blkioDeviceStr in the form of /device/path,weight,/device/path,weight * for example, /dev/disk/by-path/pci-0000:00:1f.2-scsi-0:0:0:0,800 */ static int -qemuDomainParseDeviceWeightStr(char *deviceWeightStr, - virBlkioDevicePtr *dev, size_t *size) +qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, + virBlkioDevicePtr *dev, size_t *size) { char *temp; int ndevices = 0; @@ -7435,10 +7435,10 @@ qemuDomainParseDeviceWeightStr(char *deviceWeightStr, *dev = NULL; *size = 0; - if (STREQ(deviceWeightStr, "")) + if (STREQ(blkioDeviceStr, "")) return 0; - temp = deviceWeightStr; + temp = blkioDeviceStr; while (temp) { temp = strchr(temp, ','); if (temp) { @@ -7458,7 +7458,7 @@ qemuDomainParseDeviceWeightStr(char *deviceWeightStr, return -1; i = 0; - temp = deviceWeightStr; + temp = blkioDeviceStr; while (temp) { char *p = temp; @@ -7470,11 +7470,25 @@ qemuDomainParseDeviceWeightStr(char *deviceWeightStr, if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0) goto cleanup; - /* weight */ + /* value */ temp = p + 1; - if (virStrToLong_ui(temp, &p, 10, &result[i].weight) < 0) - goto error; + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + if (virStrToLong_ui(temp, &p, 10, &result[i].weight) < 0) + goto error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { + if (virStrToLong_ui(temp, &p, 10, &result[i].riops) < 0) + goto error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { + if (virStrToLong_ui(temp, &p, 10, &result[i].wiops) < 0) + goto error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { + if (virStrToLong_ull(temp, &p, 10, &result[i].rbps) < 0) + goto error; + } else { + if (virStrToLong_ull(temp, &p, 10, &result[i].wbps) < 0) + goto error; + } i++; @@ -7495,20 +7509,21 @@ qemuDomainParseDeviceWeightStr(char *deviceWeightStr, error: virReportError(VIR_ERR_INVALID_ARG, - _("unable to parse device weight '%s'"), deviceWeightStr); + _("unable to parse blkio device '%s'"), blkioDeviceStr); cleanup: virBlkioDeviceArrayClear(result, ndevices); VIR_FREE(result); return -1; } -/* Modify dest_array to reflect all device weight changes described in +/* Modify dest_array to reflect all blkio device changes described in * src_array. */ static int -qemuDomainMergeDeviceWeights(virBlkioDevicePtr *dest_array, - size_t *dest_size, - virBlkioDevicePtr src_array, - size_t src_size) +qemuDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array, + size_t *dest_size, + virBlkioDevicePtr src_array, + size_t src_size, + const char *type) { size_t i, j; virBlkioDevicePtr dest, src; @@ -7521,18 +7536,40 @@ qemuDomainMergeDeviceWeights(virBlkioDevicePtr *dest_array, dest = &(*dest_array)[j]; if (STREQ(src->path, dest->path)) { found = true; - dest->weight = src->weight; + + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) + dest->weight = src->weight; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) + dest->riops = src->riops; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) + dest->wiops = src->wiops; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) + dest->rbps = src->rbps; + else + dest->wbps = src->wbps; + break; } } if (!found) { - if (!src->weight) + if (!src->weight && !src->riops && src->wiops && src->rbps && src->wbps) continue; if (VIR_EXPAND_N(*dest_array, *dest_size, 1) < 0) return -1; dest = &(*dest_array)[*dest_size - 1]; dest->path = src->path; - dest->weight = src->weight; + + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) + dest->weight = src->weight; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) + dest->riops = src->riops; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) + dest->wiops = src->wiops; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) + dest->rbps = src->rbps; + else + dest->wbps = src->wbps; + src->path = NULL; } } @@ -7562,6 +7599,14 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, VIR_TYPED_PARAM_UINT, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT, VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_BLKIO_DEVICE_READ_BPS, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS, + VIR_TYPED_PARAM_STRING, NULL) < 0) return -1; @@ -7604,33 +7649,72 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, if (virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui) < 0) ret = -1; - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { size_t ndevices; virBlkioDevicePtr devices = NULL; size_t j; - if (qemuDomainParseDeviceWeightStr(params[i].value.s, + if (qemuDomainParseBlkioDeviceStr(params[i].value.s, + param->field, &devices, &ndevices) < 0) { ret = -1; continue; } - for (j = 0; j < ndevices; j++) { - if (virCgroupSetBlkioDevice(priv->cgroup, - devices[j].path, - devices[j].weight, - devices[j].riops, - devices[j].wiops, - devices[j].rbps, - devices[j].wbps) < 0) { - ret = -1; - break; + if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceWeight(priv->cgroup, + devices[j].path, + devices[j].weight) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceIops(priv->cgroup, + devices[j].path, + 1, devices[j].riops) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceIops(priv->cgroup, + devices[j].path, + 0, devices[j].wiops) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceBps(priv->cgroup, + devices[j].path, + 1, devices[j].rbps) < 0) { + ret = -1; + break; + } + } + } else { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceBps(priv->cgroup, + devices[j].path, + 0, devices[j].wbps) < 0) { + ret = -1; + break; + } } } if (j != ndevices || - qemuDomainMergeDeviceWeights(&vm->def->blkio.devices, - &vm->def->blkio.ndevices, - devices, ndevices) < 0) + qemuDomainMergeBlkioDevice(&vm->def->blkio.devices, + &vm->def->blkio.ndevices, + devices, ndevices, param->field) < 0) ret = -1; virBlkioDeviceArrayClear(devices, ndevices); VIR_FREE(devices); @@ -7655,19 +7739,24 @@ qemuDomainSetBlkioParameters(virDomainPtr dom, } persistentDef->blkio.weight = params[i].value.ui; - } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { virBlkioDevicePtr devices = NULL; size_t ndevices; - if (qemuDomainParseDeviceWeightStr(params[i].value.s, + if (qemuDomainParseBlkioDeviceStr(params[i].value.s, + params[i].field, &devices, &ndevices) < 0) { ret = -1; continue; } - if (qemuDomainMergeDeviceWeights(&persistentDef->blkio.devices, - &persistentDef->blkio.ndevices, - devices, ndevices) < 0) + if (qemuDomainMergeBlkioDevice(&persistentDef->blkio.devices, + &persistentDef->blkio.ndevices, + devices, ndevices, param->field) < 0) ret = -1; virBlkioDeviceArrayClear(devices, ndevices); VIR_FREE(devices); @@ -7753,6 +7842,7 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, VIR_TYPED_PARAM_UINT, val) < 0) goto cleanup; break; + case 1: /* blkiotune.device_weight */ if (vm->def->blkio.ndevices > 0) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -7782,6 +7872,122 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; break; + case 2: /* blkiotune.device_read_iops */ + if (vm->def->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < vm->def->blkio.ndevices; j++) { + if (!vm->def->blkio.devices[j].riops) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + vm->def->blkio.devices[j].path, + vm->def->blkio.devices[j].riops); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) + goto cleanup; + break; + + case 3: /* blkiotune.device_write_iops */ + if (vm->def->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < vm->def->blkio.ndevices; j++) { + if (!vm->def->blkio.devices[j].wiops) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + vm->def->blkio.devices[j].path, + vm->def->blkio.devices[j].wiops); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) + goto cleanup; + break; + + case 4: /* blkiotune.device_read_bps */ + if (vm->def->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < vm->def->blkio.ndevices; j++) { + if (!vm->def->blkio.devices[j].rbps) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%llu", + vm->def->blkio.devices[j].path, + vm->def->blkio.devices[j].rbps); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_READ_BPS, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) + goto cleanup; + break; + + case 5: /* blkiotune.device_write_bps */ + if (vm->def->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < vm->def->blkio.ndevices; j++) { + if (!vm->def->blkio.devices[j].wbps) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%llu", + vm->def->blkio.devices[j].path, + vm->def->blkio.devices[j].wbps); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) + goto cleanup; + break; + default: break; /* should not hit here */ @@ -7839,6 +8045,140 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, } break; + case 2: /* blkiotune.device_read_iops */ + if (persistentDef->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < persistentDef->blkio.ndevices; j++) { + if (!persistentDef->blkio.devices[j].riops) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + persistentDef->blkio.devices[j].path, + persistentDef->blkio.devices[j].riops); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) + goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS); + goto cleanup; + } + break; + case 3: /* blkiotune.device_write_iops */ + if (persistentDef->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < persistentDef->blkio.ndevices; j++) { + if (!persistentDef->blkio.devices[j].wiops) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + persistentDef->blkio.devices[j].path, + persistentDef->blkio.devices[j].wiops); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) + goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS); + goto cleanup; + } + break; + case 4: /* blkiotune.device_read_bps */ + if (persistentDef->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < persistentDef->blkio.ndevices; j++) { + if (!persistentDef->blkio.devices[j].rbps) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%llu", + persistentDef->blkio.devices[j].path, + persistentDef->blkio.devices[j].rbps); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) + goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_DEVICE_READ_BPS); + goto cleanup; + } + break; + + case 5: /* blkiotune.device_write_bps */ + if (persistentDef->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < persistentDef->blkio.ndevices; j++) { + if (!persistentDef->blkio.devices[j].wbps) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%llu", + persistentDef->blkio.devices[j].path, + persistentDef->blkio.devices[j].wbps); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) + goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS); + goto cleanup; + } + break; + default: break; /* should not hit here */ diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1fe138c..f01691d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1250,6 +1250,22 @@ static const vshCmdOptDef opts_blkiotune[] = { .type = VSH_OT_STRING, .help = N_("per-device IO Weights, in the form of /path/to/device,weight,...") }, + {.name = "device-read-iops", + .type = VSH_OT_STRING, + .help = N_("per-device read I/O limit per second, in the form of /path/to/device,read_iops,...") + }, + {.name = "device-write-iops", + .type = VSH_OT_STRING, + .help = N_("per-device write I/O limit per second, in the form of /path/to/device,write_iops,...") + }, + {.name = "device-read-bps", + .type = VSH_OT_STRING, + .help = N_("per-device bytes read per second, in the form of /path/to/device,read-bps,...") + }, + {.name = "device-write-bps", + .type = VSH_OT_STRING, + .help = N_("per-device bytes wrote per second, in the form of /path/to/device,write-bps,...") + }, {.name = "config", .type = VSH_OT_BOOL, .help = N_("affect next boot") @@ -1270,6 +1286,10 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) { virDomainPtr dom; const char *device_weight = NULL; + const char *device_riops = NULL; + const char *device_wiops = NULL; + const char *device_rbps = NULL; + const char *device_wbps = NULL; int weight = 0; int nparams = 0; int maxparams = 0; @@ -1317,6 +1337,50 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) goto save_error; } + rv = vshCommandOptString(cmd, "device-read-iops", &device_riops); + if (rv < 0) { + vshError(ctl, "%s", _("Unable to parse string parameter")); + goto cleanup; + } else if (rv > 0) { + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS, + device_riops) < 0) + goto save_error; + } + + rv = vshCommandOptString(cmd, "device-write-iops", &device_wiops); + if (rv < 0) { + vshError(ctl, "%s", _("Unable to parse string parameter")); + goto cleanup; + } else if (rv > 0) { + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS, + device_wiops) < 0) + goto save_error; + } + + rv = vshCommandOptString(cmd, "device-read-bps", &device_rbps); + if (rv < 0) { + vshError(ctl, "%s", _("Unable to parse string parameter")); + goto cleanup; + } else if (rv > 0) { + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLKIO_DEVICE_READ_BPS, + device_rbps) < 0) + goto save_error; + } + + rv = vshCommandOptString(cmd, "device-write-bps", &device_wbps); + if (rv < 0) { + vshError(ctl, "%s", _("Unable to parse string parameter")); + goto cleanup; + } else if (rv > 0) { + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS, + device_wbps) < 0) + goto save_error; + } + if (nparams == 0) { /* get the number of blkio parameters */ if (virDomainGetBlkioParameters(dom, NULL, &nparams, flags) != 0) { -- 1.8.3.1

On Mon, Dec 02, 2013 at 02:48:01PM +0800, Gao feng wrote:
With this patch, user can setup the throttle blkio cgorup for domain through the virsh cmd, such as:
virsh blkiotune domain1 --device-read-bps /dev/sda1,10000,/dev/sda2,20000 --device-write-bps /dev/sda1,10000 --device-read-iops /dev/sda1,10000 --device-write-iops /dev/sda1,10000,/dev/sda2,0
Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- include/libvirt/libvirt.h.in | 45 +++++ src/qemu/qemu_driver.c | 418 +++++++++++++++++++++++++++++++++++++++---- tools/virsh-domain.c | 64 +++++++ 3 files changed, 488 insertions(+), 39 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5aad75c..d054900 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1806,6 +1806,51 @@ char * virDomainGetSchedulerType(virDomainPtr domain,
#define VIR_DOMAIN_BLKIO_DEVICE_WEIGHT "device_weight"
+ +/** + * VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS: + * + * Macro for the blkio tunable throttle.read_iops_device: it represents + * the number of reading the block device per second, as a string. The + * string is parsed as a series of /path/to/device, read_iops elements, + * separated by ','. + */ + +#define VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS "device_read_iops" + + +/** + * VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS: + * + * Macro for the blkio tunable throttle.write_iops_device: it represents + * the number of writing the block device per second, as a string. The + * string is parsed as a series of /path/to/device, write_iops elements, + * separated by ','. + */ +#define VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS "device_write_iops" + + +/** + * VIR_DOMAIN_BLKIO_DEVICE_READ_BPS: + * + * Macro for the blkio tunable throttle.read_iops_device: it represents + * the bytes of reading the block device per second, as a string. The + * string is parsed as a series of /path/to/device, read_bps elements, + * separated by ','. + */ +#define VIR_DOMAIN_BLKIO_DEVICE_READ_BPS "device_read_bps" + + +/** + * VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS: + * + * Macro for the blkio tunable throttle.read_iops_device: it represents + * the number of reading the block device per second, as a string. The + * string is parsed as a series of /path/to/device, read_iops elements, + * separated by ','. + */ +#define VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS "device_write_bps"
Again please use naming that matches the existing VIR_DOMAIN_BLOCK_IOTUNE_* constants
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 61dbe7f..1ad5b94 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
The additions ot the QEMU driver should really be separate from the patch to the public API & virsh.
@@ -7753,6 +7842,7 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, VIR_TYPED_PARAM_UINT, val) < 0) goto cleanup; break; + case 1: /* blkiotune.device_weight */ if (vm->def->blkio.ndevices > 0) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -7782,6 +7872,122 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; break;
+ case 2: /* blkiotune.device_read_iops */
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1fe138c..f01691d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1250,6 +1250,22 @@ static const vshCmdOptDef opts_blkiotune[] = { .type = VSH_OT_STRING, .help = N_("per-device IO Weights, in the form of /path/to/device,weight,...") }, + {.name = "device-read-iops", + .type = VSH_OT_STRING, + .help = N_("per-device read I/O limit per second, in the form of /path/to/device,read_iops,...") + }, + {.name = "device-write-iops", + .type = VSH_OT_STRING, + .help = N_("per-device write I/O limit per second, in the form of /path/to/device,write_iops,...") + }, + {.name = "device-read-bps", + .type = VSH_OT_STRING, + .help = N_("per-device bytes read per second, in the form of /path/to/device,read-bps,...") + }, + {.name = "device-write-bps", + .type = VSH_OT_STRING, + .help = N_("per-device bytes wrote per second, in the form of /path/to/device,write-bps,...")
Again naming consistency with existing code please. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Add the manual for the new blkiotune options, --device-read-iops, --device-write-iops, --device-read-bps, --device-write-bps. Singed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Singed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- tools/virsh.pod | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index dac9a08..8ecc27c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1625,8 +1625,12 @@ The guaranteed minimum memory allocation for the guest. Specifying -1 as a value for these limits is interpreted as unlimited. =item B<blkiotune> I<domain> [I<--weight> B<weight>] -[I<--device-weights> B<device-weights>] [[I<--config>] -[I<--live>] | [I<--current>]] +[I<--device-weights> B<device-weights>] +[I<--device-read-iops> B<device-read-iops>] +[I<--device-write-iops> B<device-write-iops>] +[I<--device-read-bps> B<device-read-bps>] +[I<--device-write-bps> B<device-write-bps>] +[[I<--config>] [I<--live>] | [I<--current>]] Display or set the blkio parameters. QEMU/KVM supports I<--weight>. I<--weight> is in range [100, 1000]. After kernel 2.6.39, the value @@ -1639,6 +1643,34 @@ or the value 0 to remove that device from per-device listings. Only the devices listed in the string are modified; any existing per-device weights for other devices remain unchanged. +B<device-read-iops> is a single string listing one or more device/read_iops +pairs, int the format of /path/to/device,read_iops,/path/to/device,read_iops. +Each read_iops is a number which type is unsigned int, value 0 to remove that +device from per-decice listing. +Only the devices listed in the string are modified; +any existing per-device read_iops for other devices remain unchange. + +B<device-write-iops> is a single string listing one or more device/write_iops +pairs, int the format of /path/to/device,write_iops,/path/to/device,write_iops. +Each write_iops is a number which type is unsigned int, value 0 to remove that +device from per-decice listing. +Only the devices listed in the string are modified; +any existing per-device write_iops for other devices remain unchange. + +B<device-read-bps> is a single string listing one or more device/read_bps +pairs, int the format of /path/to/device,read_bps,/path/to/device,read_bps. +Each read_bps is a number which type is unsigned long long, value 0 to remove +that device from per-decice listing. +Only the devices listed in the string are modified; +any existing per-device read_bps for other devices remain unchange. + +B<device-write-bps> is a single string listing one or more device/write_bps +pairs, int the format of /path/to/device,write_bps,/path/to/device,write_bps. +Each write_bps is a number which type is unsigned long long, value 0 to remove +that device from per-decice listing. +Only the devices listed in the string are modified; +any existing per-device write_bps for other devices remain unchange. + 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. -- 1.8.3.1

On Mon, Dec 02, 2013 at 02:48:02PM +0800, Gao feng wrote:
Add the manual for the new blkiotune options, --device-read-iops, --device-write-iops, --device-read-bps, --device-write-bps.
Singed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Singed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- tools/virsh.pod | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod index dac9a08..8ecc27c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1625,8 +1625,12 @@ The guaranteed minimum memory allocation for the guest. Specifying -1 as a value for these limits is interpreted as unlimited.
=item B<blkiotune> I<domain> [I<--weight> B<weight>] -[I<--device-weights> B<device-weights>] [[I<--config>] -[I<--live>] | [I<--current>]] +[I<--device-weights> B<device-weights>] +[I<--device-read-iops> B<device-read-iops>] +[I<--device-write-iops> B<device-write-iops>] +[I<--device-read-bps> B<device-read-bps>] +[I<--device-write-bps> B<device-write-bps>] +[[I<--config>] [I<--live>] | [I<--current>]]
Display or set the blkio parameters. QEMU/KVM supports I<--weight>. I<--weight> is in range [100, 1000]. After kernel 2.6.39, the value @@ -1639,6 +1643,34 @@ or the value 0 to remove that device from per-device listings. Only the devices listed in the string are modified; any existing per-device weights for other devices remain unchanged.
+B<device-read-iops> is a single string listing one or more device/read_iops +pairs, int the format of /path/to/device,read_iops,/path/to/device,read_iops. +Each read_iops is a number which type is unsigned int, value 0 to remove that +device from per-decice listing. +Only the devices listed in the string are modified; +any existing per-device read_iops for other devices remain unchange. + +B<device-write-iops> is a single string listing one or more device/write_iops +pairs, int the format of /path/to/device,write_iops,/path/to/device,write_iops. +Each write_iops is a number which type is unsigned int, value 0 to remove that +device from per-decice listing. +Only the devices listed in the string are modified; +any existing per-device write_iops for other devices remain unchange. + +B<device-read-bps> is a single string listing one or more device/read_bps +pairs, int the format of /path/to/device,read_bps,/path/to/device,read_bps. +Each read_bps is a number which type is unsigned long long, value 0 to remove +that device from per-decice listing. +Only the devices listed in the string are modified; +any existing per-device read_bps for other devices remain unchange. + +B<device-write-bps> is a single string listing one or more device/write_bps +pairs, int the format of /path/to/device,write_bps,/path/to/device,write_bps. +Each write_bps is a number which type is unsigned long long, value 0 to remove +that device from per-decice listing. +Only the devices listed in the string are modified; +any existing per-device write_bps for other devices remain unchange. + 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.
The changes to virsh.c should be in this same patch. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

With this patch,user can set throttle blkio cgroup for lxc domain through virsh tool. The functions are copied from qemu_driver. Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_driver.c | 649 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 624 insertions(+), 25 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 61a90ca..873e14b 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1545,6 +1545,20 @@ static int lxcStateCleanup(void) return 0; } +static int +lxcConnectSupportsFeature(virConnectPtr conn, int feature) +{ + if (virConnectSupportsFeatureEnsureACL(conn) < 0) + return -1; + + switch (feature) { + case VIR_DRV_FEATURE_TYPED_PARAM_STRING: + return 1; + default: + return 0; + } +} + static int lxcConnectGetVersion(virConnectPtr conn, unsigned long *version) { @@ -1910,6 +1924,159 @@ lxcDomainGetSchedulerParameters(virDomainPtr domain, return lxcDomainGetSchedulerParametersFlags(domain, params, nparams, 0); } +static int +lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, + virBlkioDevicePtr *dev, size_t *size) +{ + char *temp; + int ndevices = 0; + int nsep = 0; + size_t i; + virBlkioDevicePtr result = NULL; + + *dev = NULL; + *size = 0; + + if (STREQ(blkioDeviceStr, "")) + return 0; + + temp = blkioDeviceStr; + while (temp) { + temp = strchr(temp, ','); + if (temp) { + temp++; + nsep++; + } + } + + /* A valid string must have even number of fields, hence an odd + * number of commas. */ + if (!(nsep & 1)) + goto error; + + ndevices = (nsep + 1) / 2; + + if (VIR_ALLOC_N(result, ndevices) < 0) + return -1; + + i = 0; + temp = blkioDeviceStr; + while (temp) { + char *p = temp; + + /* device path */ + p = strchr(p, ','); + if (!p) + goto error; + + if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0) + goto cleanup; + + /* value */ + temp = p + 1; + + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + if (virStrToLong_ui(temp, &p, 10, &result[i].weight) < 0) + goto error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { + if (virStrToLong_ui(temp, &p, 10, &result[i].riops) < 0) + goto error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { + if (virStrToLong_ui(temp, &p, 10, &result[i].wiops) < 0) + goto error; + } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { + if (virStrToLong_ull(temp, &p, 10, &result[i].rbps) < 0) + goto error; + } else { + if (virStrToLong_ull(temp, &p, 10, &result[i].wbps) < 0) + goto error; + } + + i++; + + if (*p == '\0') + break; + else if (*p != ',') + goto error; + temp = p + 1; + } + + if (!i) + VIR_FREE(result); + + *dev = result; + *size = i; + + return 0; + +error: + virReportError(VIR_ERR_INVALID_ARG, + _("unable to parse device weight '%s'"), blkioDeviceStr); +cleanup: + virBlkioDeviceArrayClear(result, ndevices); + VIR_FREE(result); + return -1; +} + +static int +lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array, + size_t *dest_size, + virBlkioDevicePtr src_array, + size_t src_size, + const char *type) +{ + size_t i, j; + virBlkioDevicePtr dest, src; + + for (i = 0; i < src_size; i++) { + bool found = false; + + src = &src_array[i]; + for (j = 0; j < *dest_size; j++) { + dest = &(*dest_array)[j]; + if (STREQ(src->path, dest->path)) { + found = true; + + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) + dest->weight = src->weight; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) + dest->riops = src->riops; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) + dest->wiops = src->wiops; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) + dest->rbps = src->rbps; + else + dest->wbps = src->wbps; + + break; + } + } + if (!found) { + if (!src->weight && !src->riops && !src->wiops && !src->rbps && !src->wbps) + continue; + if (VIR_EXPAND_N(*dest_array, *dest_size, 1) < 0) + return -1; + dest = &(*dest_array)[*dest_size - 1]; + dest->path = src->path; + + if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) + dest->weight = src->weight; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) + dest->riops = src->riops; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) + dest->wiops = src->wiops; + else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) + dest->rbps = src->rbps; + else + dest->wbps = src->wbps; + + src->path = NULL; + } + } + + return 0; +} + static int lxcDomainSetBlkioParameters(virDomainPtr dom, @@ -1918,26 +2085,37 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, unsigned int flags) { virLXCDriverPtr driver = dom->conn->privateData; - virCapsPtr caps = NULL; size_t i; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; int ret = -1; + virLXCDriverConfigPtr cfg = NULL; + virCapsPtr caps = NULL; virLXCDomainObjPrivatePtr priv; - virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); if (virTypedParamsValidate(params, nparams, VIR_DOMAIN_BLKIO_WEIGHT, VIR_TYPED_PARAM_UINT, + VIR_DOMAIN_BLKIO_DEVICE_WEIGHT, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_BLKIO_DEVICE_READ_BPS, + VIR_TYPED_PARAM_STRING, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS, + VIR_TYPED_PARAM_STRING, NULL) < 0) return -1; if (!(vm = lxcDomObjFromDomain(dom))) - goto cleanup; + return -1; priv = vm->privateData; + cfg = virLXCDriverGetConfig(driver); if (virDomainSetBlkioParametersEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; @@ -1945,8 +2123,8 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, - vm, &flags, &persistentDef) < 0) + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, + &persistentDef) < 0) goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { @@ -1955,7 +2133,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, _("blkio cgroup isn't mounted")); goto cleanup; } + } + ret = 0; + if (flags & VIR_DOMAIN_AFFECT_LIVE) { for (i = 0; i < nparams; i++) { virTypedParameterPtr param = ¶ms[i]; @@ -1963,14 +2144,86 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, if (params[i].value.ui > 1000 || params[i].value.ui < 100) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("out of blkio weight range.")); - goto cleanup; + ret = -1; + continue; } if (virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui) < 0) - goto cleanup; + ret = -1; + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + size_t ndevices; + virBlkioDevicePtr devices = NULL; + size_t j; + + if (lxcDomainParseBlkioDeviceStr(params[i].value.s, + param->field, + &devices, + &ndevices) < 0) { + ret = -1; + continue; + } + if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceWeight(priv->cgroup, + devices[j].path, + devices[j].weight) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceIops(priv->cgroup, + devices[j].path, + 1, devices[j].riops) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceIops(priv->cgroup, + devices[j].path, + 0, devices[j].wiops) < 0) { + ret = -1; + break; + } + } + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceBps(priv->cgroup, + devices[j].path, + 1, devices[j].rbps) < 0) { + ret = -1; + break; + } + } + } else { + for (j = 0; j < ndevices; j++) { + if (virCgroupSetBlkioDeviceBps(priv->cgroup, + devices[j].path, + 0, devices[j].wbps) < 0) { + ret = -1; + break; + } + } + } + if (j != ndevices || + lxcDomainMergeBlkioDevice(&vm->def->blkio.devices, + &vm->def->blkio.ndevices, + devices, ndevices, param->field) < 0) + ret = -1; + virBlkioDeviceArrayClear(devices, ndevices); + VIR_FREE(devices); } } } + if (ret < 0) + goto cleanup; if (flags & VIR_DOMAIN_AFFECT_CONFIG) { /* Clang can't see that if we get here, persistentDef was set. */ sa_assert(persistentDef); @@ -1982,18 +2235,39 @@ lxcDomainSetBlkioParameters(virDomainPtr dom, if (params[i].value.ui > 1000 || params[i].value.ui < 100) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("out of blkio weight range.")); - goto cleanup; + ret = -1; + continue; } persistentDef->blkio.weight = params[i].value.ui; + } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) || + STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { + virBlkioDevicePtr devices = NULL; + size_t ndevices; + + if (lxcDomainParseBlkioDeviceStr(params[i].value.s, + params[i].field, + &devices, + &ndevices) < 0) { + ret = -1; + continue; + } + if (lxcDomainMergeBlkioDevice(&persistentDef->blkio.devices, + &persistentDef->blkio.ndevices, + devices, ndevices, param->field) < 0) + ret = -1; + virBlkioDeviceArrayClear(devices, ndevices); + VIR_FREE(devices); } } if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0) - goto cleanup; + ret = -1; } - ret = 0; cleanup: if (vm) virObjectUnlock(vm); @@ -2003,7 +2277,8 @@ cleanup: } -#define LXC_NB_BLKIO_PARAM 1 +#define LXC_NB_BLKIO_PARAM 6 + static int lxcDomainGetBlkioParameters(virDomainPtr dom, virTypedParameterPtr params, @@ -2011,25 +2286,34 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, unsigned int flags) { virLXCDriverPtr driver = dom->conn->privateData; - virCapsPtr caps = NULL; - size_t i; + size_t i, j; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; unsigned int val; int ret = -1; + virCapsPtr caps = NULL; virLXCDomainObjPrivatePtr priv; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | - VIR_DOMAIN_AFFECT_CONFIG, -1); + VIR_DOMAIN_AFFECT_CONFIG | + VIR_TYPED_PARAM_STRING_OKAY, -1); + + /* We blindly return a string, and let libvirt.c and + * remote_driver.c do the filtering on behalf of older clients + * that can't parse it. */ + flags &= ~VIR_TYPED_PARAM_STRING_OKAY; if (!(vm = lxcDomObjFromDomain(dom))) - goto cleanup; + return -1; priv = vm->privateData; if (virDomainGetBlkioParametersEnsureACL(dom->conn, vm->def) < 0) goto cleanup; + if (!(caps = virLXCDriverGetCapabilities(driver, false))) + goto cleanup; + if ((*nparams) == 0) { /* Current number of blkio parameters supported by cgroups */ *nparams = LXC_NB_BLKIO_PARAM; @@ -2037,11 +2321,8 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; } - if (!(caps = virLXCDriverGetCapabilities(driver, false))) - goto cleanup; - - if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, - vm, &flags, &persistentDef) < 0) + if (virDomainLiveConfigHelperMethod(caps, driver->xmlopt, vm, &flags, + &persistentDef) < 0) goto cleanup; if (flags & VIR_DOMAIN_AFFECT_LIVE) { @@ -2064,7 +2345,151 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; break; - /* coverity[dead_error_begin] */ + case 1: /* blkiotune.device_weight */ + if (vm->def->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < vm->def->blkio.ndevices; j++) { + if (!vm->def->blkio.devices[j].weight) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + vm->def->blkio.devices[j].path, + vm->def->blkio.devices[j].weight); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_WEIGHT, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) + goto cleanup; + break; + + case 2: /* blkiotune.device_read_iops */ + if (vm->def->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < vm->def->blkio.ndevices; j++) { + if (!vm->def->blkio.devices[j].riops) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + vm->def->blkio.devices[j].path, + vm->def->blkio.devices[j].riops); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) + goto cleanup; + break; + + case 3: /* blkiotune.device_write_iops */ + if (vm->def->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < vm->def->blkio.ndevices; j++) { + if (!vm->def->blkio.devices[j].wiops) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + vm->def->blkio.devices[j].path, + vm->def->blkio.devices[j].wiops); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) + goto cleanup; + break; + + case 4: /* blkiotune.device_read_bps */ + if (vm->def->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < vm->def->blkio.ndevices; j++) { + if (!vm->def->blkio.devices[j].rbps) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%llu", + vm->def->blkio.devices[j].path, + vm->def->blkio.devices[j].rbps); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_READ_BPS, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) + goto cleanup; + break; + + case 5: /* blkiotune.device_write_bps */ + if (vm->def->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < vm->def->blkio.ndevices; j++) { + if (!vm->def->blkio.devices[j].wbps) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%llu", + vm->def->blkio.devices[j].path, + vm->def->blkio.devices[j].wbps); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS, + VIR_TYPED_PARAM_STRING, + param->value.s) < 0) + goto cleanup; + break; + default: break; /* should not hit here */ @@ -2073,16 +2498,189 @@ lxcDomainGetBlkioParameters(virDomainPtr dom, } else if (flags & VIR_DOMAIN_AFFECT_CONFIG) { for (i = 0; i < *nparams && i < LXC_NB_BLKIO_PARAM; i++) { virTypedParameterPtr param = ¶ms[i]; + val = 0; + param->value.ui = 0; + param->type = VIR_TYPED_PARAM_UINT; switch (i) { case 0: /* fill blkio weight here */ - if (virTypedParameterAssign(param, VIR_DOMAIN_BLKIO_WEIGHT, - VIR_TYPED_PARAM_UINT, - persistentDef->blkio.weight) < 0) + if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_WEIGHT) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_WEIGHT); + goto cleanup; + } + param->value.ui = persistentDef->blkio.weight; + break; + + case 1: /* blkiotune.device_weight */ + if (persistentDef->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < persistentDef->blkio.ndevices; j++) { + if (!persistentDef->blkio.devices[j].weight) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + persistentDef->blkio.devices[j].path, + persistentDef->blkio.devices[j].weight); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) + goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_DEVICE_WEIGHT); + goto cleanup; + } + break; + + case 2: /* blkiotune.device_read_iops */ + if (persistentDef->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < persistentDef->blkio.ndevices; j++) { + if (!persistentDef->blkio.devices[j].riops) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + persistentDef->blkio.devices[j].path, + persistentDef->blkio.devices[j].riops); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS); + goto cleanup; + } + break; + case 3: /* blkiotune.device_write_iops */ + if (persistentDef->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < persistentDef->blkio.ndevices; j++) { + if (!persistentDef->blkio.devices[j].wiops) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%u", + persistentDef->blkio.devices[j].path, + persistentDef->blkio.devices[j].wiops); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) + goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS); + goto cleanup; + } + break; + case 4: /* blkiotune.device_read_bps */ + if (persistentDef->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < persistentDef->blkio.ndevices; j++) { + if (!persistentDef->blkio.devices[j].rbps) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%llu", + persistentDef->blkio.devices[j].path, + persistentDef->blkio.devices[j].rbps); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) + goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_DEVICE_READ_BPS); + goto cleanup; + } + break; + + case 5: /* blkiotune.device_write_bps */ + if (persistentDef->blkio.ndevices > 0) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + bool comma = false; + + for (j = 0; j < persistentDef->blkio.ndevices; j++) { + if (!persistentDef->blkio.devices[j].wbps) + continue; + if (comma) + virBufferAddChar(&buf, ','); + else + comma = true; + virBufferAsprintf(&buf, "%s,%llu", + persistentDef->blkio.devices[j].path, + persistentDef->blkio.devices[j].wbps); + } + if (virBufferError(&buf)) { + virReportOOMError(); + goto cleanup; + } + param->value.s = virBufferContentAndReset(&buf); + } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) + goto cleanup; + param->type = VIR_TYPED_PARAM_STRING; + if (virStrcpyStatic(param->field, + VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), + VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS); + goto cleanup; + } break; - /* coverity[dead_error_begin] */ default: break; /* should not hit here */ @@ -4623,6 +5221,7 @@ static virDriver lxcDriver = { .name = LXC_DRIVER_NAME, .connectOpen = lxcConnectOpen, /* 0.4.2 */ .connectClose = lxcConnectClose, /* 0.4.2 */ + .connectSupportsFeature = lxcConnectSupportsFeature, /* 1.1.4 */ .connectGetVersion = lxcConnectGetVersion, /* 0.4.6 */ .connectGetHostname = lxcConnectGetHostname, /* 0.6.3 */ .connectGetSysinfo = lxcConnectGetSysinfo, /* 1.0.5 */ -- 1.8.3.1

Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml index 743cf29..a113efb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml @@ -8,10 +8,18 @@ <device> <path>/dev/sda</path> <weight>400</weight> + <read_iops>10000</read_iops> + <write_iops>10000</write_iops> + <read_bps>10000</read_bps> + <write_bps>10000</write_bps> </device> <device> <path>/dev/sdb</path> <weight>900</weight> + <read_iops>20000</read_iops> + <write_iops>20000</write_iops> + <read_bps>20000</read_bps> + <write_bps>20000</write_bps> </device> </blkiotune> <vcpu placement='static'>1</vcpu> -- 1.8.3.1

On Mon, Dec 02, 2013 at 02:48:04PM +0800, Gao feng wrote:
Signed-off-by: Guan Qiang <hzguanqiang@corp.netease.com> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml index 743cf29..a113efb 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.xml @@ -8,10 +8,18 @@ <device> <path>/dev/sda</path> <weight>400</weight> + <read_iops>10000</read_iops> + <write_iops>10000</write_iops> + <read_bps>10000</read_bps> + <write_bps>10000</write_bps> </device> <device> <path>/dev/sdb</path> <weight>900</weight> + <read_iops>20000</read_iops> + <write_iops>20000</write_iops> + <read_bps>20000</read_bps> + <write_bps>20000</write_bps> </device> </blkiotune> <vcpu placement='static'>1</vcpu>
Should be done in the same patch that modifies the XML parser or the QEMU driver. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 2013-12-02 14:47 , Gao feng wrote: Right now, libvirt only supports the cfq based blkio cgorup, this means if the block devices doesn't use cfq scheduler, the blkio cgroup will loss effect. This patchset adds the throttle blkio cgroup support for libvirt, intoduces four elements for domain configuration and extend the virsh command blkiotune. intoduces/introduces/s This patchset is a new version of Guan Qiang's patchset ://www.redhat.com/archives/libvir-list/2013-October/msg01066.html Change form Guan Qiang's patchset: 1, split to 8 patches, make logic more clear 2, change the type of read/write iops form unsigned long long to unsigned int, trying to set read/write iops to the value which bigger than max number of unsigned int will fail. 3, fix some logic shortage. Gao feng (9): rename virDomainBlkioDeviceWeightParseXML to virDomainBlkioDeviceParseXML rename virBlkioDeviceWeightArrayClear to virBlkioDeviceArrayClear rename virBlkioDeviceWeightPtr to virBlkioDevicePtr domain: introduce xml elements for throttle blkio cgroup blkio: Setting throttle blkio cgroup for domain qemu: allow to setup throttle blkio cgroup through virsh virsh: add virsh manual for setting throttle blkio cgroup lxc: allow to setup throttle blkio cgroup through virsh qemu: add new throttle blkio cgroup elements to the test xml docs/schemas/domaincommon.rng | 28 +- include/libvirt/libvirt.h.in | 45 ++ src/conf/domain_conf.c | 113 +++- src/conf/domain_conf.h | 16 +- src/libvirt_private.syms | 5 +- src/lxc/lxc_cgroup.c | 12 +- src/lxc/lxc_driver.c | 649 ++++++++++++++++++++- src/qemu/qemu_cgroup.c | 13 +- src/qemu/qemu_driver.c | 432 ++++++++++++-- src/util/vircgroup.c | 170 +++++- src/util/vircgroup.h | 18 + .../qemuxml2argv-blkiotune-device.xml | 8 + tools/virsh-domain.c | 64 ++ tools/virsh.pod | 36 +- 14 files changed, 1485 insertions(+), 124 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list ------------------ Best regards! GuanQiang 10:46:48

On 12/03/2013 10:48 AM, hzguanqiang@corp.netease.com wrote:
On 2013-12-02 14:47 , Gao feng <mailto:gaofeng@cn.fujitsu.com> wrote:
Right now, libvirt only supports the cfq based blkio cgorup, this means if the block devices doesn't use cfq scheduler, the blkio cgroup will loss effect.
This patchset adds the throttle blkio cgroup support for libvirt, intoduces four elements for domain configuration and extend the virsh command blkiotune.
intoduces/introduces/s
Oh, my hand slip. Thanks! BTW, chould you help to review or test this patchset? It runs well in my test box. Thanks.

I've tested the patchset successful with virsh command for both kvm and lxc vm. Details are as following: --------------------------------------KVM------------------------- root@debian:~/images# virsh version Compiled against library: libvirt 1.2.0 Using library: libvirt 1.2.0 Using API: QEMU 1.2.0 Running hypervisor: QEMU 1.7.0 root@debian:~/images# virsh blkiotune kvm weight : 500 device_weight : device_read_iops: device_write_iops: device_read_bps: device_write_bps: root@debian:~/images# virsh blkiotune kvm --device-weights /dev/vda,250 --device-read-iops /dev/vda,1234567 --device-write-iops /dev/vda,2345678 --device-read-bps /dev/vda,123456789 --device-write-bps /dev/vda,234567890 --config --live root@debian:~/images# virsh blkiotune kvm weight : 500 device_weight : /dev/vda,250 device_read_iops: /dev/vda,1234567 device_write_iops: /dev/vda,2345678 device_read_bps: /dev/vda,123456789 device_write_bps: /dev/vda,234567890 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/kvm.libvirt-qemu/blkio.weight_device 254:0 250 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/kvm.libvirt-qemu/blkio.throttle.read_bps_device 254:0 123456789 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/kvm.libvirt-qemu/blkio.throttle.read_iops_device 254:0 1234567 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/kvm.libvirt-qemu/blkio.throttle.write_bps_device 254:0 234567890 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/kvm.libvirt-qemu/blkio.throttle.write_iops_device 254:0 2345678 ---------------------------- lxc ---------------------------------- root@debian:~/images# vir version Compiled against library: libvirt 1.2.0 Using library: libvirt 1.2.0 Using API: LXC 1.2.0 Running hypervisor: LXC 3.10.11 root@debian:~/images# vir blkiotune lxc --device-weights /dev/vda,250 --device-read-iops /dev/vda,1234567 --device-write-iops /dev/vda,2345678 --device-read-bps /dev/vda,123456789 --device-write-bps /dev/vda,234567890 --config --live root@debian:~/images# vir blkiotune lxc weight : 500 device_weight : /dev/vda,250 device_read_iops: /dev/vda,1234567 device_write_iops: /dev/vda,2345678 device_read_bps: /dev/vda,123456789 device_write_bps: /dev/vda,234567890 root@debian:~/images# vir blkiotune lxc --device-weights /dev/vda,270 --device-read-iops /dev/vda,111111111 --device-write-iops /dev/vda,22222222 --device-read-bps /dev/vda,333336789 --device-write-bps /dev/vda,5555555 --config --live root@debian:~/images# vir blkiotune lxc weight : 500 device_weight : /dev/vda,270 device_read_iops: /dev/vda,111111111 device_write_iops: /dev/vda,22222222 device_read_bps: /dev/vda,333336789 device_write_bps: /dev/vda,5555555 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/lxc.libvirt-lxc/blkio.weight_device 254:0 270 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/lxc.libvirt-lxc/blkio.throttle.read_iops_device 254:0 111111111 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/lxc.libvirt-lxc/blkio.throttle.write_iops_device 254:0 22222222 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/lxc.libvirt-lxc/blkio.throttle.read_bps_device 254:0 333336789 root@debian:~/images# cat /sys/fs/cgroup/blkio/machine/lxc.libvirt-lxc/blkio.throttle.write_bps_device 254:0 5555555 On 2013-12-03 16:44 , Gao feng wrote: On 12/03/2013 10:48 AM, hzguanqiang@corp.netease.com wrote:
On 2013-12-02 14:47 , Gao feng <mailto:gaofeng@cn.fujitsu.com> wrote:
Right now, libvirt only supports the cfq based blkio cgorup, this means if the block devices doesn't use cfq scheduler, the blkio cgroup will loss effect.
This patchset adds the throttle blkio cgroup support for libvirt, intoduces four elements for domain configuration and extend the virsh command blkiotune.
intoduces/introduces/s
Oh, my hand slip. Thanks! BTW, chould you help to review or test this patchset? It runs well in my test box. Thanks. ------------------ Best regards! GuanQiang 12:46:48

Ping On 12/02/2013 02:47 PM, Gao feng wrote:
Right now, libvirt only supports the cfq based blkio cgorup, this means if the block devices doesn't use cfq scheduler, the blkio cgroup will loss effect.
This patchset adds the throttle blkio cgroup support for libvirt, intoduces four elements for domain configuration and extend the virsh command blkiotune.
This patchset is a new version of Guan Qiang's patchset ://www.redhat.com/archives/libvir-list/2013-October/msg01066.html
Change form Guan Qiang's patchset: 1, split to 8 patches, make logic more clear 2, change the type of read/write iops form unsigned long long to unsigned int, trying to set read/write iops to the value which bigger than max number of unsigned int will fail. 3, fix some logic shortage.
Gao feng (9): rename virDomainBlkioDeviceWeightParseXML to virDomainBlkioDeviceParseXML rename virBlkioDeviceWeightArrayClear to virBlkioDeviceArrayClear rename virBlkioDeviceWeightPtr to virBlkioDevicePtr domain: introduce xml elements for throttle blkio cgroup blkio: Setting throttle blkio cgroup for domain qemu: allow to setup throttle blkio cgroup through virsh virsh: add virsh manual for setting throttle blkio cgroup lxc: allow to setup throttle blkio cgroup through virsh qemu: add new throttle blkio cgroup elements to the test xml
docs/schemas/domaincommon.rng | 28 +- include/libvirt/libvirt.h.in | 45 ++ src/conf/domain_conf.c | 113 +++- src/conf/domain_conf.h | 16 +- src/libvirt_private.syms | 5 +- src/lxc/lxc_cgroup.c | 12 +- src/lxc/lxc_driver.c | 649 ++++++++++++++++++++- src/qemu/qemu_cgroup.c | 13 +- src/qemu/qemu_driver.c | 432 ++++++++++++-- src/util/vircgroup.c | 170 +++++- src/util/vircgroup.h | 18 + .../qemuxml2argv-blkiotune-device.xml | 8 + tools/virsh-domain.c | 64 ++ tools/virsh.pod | 36 +- 14 files changed, 1485 insertions(+), 124 deletions(-)
participants (3)
-
Daniel P. Berrange
-
Gao feng
-
hzguanqiang@corp.netease.com