Re: [libvirt] [PATCHv6 5/5] blkiotune: add qemu support for blkiotune.device_weight

2011-11-06 Thread Xu He Jie

于 2011年11月04日 02:06, Eric Blake 写道:

From: Hu Taohu...@cn.fujitsu.com

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

v5: did not include this patch
v6: split v4 2/2 into two parts; this part covers just the qemu.
Rebase to accomodate 'device_weight' naming and VIR_TYPED_PARAM_STRING
handling.

This patch is broken, with several flaws:
1. 'virsh blkiotune dom --device-weight /dev/sda,500 --live' does not
alter the 'virsh dumpxml dom' live XML.  The cgroups change is
active, but unless we also reflect the change in the eventual xml dump,
then we aren't accurately telling the user the current state of the
domain.  This means that qemuDomainSetBlkioParameters needs a fix.

2. 'virsh blkiotune dom --live' now shows output that looks like:
weight  : 500
device_weight   : 8,0   500
whereas 'virsh blkiotune dom --config' shows:
weight  : 500
device_weight   : /dev/sda,500

The --config version is correct - it accurately reflects the XML.
The --live version is wrong - we MUST NOT expose major,minor
back to the user, since we intentionally decided that the XML
should track only the device name, not the major/minor implementation
detail of cgroups.  I think the correct fix would be to just delete
virCgroupGetBlkioDeviceWeight and treat cgroups blkio.weight_device
as write-only, and instead have qemuDomainGetBlkioParameters always
produce output from the domain definition (--live vs. --config merely
choosing between vm-def vs. persistentDef), which stems back to my
solution of point 1 in having in vm-def accurately reflect whatever
is written into cgroups.

  src/qemu/qemu_cgroup.c |   22 ++
  src/qemu/qemu_driver.c |  191 ++-
  src/util/cgroup.c  |   33 
  src/util/cgroup.h  |4 +
  4 files changed, 245 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2a10bd2..d397578 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -312,6 +312,28 @@ int qemuSetupCgroup(struct qemud_driver *driver,
  }
  }

+if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
+char *tmp;
+if (virBlkioDeviceWeightToStr(vm-def-blkio.devices,
+  vm-def-blkio.ndevices,
+tmp)  0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(Unable to set io device weight for domain %s),
+vm-def-name);
+goto cleanup;
+}
+if (tmp) {
+rc = virCgroupSetBlkioDeviceWeight(cgroup, tmp);
+VIR_FREE(tmp);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set io device weight for domain 
%s),
+ vm-def-name);
+goto cleanup;
+}
+}
+}
+
  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 c8a858e..6b0acf0 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);

@@ -5888,6 +5888,82 @@ 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)
+{
+char *temp;
+int ndevices = 0;
+int i;
+virBlkioDeviceWeightPtr result = NULL;
+
+temp = deviceWeightStr;
+while (temp) {
+temp = strchr(temp, ';');
+if (temp) {
+temp++;
+if (*temp == '\0')
+break;
+ndevices++;
+}
+}
+ndevices++;
+
+if (VIR_ALLOC_N(result, ndevices)  0) {
+virReportOOMError();
+return -1;
+}
+
+i = 0;
+temp = deviceWeightStr;
+while (temp  i  ndevices) {
+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_i(temp,p, 10,result[i].weight)  0)
+goto error;
+
+i++;
+if (*p == '\0')
+break;
+else if (*p != ';')
+goto error;
+temp = p + 1;
+}
+if (i != ndevices)
+goto error;
+
+*dw = result;
+*size = i;
+
+return 0;
+

[libvirt] [PATCHv6 5/5] blkiotune: add qemu support for blkiotune.device_weight

2011-11-03 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.
---

v5: did not include this patch
v6: split v4 2/2 into two parts; this part covers just the qemu.
Rebase to accomodate 'device_weight' naming and VIR_TYPED_PARAM_STRING
handling.

This patch is broken, with several flaws:
1. 'virsh blkiotune dom --device-weight /dev/sda,500 --live' does not
alter the 'virsh dumpxml dom' live XML.  The cgroups change is
active, but unless we also reflect the change in the eventual xml dump,
then we aren't accurately telling the user the current state of the
domain.  This means that qemuDomainSetBlkioParameters needs a fix.

2. 'virsh blkiotune dom --live' now shows output that looks like:
weight  : 500
device_weight   : 8,0   500
whereas 'virsh blkiotune dom --config' shows:
weight  : 500
device_weight   : /dev/sda,500

The --config version is correct - it accurately reflects the XML.
The --live version is wrong - we MUST NOT expose major,minor
back to the user, since we intentionally decided that the XML
should track only the device name, not the major/minor implementation
detail of cgroups.  I think the correct fix would be to just delete
virCgroupGetBlkioDeviceWeight and treat cgroups blkio.weight_device
as write-only, and instead have qemuDomainGetBlkioParameters always
produce output from the domain definition (--live vs. --config merely
choosing between vm-def vs. persistentDef), which stems back to my
solution of point 1 in having in vm-def accurately reflect whatever
is written into cgroups.

 src/qemu/qemu_cgroup.c |   22 ++
 src/qemu/qemu_driver.c |  191 ++-
 src/util/cgroup.c  |   33 
 src/util/cgroup.h  |4 +
 4 files changed, 245 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 2a10bd2..d397578 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -312,6 +312,28 @@ int qemuSetupCgroup(struct qemud_driver *driver,
 }
 }

+if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
+char *tmp;
+if (virBlkioDeviceWeightToStr(vm-def-blkio.devices,
+  vm-def-blkio.ndevices,
+  tmp)  0) {
+qemuReportError(VIR_ERR_INTERNAL_ERROR,
+_(Unable to set io device weight for domain %s),
+vm-def-name);
+goto cleanup;
+}
+if (tmp) {
+rc = virCgroupSetBlkioDeviceWeight(cgroup, tmp);
+VIR_FREE(tmp);
+if (rc != 0) {
+virReportSystemError(-rc,
+ _(Unable to set io device weight for 
domain %s),
+ vm-def-name);
+goto cleanup;
+}
+}
+}
+
 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 c8a858e..6b0acf0 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);

@@ -5888,6 +5888,82 @@ 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)
+{
+char *temp;
+int ndevices = 0;
+int i;
+virBlkioDeviceWeightPtr result = NULL;
+
+temp = deviceWeightStr;
+while (temp) {
+temp = strchr(temp, ';');
+if (temp) {
+temp++;
+if (*temp == '\0')
+break;
+ndevices++;
+}
+}
+ndevices++;
+
+if (VIR_ALLOC_N(result, ndevices)  0) {
+virReportOOMError();
+return -1;
+}
+
+i = 0;
+temp = deviceWeightStr;
+while (temp  i  ndevices) {
+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_i(temp, p, 10, result[i].weight)  0)
+goto error;
+
+i++;
+if (*p == '\0')
+break;
+else if (*p != ';')
+goto error;
+temp = p + 1;
+}
+if (i != ndevices)
+goto error;
+
+*dw = result;
+*size = i;
+
+return 0;
+
+error:
+