[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 11/08/2011 06:00 AM, Hu Tao wrote:
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   |   22 +++++
  src/qemu/qemu_driver.c   |  216 ++++++++++++++++++++++++++++++++++++++++++++-
  src/util/cgroup.c        |   20 ++++
  src/util/cgroup.h        |    3 +
  5 files changed, 257 insertions(+), 5 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f7d0fb2..b32d5de 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..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);
The ToStr function doesn't report an OOM error. So this is ok. I wonder whether that function should report an OOM error, though? Eric?
+            goto cleanup;
+        }
+        if (tmp) {
I think you can remove the if branch.
+            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..40568d0 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,86 @@ cleanup:
      return ret;
  }

+/* deviceWeightStr 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
+parseBlkioWeightDeviceStr(char *deviceWeightStr,
+                          virBlkioDeviceWeightPtr *dw, int *size)
+{
+    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 */
...and 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&&  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_ui(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:
+    qemuReportError(VIR_ERR_INVALID_ARG,
+                    _("unable to parse %s"), deviceWeightStr);
+cleanup:
+    virBlkioDeviceWeightArrayClear(result, ndevices);
+    VIR_FREE(result);
+    return -1;
+}
+
  static int qemuDomainSetBlkioParameters(virDomainPtr dom,
                                           virTypedParameterPtr params,
                                           int nparams,
@@ -5954,10 +6034,10 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
      ret = 0;
      if (flags&  VIR_DOMAIN_AFFECT_LIVE) {
          for (i = 0; i<  nparams; i++) {
+            int rc;
              virTypedParameterPtr param =&params[i];

              if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) {
-                int rc;
                  if (param->type != VIR_TYPED_PARAM_UINT) {
                      qemuReportError(VIR_ERR_INVALID_ARG, "%s",
                                      _("invalid type for blkio weight tunable, expected a 'unsigned int'"));
@@ -5978,6 +6058,53 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
                                           _("unable to set blkio weight tunable"));
                      ret = -1;
                  }
+            } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
+                int ndevices;
+                virBlkioDeviceWeightPtr devices = NULL;
+                char *tmp;
+                if (param->type != VIR_TYPED_PARAM_STRING) {
+                    qemuReportError(VIR_ERR_INVALID_ARG, "%s",
+                                    _("invalid type for device_weight tunable, expected a 'char *'"));
+                    ret = -1;
+                    continue;
+                }
+
+                if (parseBlkioWeightDeviceStr(params[i].value.s,
+&devices,
+&ndevices)<  0) {
+                    ret = -1;
+                    continue;
+                }
+                if (virBlkioDeviceWeightToStr(devices,
+                                              ndevices,
+&tmp)<  0) {
+                    qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                                    _("Unable to set blkio weight_device tunable"));
+                    virBlkioDeviceWeightArrayClear(devices, ndevices);
+                    VIR_FREE(devices);
+                    ret = -1;
+                    continue;
+                }
+                if (tmp) {
+                    rc = virCgroupSetBlkioDeviceWeight(group, tmp);
+                    VIR_FREE(tmp);
+                    if (rc != 0) {
+                            virReportSystemError(-rc, "%s",
+                                                 _("unable to set blkio weight_device tunable"));
indentation
+                            ret = -1;
+                            virBlkioDeviceWeightArrayClear(devices, ndevices);
+                            VIR_FREE(devices);
+                            continue;
+                    }
+                    virBlkioDeviceWeightArrayClear(vm->def->blkio.devices,
+                                                   vm->def->blkio.ndevices);
+                    VIR_FREE(vm->def->blkio.devices);
+                    vm->def->blkio.devices = devices;
+                    vm->def->blkio.ndevices = ndevices;
+                } else {
+                    virBlkioDeviceWeightArrayClear(devices, ndevices);
+                    VIR_FREE(devices);
+                }
              } else {
                  qemuReportError(VIR_ERR_INVALID_ARG,
                                  _("Parameter `%s' not supported"), param->field);
@@ -6007,9 +6134,24 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
                  }

                  persistentDef->blkio.weight = params[i].value.ui;
+            } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
+                virBlkioDeviceWeightPtr devices = NULL;
+                int ndevices;
+                if (parseBlkioWeightDeviceStr(params[i].value.s,
+&devices,
+&ndevices)<  0) {
+                    ret = -1;
+                    continue;
+                }
+                virBlkioDeviceWeightArrayClear(persistentDef->blkio.devices,
+                                               persistentDef->blkio.ndevices);
+                VIR_FREE(persistentDef->blkio.devices);
+                persistentDef->blkio.devices = devices;
+                persistentDef->blkio.ndevices = ndevices;
              } else {
                  qemuReportError(VIR_ERR_INVALID_ARG,
-                                _("Parameter `%s' not supported"), param->field);
+                                _("Parameter `%s' not supported"),
+                                param->field);
                  ret = -1;
              }
          }
@@ -6032,7 +6174,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
                                           unsigned int flags)
  {
      struct qemud_driver *driver = dom->conn->privateData;
-    int i;
+    int i, j;
      virCgroupPtr group = NULL;
      virDomainObjPtr vm = NULL;
      virDomainDefPtr persistentDef = NULL;
@@ -6046,7 +6188,8 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
                    VIR_TYPED_PARAM_STRING_OKAY, -1);
      qemuDriverLock(driver);

-    /* We don't return strings, and thus trivially support this flag.  */
+    /* We blindly return a string, and let libvirt.c do the filtering
+     * on behalf of older clients that can't parse it.  */
      flags&= ~VIR_TYPED_PARAM_STRING_OKAY;

      vm = virDomainFindByUUID(&driver->domains, dom->uuid);
@@ -6125,6 +6268,37 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
                  }
                  param->value.ui = val;
                  break;
+            case 1: /* blkiotune.device_weight */
+                if (vm->def->blkio.ndevices>  0) {
+                    virBuffer buf = VIR_BUFFER_INITIALIZER;
+                    for (j = 0; j<  vm->def->blkio.ndevices; j++) {
+                        if (j)
+                            virBufferAddChar(&buf, ',');
+                        virBufferAsprintf(&buf, "%s,%u",
+                                          vm->def->blkio.devices[j].path,
+                                          vm->def->blkio.devices[j].weight);
+                    }
+                    if (virBufferError(&buf)) {
virBufferFreeAndReset(&buf)
+                        virReportOOMError();
+                        goto cleanup;
+                    }
+                    param->value.s = virBufferContentAndReset(&buf);
+                } else {
+                    param->value.s = strdup("");
+                    if (!param->value.s) {
+                        virReportOOMError();
+                        goto cleanup;
+                    }
+                }
+                param->type = VIR_TYPED_PARAM_STRING;
+                if (virStrcpyStatic(param->field,
+                                    VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) == NULL) {
+                    qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                                    _("Field name '%s' too long"),
.. is too long
+                                    VIR_DOMAIN_BLKIO_DEVICE_WEIGHT);
+                    goto cleanup;
+                }
+                break;

              default:
                  break;
@@ -6149,6 +6323,38 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
                  param->value.ui = persistentDef->blkio.weight;
                  break;

+            case 1: /* blkiotune.device_weight */
+                if (persistentDef->blkio.ndevices>  0) {
+                    virBuffer buf = VIR_BUFFER_INITIALIZER;
+                    for (j = 0; j<  persistentDef->blkio.ndevices; j++) {
+                        if (j)
+                            virBufferAddChar(&buf, ',');
+                        virBufferAsprintf(&buf, "%s,%u",
+                                          persistentDef->blkio.devices[j].path,
+                                          persistentDef->blkio.devices[j].weight);
+                    }
+                    if (virBufferError(&buf)) {
virBufferFreeAndReset(&buf);
+                        virReportOOMError();
+                        goto cleanup;
+                    }
+                    param->value.s = virBufferContentAndReset(&buf);
+                } else {
+                    param->value.s = strdup("");
+                    if (!param->value.s) {
+                        virReportOOMError();
+                        goto cleanup;
+                    }
+                }
+                param->type = VIR_TYPED_PARAM_STRING;
+                if (virStrcpyStatic(param->field,
+                                    VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) == NULL) {
+                    qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                                    _("Field name '%s' too long"),
+                                    VIR_DOMAIN_BLKIO_DEVICE_WEIGHT);
+                    goto cleanup;
+                }
+                break;
+
              default:
                  break;
                  /* should not hit here */
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index c8d1f33..b510640 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -982,6 +982,26 @@ int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight)
  }

  /**
+ * virCgroupSetBlkioDeviceWeight:
+ *
+ * @group: The cgroup to change io device weight device for
+ * @device_weight: The device weight for this cgroup
+ *
+ * device_weight is treated as a write-only parameter, so
+ * there isn't a getter counterpart.
+ *
+ * Returns: 0 on success
+ */
+int virCgroupSetBlkioDeviceWeight(virCgroupPtr group,
+                                  const char *device_weight)
+{
+    return virCgroupSetValueStr(group,
+                                VIR_CGROUP_CONTROLLER_BLKIO,
+                                "blkio.weight_device",
+                                device_weight);
+}
+
+/**
   * virCgroupSetMemory:
   *
   * @group: The cgroup to change memory for
diff --git a/src/util/cgroup.h b/src/util/cgroup.h
index d190bb3..9972671 100644
--- a/src/util/cgroup.h
+++ b/src/util/cgroup.h
@@ -55,6 +55,9 @@ int virCgroupAddTask(virCgroupPtr group, pid_t pid);
  int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight);
  int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight);

+int virCgroupSetBlkioDeviceWeight(virCgroupPtr group,
+                                  const char *device_weight);
+
  int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb);
  int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb);

ACK with those nits fixed.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]