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@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Zhi Yong Wu <wuzhy@xxxxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list