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

Re: [libvirt] [PATCH v2 4/9] qemu: Add bps_max and friends qemu driver



On 15.09.2014 19:27, Matthias Gatto wrote:
Add support for bps_max and friends in the driver part.
In the part checking if a qemu is running, check if the running binary support bps_max,
if not print an error message, if yes add it to "info" variable

Signed-off-by: Matthias Gatto <matthias gatto outscale com>
---
  src/qemu/qemu_driver.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 152 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2c3f179..7d024b3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -104,6 +104,7 @@ VIR_LOG_INIT("qemu.qemu_driver");
  #define QEMU_NB_MEM_PARAM  3

  #define QEMU_NB_BLOCK_IO_TUNE_PARAM  6
+#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX  13

Why? I think we can just proceed with current _PARAM changed so it reflects the number of arguments that can be returned.

Aha! going through the later patches I think I understand why you want this - in case qemu supports QEMU_CAPS_DRIVE_IOTUNE_MAX then we should report 13, if it doesn't then 6. Well, the code doesn't say that [2].


  #define QEMU_NB_NUMA_PARAM 2

@@ -15818,8 +15819,12 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
      int idx = -1;
      bool set_bytes = false;
      bool set_iops = false;
+    bool set_bytes_max = false;
+    bool set_iops_max = false;
+    bool set_size_iops = false;
      virQEMUDriverConfigPtr cfg = NULL;
      virCapsPtr caps = NULL;
+    info.suport_max_options = true;

This is useless ... [1]


      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
                    VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -15836,6 +15841,20 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
                                 VIR_TYPED_PARAM_ULLONG,
                                 VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
                                 VIR_TYPED_PARAM_ULLONG,
+                               VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX,
+                               VIR_TYPED_PARAM_ULLONG,
+                               VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX,
+                               VIR_TYPED_PARAM_ULLONG,
+                               VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX,
+                               VIR_TYPED_PARAM_ULLONG,
+                               VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX,
+                               VIR_TYPED_PARAM_ULLONG,
+                               VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX,
+                               VIR_TYPED_PARAM_ULLONG,
+                               VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX,
+                               VIR_TYPED_PARAM_ULLONG,
+                               VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
+                               VIR_TYPED_PARAM_ULLONG,
                                 NULL) < 0)
          return -1;

@@ -15902,6 +15921,34 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
                           VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) {
              info.write_iops_sec = param->value.ul;
              set_iops = true;
+        } else if (STREQ(param->field,
+                         VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX)) {
+            info.total_bytes_sec_max = param->value.ul;
+            set_bytes_max = true;
+        } else if (STREQ(param->field,
+                         VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX)) {
+            info.read_bytes_sec_max = param->value.ul;
+            set_bytes_max = true;
+        } else if (STREQ(param->field,
+                         VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX)) {
+            info.write_bytes_sec_max = param->value.ul;
+            set_bytes_max = true;
+        } else if (STREQ(param->field,
+                         VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX)) {
+            info.total_iops_sec_max = param->value.ul;
+            set_iops_max = true;
+        } else if (STREQ(param->field,
+                         VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX)) {
+            info.read_iops_sec_max = param->value.ul;
+            set_iops_max = true;
+        } else if (STREQ(param->field,
+                         VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX)) {
+            info.write_iops_sec_max = param->value.ul;
+            set_iops_max = true;
+        } else if (STREQ(param->field,
+                         VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC)) {
+            info.size_iops_sec = param->value.ul;
+            set_size_iops = true;
          }
      }

@@ -15919,11 +15966,33 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
          goto endjob;
      }

+    if ((info.total_bytes_sec_max && info.read_bytes_sec_max) ||
+        (info.total_bytes_sec_max && info.write_bytes_sec_max)) {
+        virReportError(VIR_ERR_INVALID_ARG, "%s",
+                       _("total and read/write of bytes_sec_max cannot be set at the same time"));
+        goto endjob;
+    }
+
+    if ((info.total_iops_sec_max && info.read_iops_sec_max) ||
+        (info.total_iops_sec_max && info.write_iops_sec_max)) {
+        virReportError(VIR_ERR_INVALID_ARG, "%s",
+                       _("total and read/write of iops_sec_max cannot be set at the same time"));
+        goto endjob;
+    }
+
      if (flags & VIR_DOMAIN_AFFECT_LIVE) {
          if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) {
              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("block I/O throttling not supported with this "
-                         "QEMU binary"));
+                           _("block I/O throttling not supported with this "
+                             "QEMU binary"));
+            goto endjob;
+        }
+
+        info.suport_max_options = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX);

1: esp. if this occurs

+        if ((!info.suport_max_options) && (set_iops_max || set_bytes_max)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("a block I/O throttling parameter is not supported with this "
+                             "QEMU binary"));
              goto endjob;
          }

@@ -15936,11 +16005,23 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
              info.read_bytes_sec = oldinfo->read_bytes_sec;
              info.write_bytes_sec = oldinfo->write_bytes_sec;
          }
+        if (!set_bytes_max) {
+            info.total_bytes_sec_max = oldinfo->total_bytes_sec_max;
+            info.read_bytes_sec_max = oldinfo->read_bytes_sec_max;
+            info.write_bytes_sec_max = oldinfo->write_bytes_sec_max;
+        }
          if (!set_iops) {
              info.total_iops_sec = oldinfo->total_iops_sec;
              info.read_iops_sec = oldinfo->read_iops_sec;
              info.write_iops_sec = oldinfo->write_iops_sec;
          }
+        if (!set_iops_max) {
+            info.total_iops_sec_max = oldinfo->total_iops_sec_max;
+            info.read_iops_sec_max = oldinfo->read_iops_sec_max;
+            info.write_iops_sec_max = oldinfo->write_iops_sec_max;
+        }
+        if (!set_size_iops)
+            info.size_iops_sec = oldinfo->size_iops_sec;
          qemuDomainObjEnterMonitor(driver, vm);
          ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info);
          qemuDomainObjExitMonitor(driver, vm);
@@ -15957,11 +16038,23 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
              info.read_bytes_sec = oldinfo->read_bytes_sec;
              info.write_bytes_sec = oldinfo->write_bytes_sec;
          }
+        if (!set_bytes_max) {
+            info.total_bytes_sec_max = oldinfo->total_bytes_sec_max;
+            info.read_bytes_sec_max = oldinfo->read_bytes_sec_max;
+            info.write_bytes_sec_max = oldinfo->write_bytes_sec_max;
+        }
          if (!set_iops) {
              info.total_iops_sec = oldinfo->total_iops_sec;
              info.read_iops_sec = oldinfo->read_iops_sec;
              info.write_iops_sec = oldinfo->write_iops_sec;
          }
+        if (!set_iops_max) {
+            info.total_iops_sec_max = oldinfo->total_iops_sec_max;
+            info.read_iops_sec_max = oldinfo->read_iops_sec_max;
+            info.write_iops_sec_max = oldinfo->write_iops_sec_max;
+        }
+        if (!set_size_iops)
+            info.size_iops_sec = oldinfo->size_iops_sec;
          persistentDef->disks[idx]->blkdeviotune = info;
          ret = virDomainSaveConfig(cfg->configDir, persistentDef);
          if (ret < 0) {
@@ -15972,6 +16065,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
      }

   endjob:
+
      if (!qemuDomainObjEndJob(driver, vm))
          vm = NULL;

@@ -16001,6 +16095,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
      size_t i;
      virCapsPtr caps = NULL;
      virQEMUDriverConfigPtr cfg = NULL;
+    reply.suport_max_options = true;

      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
                    VIR_DOMAIN_AFFECT_CONFIG |
@@ -16027,7 +16122,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,

      if ((*nparams) == 0) {
          /* Current number of parameters supported by QEMU Block I/O Throttling */
-        *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM;
+        *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX;

2: ^^^

          ret = 0;
          goto cleanup;
      }
@@ -16046,6 +16141,9 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,

      if (flags & VIR_DOMAIN_AFFECT_LIVE) {
          priv = vm->privateData;
+        reply.suport_max_options = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX);
+        if (!reply.suport_max_options)
+            *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM;
          qemuDomainObjEnterMonitor(driver, vm);
          ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply);

Instead of passing support_max_options like you're, I'd change the qemuMonitorGetBlockIoThrottle() to accept one more boole attribute to fetch minimal or extended set of values from qemu.

          qemuDomainObjExitMonitor(driver, vm);
@@ -16060,7 +16158,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
          reply = persistentDef->disks[idx]->blkdeviotune;
      }

-    for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM && i < *nparams; i++) {
+    for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX && i < *nparams; i++) {
          virTypedParameterPtr param = &params[i];

          switch (i) {
@@ -16106,13 +16204,61 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
                                          reply.write_iops_sec) < 0)
                  goto endjob;
              break;
+        case 6:
+            if (virTypedParameterAssign(param,
+                                        VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX,
+                                        VIR_TYPED_PARAM_ULLONG,
+                                        reply.total_bytes_sec_max) < 0)
+                goto endjob;
+            break;
+        case 7:
+            if (virTypedParameterAssign(param,
+                                        VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX,
+                                        VIR_TYPED_PARAM_ULLONG,
+                                        reply.read_bytes_sec_max) < 0)
+                goto endjob;
+            break;
+        case 8:
+            if (virTypedParameterAssign(param,
+                                        VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX,
+                                        VIR_TYPED_PARAM_ULLONG,
+                                        reply.write_bytes_sec_max) < 0)
+                goto endjob;
+            break;
+        case 9:
+            if (virTypedParameterAssign(param,
+                                        VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX,
+                                        VIR_TYPED_PARAM_ULLONG,
+                                        reply.total_iops_sec_max) < 0)
+                goto endjob;
+            break;
+        case 10:
+            if (virTypedParameterAssign(param,
+                                        VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX,
+                                        VIR_TYPED_PARAM_ULLONG,
+                                        reply.read_iops_sec_max) < 0)
+                goto endjob;
+            break;
+        case 11:
+            if (virTypedParameterAssign(param,
+                                        VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX,
+                                        VIR_TYPED_PARAM_ULLONG,
+                                        reply.write_iops_sec_max) < 0)
+                goto endjob;
+            break;
+        case 12:
+            if (virTypedParameterAssign(param,
+                                        VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
+                                        VIR_TYPED_PARAM_ULLONG,
+                                        reply.size_iops_sec) < 0)
+                goto endjob;
          default:
              break;
          }
      }

-    if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM)
-        *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM;
+    if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX)
+        *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX;
      ret = 0;

   endjob:


Michal


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