[libvirt] [PATCH 4/8] Support block I/O throtte in XML
Enable block I/O throttle for per-disk in XML. Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 101 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++ src/qemu/qemu_command.c | 33 +++++++++++++++ src/util/xml.h | 2 + 4 files changed, 145 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58f4d0f..a157b80 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2318,7 +2318,8 @@ static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, virBitmapPtr bootMap, - unsigned int flags) + unsigned int flags, + xmlXPathContextPtr ctxt) { virDomainDiskDefPtr def; xmlNodePtr cur, child; @@ -2517,6 +2518,62 @@ virDomainDiskDefParseXML(virCapsPtr caps, } child = child->next; } + } else if (xmlStrEqual(cur->name, BAD_CAST "iotune")) { + if (virXPathULongLong("string(./devices/disk/iotune/total_bytes_sec)", + ctxt, &def->blkdeviotune.total_bytes_sec) < 0) { + def->blkdeviotune.total_bytes_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/read_bytes_sec)", + ctxt, &def->blkdeviotune.read_bytes_sec) < 0) { + def->blkdeviotune.read_bytes_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/write_bytes_sec)", + ctxt, &def->blkdeviotune.write_bytes_sec) < 0) { + def->blkdeviotune.write_bytes_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/total_iops_sec)", + ctxt, &def->blkdeviotune.total_iops_sec) < 0) { + def->blkdeviotune.total_iops_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/read_iops_sec)", + ctxt, &def->blkdeviotune.read_iops_sec) < 0) { + def->blkdeviotune.read_iops_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if (virXPathULongLong("string(./devices/disk/iotune/write_iops_sec)", + ctxt, &def->blkdeviotune.write_iops_sec) < 0) { + def->blkdeviotune.write_iops_sec = 0; + } else { + def->blkdeviotune.mark = 1; + } + + if ((def->blkdeviotune.total_bytes_sec && def->blkdeviotune.read_bytes_sec) + || (def->blkdeviotune.total_bytes_sec && def->blkdeviotune.write_bytes_sec)) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("total and read/write bytes_sec cannot be set at the same time")); + goto error; + } + + if ((def->blkdeviotune.total_iops_sec && def->blkdeviotune.read_iops_sec) + || (def->blkdeviotune.total_iops_sec && def->blkdeviotune.write_iops_sec)) { + virDomainReportError(VIR_ERR_XML_ERROR, + _("total and read/write iops_sec cannot be set at the same time")); + goto error; + } } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->readonly = 1; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { @@ -6003,7 +6060,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, - NULL, flags))) + NULL, flags, NULL))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -7076,7 +7133,8 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], bootMap, - flags); + flags, + ctxt); if (!disk) goto error; @@ -9511,6 +9569,43 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <target dev='%s' bus='%s'/>\n", def->dst, bus); + /*disk I/O throttling*/ + if (def->blkdeviotune.mark) { + virBufferAddLit(buf, " <iotune>\n"); + if (def->blkdeviotune.total_bytes_sec) { + virBufferAsprintf(buf, " <total_bytes_sec>%llu</total_bytes_sec>\n", + def->blkdeviotune.total_bytes_sec); + } + + if (def->blkdeviotune.read_bytes_sec) { + virBufferAsprintf(buf, " <read_bytes_sec>%llu</read_bytes_sec>\n", + def->blkdeviotune.read_bytes_sec); + + } + + if (def->blkdeviotune.write_bytes_sec) { + virBufferAsprintf(buf, " <write_bytes_sec>%llu</write_bytes_sec>\n", + def->blkdeviotune.write_bytes_sec); + } + + if (def->blkdeviotune.total_iops_sec) { + virBufferAsprintf(buf, " <total_iops_sec>%llu</total_iops_sec>\n", + def->blkdeviotune.total_iops_sec); + } + + if (def->blkdeviotune.read_iops_sec) { + virBufferAsprintf(buf, " <read_iops_sec>%llu</read_iops_sec>", + def->blkdeviotune.read_iops_sec); + } + + if (def->blkdeviotune.write_iops_sec) { + virBufferAsprintf(buf, " <write_iops_sec>%llu</write_iops_sec>", + def->blkdeviotune.write_iops_sec); + } + + virBufferAddLit(buf, " </iotune>\n"); + } + if (def->bootIndex) virBufferAsprintf(buf, " <boot order='%d'/>\n", def->bootIndex); if (def->readonly) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a3cb834..d95e239 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -333,6 +333,18 @@ struct _virDomainDiskDef { } auth; char *driverName; char *driverType; + + /*disk I/O throttling*/ + struct { + unsigned long long total_bytes_sec; + unsigned long long read_bytes_sec; + unsigned long long write_bytes_sec; + unsigned long long total_iops_sec; + unsigned long long read_iops_sec; + unsigned long long write_iops_sec; + unsigned int mark; + } blkdeviotune; + char *serial; int cachemode; int error_policy; /* enum virDomainDiskErrorPolicy */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2fbf691..91c6508 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1690,6 +1690,39 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } } + /*block I/O throttling*/ + if (disk->blkdeviotune.mark) { + if (disk->blkdeviotune.total_bytes_sec) { + virBufferAsprintf(&opt, ",bps=%llu", + disk->blkdeviotune.total_bytes_sec); + } + + if (disk->blkdeviotune.read_bytes_sec) { + virBufferAsprintf(&opt, ",bps_rd=%llu", + disk->blkdeviotune.read_bytes_sec); + } + + if (disk->blkdeviotune.write_bytes_sec) { + virBufferAsprintf(&opt, ",bps_wr=%llu", + disk->blkdeviotune.write_bytes_sec); + } + + if (disk->blkdeviotune.total_iops_sec) { + virBufferAsprintf(&opt, ",iops=%llu", + disk->blkdeviotune.total_iops_sec); + } + + if (disk->blkdeviotune.read_iops_sec) { + virBufferAsprintf(&opt, ",iops_rd=%llu", + disk->blkdeviotune.read_iops_sec); + } + + if (disk->blkdeviotune.write_iops_sec) { + virBufferAsprintf(&opt, ",iops_wr=%llu", + disk->blkdeviotune.write_iops_sec); + } + } + if (virBufferError(&opt)) { virReportOOMError(); goto error; diff --git a/src/util/xml.h b/src/util/xml.h index c492063..5742f19 100644 --- a/src/util/xml.h +++ b/src/util/xml.h @@ -50,6 +50,8 @@ xmlNodePtr virXPathNode(const char *xpath, int virXPathNodeSet(const char *xpath, xmlXPathContextPtr ctxt, xmlNodePtr **list); +int virXMLStringToULongLong (const char *stringval, + unsigned long long *value); char * virXMLPropString(xmlNodePtr node, const char *name); -- 1.7.1
subject line: s/throtte/throttle/ On 11/15/2011 02:02 AM, Lei Li wrote:
Enable block I/O throttle for per-disk in XML.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 101 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++ src/qemu/qemu_command.c | 33 +++++++++++++++ src/util/xml.h | 2 + 4 files changed, 145 insertions(+), 3 deletions(-)
I'm now applying this before 3/8.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58f4d0f..a157b80 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2318,7 +2318,8 @@ static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, virBitmapPtr bootMap, - unsigned int flags) + unsigned int flags, + xmlXPathContextPtr ctxt)
The ctxt parameter is usually passed right after node. I adjusted here and all callers.
{ virDomainDiskDefPtr def; xmlNodePtr cur, child; @@ -2517,6 +2518,62 @@ virDomainDiskDefParseXML(virCapsPtr caps, } child = child->next; } + } else if (xmlStrEqual(cur->name, BAD_CAST "iotune")) { + if (virXPathULongLong("string(./devices/disk/iotune/total_bytes_sec)", + ctxt, &def->blkdeviotune.total_bytes_sec) < 0) {
Indentation.
+ def->blkdeviotune.total_bytes_sec = 0; + } else { + def->blkdeviotune.mark = 1;
I'm not sure the mark field buys us anything. If 0 means unlimited, then either we have a limit (at least one field is non-zero) or we don't (all 6 fields are 0).
@@ -6003,7 +6060,7 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, - NULL, flags))) + NULL, flags, NULL)))
Why are you passing a NULL ctxt? That's a guaranteed NULL deref the xmlStrEqual(cur->name, BAD_CAST "iotune")) line hits, during any attempt to hot-plug or hot-unplug a disk with iotuning set.
@@ -9511,6 +9569,43 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " <target dev='%s' bus='%s'/>\n", def->dst, bus);
+ /*disk I/O throttling*/ + if (def->blkdeviotune.mark) {
Again, I'm not sure if a mark buys us anything.
+++ b/src/qemu/qemu_command.c @@ -1690,6 +1690,39 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } }
+ /*block I/O throttling*/ + if (disk->blkdeviotune.mark) {
This hunk belongs with 3/8 (qemu implementation), not here.
diff --git a/src/util/xml.h b/src/util/xml.h index c492063..5742f19 100644 --- a/src/util/xml.h +++ b/src/util/xml.h @@ -50,6 +50,8 @@ xmlNodePtr virXPathNode(const char *xpath, int virXPathNodeSet(const char *xpath, xmlXPathContextPtr ctxt, xmlNodePtr **list); +int virXMLStringToULongLong (const char *stringval, + unsigned long long *value);
This looks like leftovers from a failed experiment. Here's what I'm squashing in, so far: diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index fb86c18..a2702c5 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -2394,9 +2394,9 @@ cleanup: static virDomainDiskDefPtr virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, + xmlXPathContextPtr ctxt, virBitmapPtr bootMap, - unsigned int flags, - xmlXPathContextPtr ctxt) + unsigned int flags) { virDomainDiskDefPtr def; xmlNodePtr cur, child; @@ -2597,45 +2597,33 @@ virDomainDiskDefParseXML(virCapsPtr caps, } } else if (xmlStrEqual(cur->name, BAD_CAST "iotune")) { if (virXPathULongLong("string(./devices/disk/iotune/total_bytes_sec)", - ctxt, &def->blkdeviotune.total_bytes_sec) < 0) { + ctxt, &def->blkdeviotune.total_bytes_sec) < 0) { def->blkdeviotune.total_bytes_sec = 0; - } else { - def->blkdeviotune.mark = 1; } if (virXPathULongLong("string(./devices/disk/iotune/read_bytes_sec)", - ctxt, &def->blkdeviotune.read_bytes_sec) < 0) { + ctxt, &def->blkdeviotune.read_bytes_sec) < 0) { def->blkdeviotune.read_bytes_sec = 0; - } else { - def->blkdeviotune.mark = 1; } if (virXPathULongLong("string(./devices/disk/iotune/write_bytes_sec)", - ctxt, &def->blkdeviotune.write_bytes_sec) < 0) { + ctxt, &def->blkdeviotune.write_bytes_sec) < 0) { def->blkdeviotune.write_bytes_sec = 0; - } else { - def->blkdeviotune.mark = 1; } - if (virXPathULongLong("string(./devices/disk/iotune/total_iops_sec)", - ctxt, &def->blkdeviotune.total_iops_sec) < 0) { + if (virXPathULongLong("string(./devices/disk/iotune/total_iops_sec)", + ctxt, &def->blkdeviotune.total_iops_sec) < 0) { def->blkdeviotune.total_iops_sec = 0; - } else { - def->blkdeviotune.mark = 1; } if (virXPathULongLong("string(./devices/disk/iotune/read_iops_sec)", - ctxt, &def->blkdeviotune.read_iops_sec) < 0) { + ctxt, &def->blkdeviotune.read_iops_sec) < 0) { def->blkdeviotune.read_iops_sec = 0; - } else { - def->blkdeviotune.mark = 1; } if (virXPathULongLong("string(./devices/disk/iotune/write_iops_sec)", - ctxt, &def->blkdeviotune.write_iops_sec) < 0) { + ctxt, &def->blkdeviotune.write_iops_sec) < 0) { def->blkdeviotune.write_iops_sec = 0; - } else { - def->blkdeviotune.mark = 1; } if ((def->blkdeviotune.total_bytes_sec && def->blkdeviotune.read_bytes_sec) @@ -6135,8 +6123,8 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; - if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, - NULL, flags, NULL))) + if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, + NULL, flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -7205,9 +7193,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, for (i = 0 ; i < n ; i++) { virDomainDiskDefPtr disk = virDomainDiskDefParseXML(caps, nodes[i], + ctxt, bootMap, - flags, - ctxt); + flags); if (!disk) goto error; @@ -9648,7 +9636,12 @@ virDomainDiskDefFormat(virBufferPtr buf, def->dst, bus); /*disk I/O throttling*/ - if (def->blkdeviotune.mark) { + if (def->blkdeviotune.total_bytes_sec || + def->blkdeviotune.read_bytes_sec || + def->blkdeviotune.write_bytes_sec || + def->blkdeviotune.total_iops_sec || + def->blkdeviotune.read_iops_sec || + def->blkdeviotune.write_iops_sec) { virBufferAddLit(buf, " <iotune>\n"); if (def->blkdeviotune.total_bytes_sec) { virBufferAsprintf(buf, " <total_bytes_sec>%llu</total_bytes_sec>\n", diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h index 04cd7d8..9180769 100644 --- i/src/conf/domain_conf.h +++ w/src/conf/domain_conf.h @@ -344,7 +344,6 @@ struct _virDomainDiskDef { unsigned long long total_iops_sec; unsigned long long read_iops_sec; unsigned long long write_iops_sec; - unsigned int mark; } blkdeviotune; char *serial; diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c index 449b685..85b34bb 100644 --- i/src/qemu/qemu_command.c +++ w/src/qemu/qemu_command.c @@ -1866,39 +1866,6 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, } } - /*block I/O throttling*/ - if (disk->blkdeviotune.mark) { - if (disk->blkdeviotune.total_bytes_sec) { - virBufferAsprintf(&opt, ",bps=%llu", - disk->blkdeviotune.total_bytes_sec); - } - - if (disk->blkdeviotune.read_bytes_sec) { - virBufferAsprintf(&opt, ",bps_rd=%llu", - disk->blkdeviotune.read_bytes_sec); - } - - if (disk->blkdeviotune.write_bytes_sec) { - virBufferAsprintf(&opt, ",bps_wr=%llu", - disk->blkdeviotune.write_bytes_sec); - } - - if (disk->blkdeviotune.total_iops_sec) { - virBufferAsprintf(&opt, ",iops=%llu", - disk->blkdeviotune.total_iops_sec); - } - - if (disk->blkdeviotune.read_iops_sec) { - virBufferAsprintf(&opt, ",iops_rd=%llu", - disk->blkdeviotune.read_iops_sec); - } - - if (disk->blkdeviotune.write_iops_sec) { - virBufferAsprintf(&opt, ",iops_wr=%llu", - disk->blkdeviotune.write_iops_sec); - } - } - if (virBufferError(&opt)) { virReportOOMError(); goto error; diff --git i/src/util/xml.h w/src/util/xml.h index 5742f19..c492063 100644 --- i/src/util/xml.h +++ w/src/util/xml.h @@ -50,8 +50,6 @@ xmlNodePtr virXPathNode(const char *xpath, int virXPathNodeSet(const char *xpath, xmlXPathContextPtr ctxt, xmlNodePtr **list); -int virXMLStringToULongLong (const char *stringval, - unsigned long long *value); char * virXMLPropString(xmlNodePtr node, const char *name); -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/22/2011 09:45 PM, Eric Blake wrote:
subject line: s/throtte/throttle/
On 11/15/2011 02:02 AM, Lei Li wrote:
Enable block I/O throttle for per-disk in XML.
Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> --- src/conf/domain_conf.c | 101 +++++++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 12 ++++++ src/qemu/qemu_command.c | 33 +++++++++++++++ src/util/xml.h | 2 + 4 files changed, 145 insertions(+), 3 deletions(-)
I'm now applying this before 3/8.
Here's what I'm squashing in, so far:
And one more (read my upcoming comments on 3/8 for why). diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h index 9180769..ff6921a 100644 --- i/src/conf/domain_conf.h +++ w/src/conf/domain_conf.h @@ -313,6 +313,17 @@ enum virDomainDiskSecretType { VIR_DOMAIN_DISK_SECRET_TYPE_LAST }; +typedef struct _virDomainBlockIoTuneInfo virDomainBlockIoTuneInfo; +struct _virDomainBlockIoTuneInfo { + unsigned long long total_bytes_sec; + unsigned long long read_bytes_sec; + unsigned long long write_bytes_sec; + unsigned long long total_iops_sec; + unsigned long long read_iops_sec; + unsigned long long write_iops_sec; +}; +typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; + /* Stores the virtual disk configuration */ typedef struct _virDomainDiskDef virDomainDiskDef; typedef virDomainDiskDef *virDomainDiskDefPtr; @@ -336,15 +347,7 @@ struct _virDomainDiskDef { char *driverName; char *driverType; - /*disk I/O throttling*/ - struct { - unsigned long long total_bytes_sec; - unsigned long long read_bytes_sec; - unsigned long long write_bytes_sec; - unsigned long long total_iops_sec; - unsigned long long read_iops_sec; - unsigned long long write_iops_sec; - } blkdeviotune; + virDomainBlockIoTuneInfo blkdeviotune; char *serial; int cachemode; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Lei Li