Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

2011-11-29 Thread Daniel P. Berrange
On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote:
 From: Hu Tao hu...@cn.fujitsu.com
 
 Implement setting/getting per-device blkio weights in qemu,
 using the cgroups blkio.weight_device tunable.
 ---
  src/libvirt_private.syms   |1 +
  src/qemu/qemu_cgroup.c |   20 ++
  src/qemu/qemu_driver.c |  218 
 +++-
  src/util/cgroup.c  |   51 +
  src/util/cgroup.h  |4 +
  .../qemuxml2argv-blkiotune-device.args |4 +
  tests/qemuxml2argvtest.c   |1 +
  tests/qemuxml2xmltest.c|1 +
  8 files changed, 295 insertions(+), 5 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.args

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

2011-11-29 Thread Daniel P. Berrange
On Tue, Nov 15, 2011 at 03:22:03PM +0800, Hu Tao wrote:
 On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote:
  From: Hu Tao hu...@cn.fujitsu.com
  
  Implement setting/getting per-device blkio weights in qemu,
  using the cgroups blkio.weight_device tunable.
  ---
   src/libvirt_private.syms   |1 +
   src/qemu/qemu_cgroup.c |   20 ++
   src/qemu/qemu_driver.c |  218 
  +++-
   src/util/cgroup.c  |   51 +
   src/util/cgroup.h  |4 +
   .../qemuxml2argv-blkiotune-device.args |4 +
   tests/qemuxml2argvtest.c   |1 +
   tests/qemuxml2xmltest.c|1 +
   8 files changed, 295 insertions(+), 5 deletions(-)
   create mode 100644 
  tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.args
  
  diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
  index d78fd53..3575fe0 100644
  --- a/src/libvirt_private.syms
  +++ b/src/libvirt_private.syms
  @@ -89,6 +89,7 @@ virCgroupKillRecursive;
   virCgroupMounted;
   virCgroupPathOfController;
   virCgroupRemove;
  +virCgroupSetBlkioDeviceWeight;
   virCgroupSetBlkioWeight;
   virCgroupSetCpuShares;
   virCgroupSetCpuCfsPeriod;
  diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
  index 2a10bd2..eda4c66 100644
  --- a/src/qemu/qemu_cgroup.c
  +++ b/src/qemu/qemu_cgroup.c
  @@ -312,6 +312,26 @@ int qemuSetupCgroup(struct qemud_driver *driver,
   }
   }
  
  +if (vm-def-blkio.ndevices) {
  +if (qemuCgroupControllerActive(driver, 
  VIR_CGROUP_CONTROLLER_BLKIO)) {
  +for (i = 0; i  vm-def-blkio.ndevices; i++) {
  +virBlkioDeviceWeightPtr dw = vm-def-blkio.devices[i];
  +rc = virCgroupSetBlkioDeviceWeight(cgroup, dw-path,
  +   dw-weight);
  +if (rc != 0) {
  +virReportSystemError(-rc,
  + _(Unable to set io device weight 
  
  +   for domain %s),
  + vm-def-name);
  +goto cleanup;
  +}
  +}
  +} else {
  +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
  +_(Block I/O tuning is not available on this 
  host));
  +}
  +}
  +
   if (vm-def-mem.hard_limit != 0 ||
   vm-def-mem.soft_limit != 0 ||
   vm-def-mem.swap_hard_limit != 0) {
  diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
  index b0ce115..43f4041 100644
  --- a/src/qemu/qemu_driver.c
  +++ b/src/qemu/qemu_driver.c
  @@ -112,7 +112,7 @@
   # define KVM_CAP_NR_VCPUS 9   /* returns max vcpus per vm */
   #endif
  
  -#define QEMU_NB_BLKIO_PARAM  1
  +#define QEMU_NB_BLKIO_PARAM  2
  
   static void processWatchdogEvent(void *data, void *opaque);
  
  @@ -5885,6 +5885,105 @@ cleanup:
   return ret;
   }
  
  +/* deviceWeightStr in the form of /device/path,weight,/device/path,weight
  + * for example, /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0,800
  + */
  +static int
  +parseBlkioWeightDeviceStr(char *deviceWeightStr,
  +  virBlkioDeviceWeightPtr *dw, int *size,
  +  virCgroupPtr cgroup)
  +{
  +char *temp;
  +int ndevices = 0;
  +int nsep = 0;
  +int i;
  +virBlkioDeviceWeightPtr result = NULL;
  +
  +temp = deviceWeightStr;
  +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) {
  +virReportOOMError();
  +return -1;
  +}
  +
  +i = 0;
  +temp = deviceWeightStr;
  +while (temp) {
  +char *p = temp;
  +
  +/* device path */
  +p = strchr(p, ',');
  +if (!p)
  +goto error;
  +
  +result[i].path = strndup(temp, p - temp);
  +if (!result[i].path) {
  +virReportOOMError();
  +goto cleanup;
  +}
  +
  +/* weight */
  +temp = p + 1;
  +
  +if (virStrToLong_ui(temp, p, 10, result[i].weight)  0)
  +goto error;
  +
  +if (cgroup) {
  +int rc = virCgroupSetBlkioDeviceWeight(cgroup,
  +   result[i].path,
  +   result[i].weight);
 
 Does more than the function name says. Would it be better to just do the
 parsing here, and set the cgroup values after parsing(see my comment to
 

Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

2011-11-29 Thread Eric Blake
On 11/14/2011 09:30 PM, Eric Blake wrote:
 From: Hu Tao hu...@cn.fujitsu.com
 
 Implement setting/getting per-device blkio weights in qemu,
 using the cgroups blkio.weight_device tunable.

 +if (vm-def-blkio.ndevices) {
 +if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) 
 {
 +for (i = 0; i  vm-def-blkio.ndevices; i++) {
 +virBlkioDeviceWeightPtr dw = vm-def-blkio.devices[i];
 +rc = virCgroupSetBlkioDeviceWeight(cgroup, dw-path,
 +   dw-weight);
 +if (rc != 0) {
 +virReportSystemError(-rc,
 + _(Unable to set io device weight 
 +   for domain %s),
 + vm-def-name);
 +goto cleanup;
 +}
 +}
 +} else {
 +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +_(Block I/O tuning is not available on this 
 host));
 +}

Copy and paste, but this should 'goto cleanup' to ensure the error is
propagated.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

2011-11-29 Thread Eric Blake
On 11/15/2011 12:22 AM, Hu Tao wrote:
 On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote:
 From: Hu Tao hu...@cn.fujitsu.com

 Implement setting/getting per-device blkio weights in qemu,
 using the cgroups blkio.weight_device tunable.
 
 Does more than the function name says. Would it be better to just do the
 parsing here, and set the cgroup values after parsing(see my comment to
 patch 3 for filtering out 0-weight when writing to xml):

ACK to your improvements for setting, but you forgot to do 0-weight
filtering from the getting side:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index a8fea6c..105bdde 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -6270,9 +6270,14 @@ static int
qemuDomainGetBlkioParameters(virDomainPtr dom,
 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 (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);
@@ -6282,7 +6287,8 @@ static int
qemuDomainGetBlkioParameters(virDomainPtr dom,
 goto cleanup;
 }
 param-value.s = virBufferContentAndReset(buf);
-} else {
+}
+if (!param-value.s) {
 param-value.s = strdup();
 if (!param-value.s) {
 virReportOOMError();


-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

2011-11-29 Thread Eric Blake
On 11/29/2011 10:14 AM, Daniel P. Berrange wrote:

 Does more than the function name says. Would it be better to just do the
 parsing here, and set the cgroup values after parsing(see my comment to
 patch 3 for filtering out 0-weight when writing to xml):

 
 Yes, IMHO it is better to do the setting of values, after parsing
 is fully complete.

Okay, the series is now pushed with this last round of improvements
included.  However, I'm still working on a 5/4.  Consider:

# virsh blkiotune dom --device-weights /dev/sda,502,/dev/sdb,498
# virsh blkiotune dom --device-weights /dev/sda,503
# virsh blkiotune dom
weight : 500
device_weight  : /dev/sda,503
# cat /cgroup/blkio/libvirt/qemu/dom/blkio.weight_device
8:0 503
8:16498

Oops - /dev/sdb wasn't cleared, even though our new query no longer
reports it.

Calling virsh blkiotune dome --device-weights should either clear all
existing weights before writing in just the specified weights of the new
string.  Or it should only affect the weights of paths specified in the
new string, while preserving all pre-existing weights for all other paths.

I'm leaning towards the latter (that is, you _have_ to pass an explicit
0 to set in order to remove /dev/sdb from the XML and from get, and that
mentioning just /dev/sda on set leaves /dev/sdb's setting alone).
Besides, that's how cgroups does it - write an explicit 0 to remove a
device from the cgroup file; all other writes modify a device if it is
already listed, otherwise they add a new line to the file.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

2011-11-21 Thread Eric Blake
On 11/15/2011 03:33 PM, Eric Blake wrote:
 On 11/15/2011 12:22 AM, Hu Tao wrote:
 On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote:
 From: Hu Tao hu...@cn.fujitsu.com

 Implement setting/getting per-device blkio weights in qemu,
 using the cgroups blkio.weight_device tunable.
 

 +/* deviceWeightStr in the form of /device/path,weight,/device/path,weight
 + * for example, /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0,800
 + */
 +static int
 +parseBlkioWeightDeviceStr(char *deviceWeightStr,
 +  virBlkioDeviceWeightPtr *dw, int *size,
 +  virCgroupPtr cgroup)
 

 Does more than the function name says. Would it be better to just do the
 parsing here, and set the cgroup values after parsing(see my comment to
 patch 3 for filtering out 0-weight when writing to xml):
 
 Either approach works for me (renaming this function more accurately so
 that the domain_conf never has 0-weight entries, or going with your
 improvements to allow 0-weight entries in the domain_conf def but not
 expose them in the XML).  I guess whoever reviews this series can cast
 the deciding vote on which of the two approaches we commit.

Ping - this series seems worth including in 0.9.8; can we get a
third-party review?

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

2011-11-15 Thread Eric Blake
On 11/15/2011 12:22 AM, Hu Tao wrote:
 On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote:
 From: Hu Tao hu...@cn.fujitsu.com

 Implement setting/getting per-device blkio weights in qemu,
 using the cgroups blkio.weight_device tunable.


 +/* deviceWeightStr in the form of /device/path,weight,/device/path,weight
 + * for example, /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0,800
 + */
 +static int
 +parseBlkioWeightDeviceStr(char *deviceWeightStr,
 +  virBlkioDeviceWeightPtr *dw, int *size,
 +  virCgroupPtr cgroup)

 
 Does more than the function name says. Would it be better to just do the
 parsing here, and set the cgroup values after parsing(see my comment to
 patch 3 for filtering out 0-weight when writing to xml):

Either approach works for me (renaming this function more accurately so
that the domain_conf never has 0-weight entries, or going with your
improvements to allow 0-weight entries in the domain_conf def but not
expose them in the XML).  I guess whoever reviews this series can cast
the deciding vote on which of the two approaches we commit.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

2011-11-14 Thread Eric Blake
From: Hu Tao hu...@cn.fujitsu.com

Implement setting/getting per-device blkio weights in qemu,
using the cgroups blkio.weight_device tunable.
---
 src/libvirt_private.syms   |1 +
 src/qemu/qemu_cgroup.c |   20 ++
 src/qemu/qemu_driver.c |  218 +++-
 src/util/cgroup.c  |   51 +
 src/util/cgroup.h  |4 +
 .../qemuxml2argv-blkiotune-device.args |4 +
 tests/qemuxml2argvtest.c   |1 +
 tests/qemuxml2xmltest.c|1 +
 8 files changed, 295 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.args

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d78fd53..3575fe0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -89,6 +89,7 @@ virCgroupKillRecursive;
 virCgroupMounted;
 virCgroupPathOfController;
 virCgroupRemove;
+virCgroupSetBlkioDeviceWeight;
 virCgroupSetBlkioWeight;
 virCgroupSetCpuShares;
 virCgroupSetCpuCfsPeriod;
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2a10bd2..eda4c66 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -312,6 +312,26 @@ int qemuSetupCgroup(struct qemud_driver *driver,
 }
 }

+if (vm-def-blkio.ndevices) {
+if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
+for (i = 0; i  vm-def-blkio.ndevices; i++) {
+virBlkioDeviceWeightPtr dw = vm-def-blkio.devices[i];
+rc = virCgroupSetBlkioDeviceWeight(cgroup, dw-path,
+   dw-weight);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set io device weight 
+   for domain %s),
+ vm-def-name);
+goto cleanup;
+}
+}
+} else {
+qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+_(Block I/O tuning is not available on this 
host));
+}
+}
+
 if (vm-def-mem.hard_limit != 0 ||
 vm-def-mem.soft_limit != 0 ||
 vm-def-mem.swap_hard_limit != 0) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b0ce115..43f4041 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -112,7 +112,7 @@
 # define KVM_CAP_NR_VCPUS 9   /* returns max vcpus per vm */
 #endif

-#define QEMU_NB_BLKIO_PARAM  1
+#define QEMU_NB_BLKIO_PARAM  2

 static void processWatchdogEvent(void *data, void *opaque);

@@ -5885,6 +5885,105 @@ cleanup:
 return ret;
 }

+/* deviceWeightStr in the form of /device/path,weight,/device/path,weight
+ * for example, /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0,800
+ */
+static int
+parseBlkioWeightDeviceStr(char *deviceWeightStr,
+  virBlkioDeviceWeightPtr *dw, int *size,
+  virCgroupPtr cgroup)
+{
+char *temp;
+int ndevices = 0;
+int nsep = 0;
+int i;
+virBlkioDeviceWeightPtr result = NULL;
+
+temp = deviceWeightStr;
+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) {
+virReportOOMError();
+return -1;
+}
+
+i = 0;
+temp = deviceWeightStr;
+while (temp) {
+char *p = temp;
+
+/* device path */
+p = strchr(p, ',');
+if (!p)
+goto error;
+
+result[i].path = strndup(temp, p - temp);
+if (!result[i].path) {
+virReportOOMError();
+goto cleanup;
+}
+
+/* weight */
+temp = p + 1;
+
+if (virStrToLong_ui(temp, p, 10, result[i].weight)  0)
+goto error;
+
+if (cgroup) {
+int rc = virCgroupSetBlkioDeviceWeight(cgroup,
+   result[i].path,
+   result[i].weight);
+if (rc  0) {
+virReportSystemError(-rc,
+ _(Unable to set io device weight 
+   for path %s),
+ result[i].path);
+goto cleanup;
+}
+}
+
+/* 0-weight entries only affect cgroups, they don't stick in xml */
+if (result[i].weight)
+i++;
+else
+VIR_FREE(result[i].path);
+if (*p == '\0')
+break;
+else if (*p != 

Re: [libvirt] [PATCHv4 4/4] blkiotune: add qemu support for blkiotune.device_weight

2011-11-14 Thread Hu Tao
On Mon, Nov 14, 2011 at 09:30:02PM -0700, Eric Blake wrote:
 From: Hu Tao hu...@cn.fujitsu.com
 
 Implement setting/getting per-device blkio weights in qemu,
 using the cgroups blkio.weight_device tunable.
 ---
  src/libvirt_private.syms   |1 +
  src/qemu/qemu_cgroup.c |   20 ++
  src/qemu/qemu_driver.c |  218 
 +++-
  src/util/cgroup.c  |   51 +
  src/util/cgroup.h  |4 +
  .../qemuxml2argv-blkiotune-device.args |4 +
  tests/qemuxml2argvtest.c   |1 +
  tests/qemuxml2xmltest.c|1 +
  8 files changed, 295 insertions(+), 5 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkiotune-device.args
 
 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index d78fd53..3575fe0 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -89,6 +89,7 @@ virCgroupKillRecursive;
  virCgroupMounted;
  virCgroupPathOfController;
  virCgroupRemove;
 +virCgroupSetBlkioDeviceWeight;
  virCgroupSetBlkioWeight;
  virCgroupSetCpuShares;
  virCgroupSetCpuCfsPeriod;
 diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
 index 2a10bd2..eda4c66 100644
 --- a/src/qemu/qemu_cgroup.c
 +++ b/src/qemu/qemu_cgroup.c
 @@ -312,6 +312,26 @@ int qemuSetupCgroup(struct qemud_driver *driver,
  }
  }
 
 +if (vm-def-blkio.ndevices) {
 +if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) 
 {
 +for (i = 0; i  vm-def-blkio.ndevices; i++) {
 +virBlkioDeviceWeightPtr dw = vm-def-blkio.devices[i];
 +rc = virCgroupSetBlkioDeviceWeight(cgroup, dw-path,
 +   dw-weight);
 +if (rc != 0) {
 +virReportSystemError(-rc,
 + _(Unable to set io device weight 
 +   for domain %s),
 + vm-def-name);
 +goto cleanup;
 +}
 +}
 +} else {
 +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +_(Block I/O tuning is not available on this 
 host));
 +}
 +}
 +
  if (vm-def-mem.hard_limit != 0 ||
  vm-def-mem.soft_limit != 0 ||
  vm-def-mem.swap_hard_limit != 0) {
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index b0ce115..43f4041 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -112,7 +112,7 @@
  # define KVM_CAP_NR_VCPUS 9   /* returns max vcpus per vm */
  #endif
 
 -#define QEMU_NB_BLKIO_PARAM  1
 +#define QEMU_NB_BLKIO_PARAM  2
 
  static void processWatchdogEvent(void *data, void *opaque);
 
 @@ -5885,6 +5885,105 @@ cleanup:
  return ret;
  }
 
 +/* deviceWeightStr in the form of /device/path,weight,/device/path,weight
 + * for example, /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0,800
 + */
 +static int
 +parseBlkioWeightDeviceStr(char *deviceWeightStr,
 +  virBlkioDeviceWeightPtr *dw, int *size,
 +  virCgroupPtr cgroup)
 +{
 +char *temp;
 +int ndevices = 0;
 +int nsep = 0;
 +int i;
 +virBlkioDeviceWeightPtr result = NULL;
 +
 +temp = deviceWeightStr;
 +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) {
 +virReportOOMError();
 +return -1;
 +}
 +
 +i = 0;
 +temp = deviceWeightStr;
 +while (temp) {
 +char *p = temp;
 +
 +/* device path */
 +p = strchr(p, ',');
 +if (!p)
 +goto error;
 +
 +result[i].path = strndup(temp, p - temp);
 +if (!result[i].path) {
 +virReportOOMError();
 +goto cleanup;
 +}
 +
 +/* weight */
 +temp = p + 1;
 +
 +if (virStrToLong_ui(temp, p, 10, result[i].weight)  0)
 +goto error;
 +
 +if (cgroup) {
 +int rc = virCgroupSetBlkioDeviceWeight(cgroup,
 +   result[i].path,
 +   result[i].weight);

Does more than the function name says. Would it be better to just do the
parsing here, and set the cgroup values after parsing(see my comment to
patch 3 for filtering out 0-weight when writing to xml):

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 43f4041..275d5c8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@