[PATCH v2 11/14] domain_cgroup.c: add virDomainCgroupSetupDomainBlkioParameters()

Daniel Henrique Barboza posted 14 patches 4 years, 9 months ago
[PATCH v2 11/14] domain_cgroup.c: add virDomainCgroupSetupDomainBlkioParameters()
Posted by Daniel Henrique Barboza 4 years, 9 months ago
After the introduction of virDomainDriverMergeBlkioDevice() in a
previous patch, it is now clear that lxcDomainSetBlkioParameters() and
qemuDomainSetBlkioParameters() uses the same loop to set cgroup
blkio parameter of a domain.

Avoid the repetition by adding a new helper called
virDomainCgroupSetupDomainBlkioParameters().

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 po/POTFILES.in                 |   1 +
 src/hypervisor/domain_cgroup.c | 101 +++++++++++++++++++++++++++++++++
 src/hypervisor/domain_cgroup.h |   4 ++
 src/libvirt_private.syms       |   1 +
 src/lxc/lxc_driver.c           |  87 +---------------------------
 src/qemu/qemu_driver.c         |  88 +---------------------------
 6 files changed, 113 insertions(+), 169 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index aa5c1fb6c7..1675c45206 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -78,6 +78,7 @@
 @SRCDIR@/src/hyperv/hyperv_driver.c
 @SRCDIR@/src/hyperv/hyperv_util.c
 @SRCDIR@/src/hyperv/hyperv_wmi.c
+@SRCDIR@/src/hypervisor/domain_cgroup.c
 @SRCDIR@/src/hypervisor/domain_driver.c
 @SRCDIR@/src/interface/interface_backend_netcf.c
 @SRCDIR@/src/interface/interface_backend_udev.c
diff --git a/src/hypervisor/domain_cgroup.c b/src/hypervisor/domain_cgroup.c
index e60abd536c..32b07be3d6 100644
--- a/src/hypervisor/domain_cgroup.c
+++ b/src/hypervisor/domain_cgroup.c
@@ -21,6 +21,9 @@
 #include <config.h>
 
 #include "domain_cgroup.h"
+#include "domain_driver.h"
+
+#define VIR_FROM_THIS VIR_FROM_DOMAIN
 
 
 int
@@ -84,3 +87,101 @@ virDomainCgroupSetupMemtune(virCgroupPtr cgroup, virDomainMemtune mem)
 
     return 0;
 }
+
+
+int
+virDomainCgroupSetupDomainBlkioParameters(virCgroupPtr cgroup,
+                                          virDomainDefPtr def,
+                                          virTypedParameterPtr params,
+                                          int nparams)
+{
+    size_t i;
+    int ret = 0;
+
+    for (i = 0; i < nparams; i++) {
+        virTypedParameterPtr param = &params[i];
+
+        if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) {
+            if (virCgroupSetBlkioWeight(cgroup, params[i].value.ui) < 0)
+                ret = -1;
+        } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) ||
+                   STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) ||
+                   STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) ||
+                   STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) ||
+                   STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
+            size_t ndevices;
+            virBlkioDevicePtr devices = NULL;
+            size_t j;
+
+            if (virDomainDriverParseBlkioDeviceStr(params[i].value.s,
+                                                   param->field,
+                                                   &devices,
+                                                   &ndevices) < 0) {
+                ret = -1;
+                continue;
+            }
+
+            if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
+                for (j = 0; j < ndevices; j++) {
+                    if (virCgroupSetupBlkioDeviceWeight(cgroup, devices[j].path,
+                                                        &devices[j].weight) < 0) {
+                        ret = -1;
+                        break;
+                    }
+                }
+            } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
+                for (j = 0; j < ndevices; j++) {
+                    if (virCgroupSetupBlkioDeviceReadIops(cgroup, devices[j].path,
+                                                          &devices[j].riops) < 0) {
+                        ret = -1;
+                        break;
+                    }
+                }
+            } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
+                for (j = 0; j < ndevices; j++) {
+                    if (virCgroupSetupBlkioDeviceWriteIops(cgroup, devices[j].path,
+                                                           &devices[j].wiops) < 0) {
+                        ret = -1;
+                        break;
+                    }
+                }
+            } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
+                for (j = 0; j < ndevices; j++) {
+                    if (virCgroupSetupBlkioDeviceReadBps(cgroup, devices[j].path,
+                                                         &devices[j].rbps) < 0) {
+                        ret = -1;
+                        break;
+                    }
+                }
+            } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
+                for (j = 0; j < ndevices; j++) {
+                    if (virCgroupSetupBlkioDeviceWriteBps(cgroup, devices[j].path,
+                                                          &devices[j].wbps) < 0) {
+                        ret = -1;
+                        break;
+                    }
+                }
+            } else {
+                virReportError(VIR_ERR_INVALID_ARG, _("Unknown blkio parameter %s"),
+                               param->field);
+                ret = -1;
+                virBlkioDeviceArrayClear(devices, ndevices);
+                g_free(devices);
+
+                continue;
+            }
+
+            if (j != ndevices ||
+                virDomainDriverMergeBlkioDevice(&def->blkio.devices,
+                                                &def->blkio.ndevices,
+                                                devices, ndevices,
+                                                param->field) < 0)
+                ret = -1;
+
+            virBlkioDeviceArrayClear(devices, ndevices);
+            g_free(devices);
+        }
+    }
+
+    return ret;
+}
diff --git a/src/hypervisor/domain_cgroup.h b/src/hypervisor/domain_cgroup.h
index 82ba47e85f..7f3a0d12f9 100644
--- a/src/hypervisor/domain_cgroup.h
+++ b/src/hypervisor/domain_cgroup.h
@@ -26,3 +26,7 @@
 
 int virDomainCgroupSetupBlkio(virCgroupPtr cgroup, virDomainBlkiotune blkio);
 int virDomainCgroupSetupMemtune(virCgroupPtr cgroup, virDomainMemtune mem);
+int virDomainCgroupSetupDomainBlkioParameters(virCgroupPtr cgroup,
+                                              virDomainDefPtr def,
+                                              virTypedParameterPtr params,
+                                              int nparams);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 88f9ca16d1..a01a0223f3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1391,6 +1391,7 @@ virSetConnectStorage;
 
 # hypervisor/domain_cgroup.h
 virDomainCgroupSetupBlkio;
+virDomainCgroupSetupDomainBlkioParameters;
 virDomainCgroupSetupMemtune;
 
 
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 78a0174277..c93dee37f8 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -56,6 +56,7 @@
 #include "virpidfile.h"
 #include "virfdstream.h"
 #include "domain_audit.h"
+#include "domain_cgroup.h"
 #include "domain_driver.h"
 #include "domain_nwfilter.h"
 #include "virinitctl.h"
@@ -2364,90 +2365,8 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
 
     ret = 0;
     if (def) {
-        for (i = 0; i < nparams; i++) {
-            virTypedParameterPtr param = &params[i];
-
-            if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) {
-                if (virCgroupSetBlkioWeight(priv->cgroup, params[i].value.ui) < 0)
-                    ret = -1;
-            } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) ||
-                       STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) ||
-                       STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) ||
-                       STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) ||
-                       STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
-                virCgroupPtr cgroup = priv->cgroup;
-                size_t ndevices;
-                virBlkioDevicePtr devices = NULL;
-                size_t j;
-
-                if (virDomainDriverParseBlkioDeviceStr(params[i].value.s,
-                                                       param->field,
-                                                       &devices,
-                                                       &ndevices) < 0) {
-                    ret = -1;
-                    continue;
-                }
-
-                if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceWeight(cgroup, devices[j].path,
-                                                            &devices[j].weight) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceReadIops(cgroup, devices[j].path,
-                                                              &devices[j].riops) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceWriteIops(cgroup, devices[j].path,
-                                                               &devices[j].wiops) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceReadBps(cgroup, devices[j].path,
-                                                             &devices[j].rbps) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceWriteBps(cgroup, devices[j].path,
-                                                              &devices[j].wbps) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else {
-                    virReportError(VIR_ERR_INVALID_ARG, _("Unknown blkio parameter %s"),
-                                   param->field);
-                    ret = -1;
-                    virBlkioDeviceArrayClear(devices, ndevices);
-                    VIR_FREE(devices);
-
-                    continue;
-                }
-
-                if (j != ndevices ||
-                    virDomainDriverMergeBlkioDevice(&def->blkio.devices,
-                                                    &def->blkio.ndevices,
-                                                    devices, ndevices,
-                                                    param->field) < 0)
-                    ret = -1;
-                virBlkioDeviceArrayClear(devices, ndevices);
-                VIR_FREE(devices);
-            }
-        }
+        ret = virDomainCgroupSetupDomainBlkioParameters(priv->cgroup, def,
+                                                        params, nparams);
     }
     if (ret < 0)
         goto endjob;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2205dd1194..850d6699ce 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -65,6 +65,7 @@
 #include "viruuid.h"
 #include "domain_conf.h"
 #include "domain_audit.h"
+#include "domain_cgroup.h"
 #include "domain_driver.h"
 #include "node_device_conf.h"
 #include "virpci.h"
@@ -9375,91 +9376,8 @@ qemuDomainSetBlkioParameters(virDomainPtr dom,
 
     ret = 0;
     if (def) {
-        for (i = 0; i < nparams; i++) {
-            virTypedParameterPtr param = &params[i];
-
-            if (STREQ(param->field, VIR_DOMAIN_BLKIO_WEIGHT)) {
-                if (virCgroupSetBlkioWeight(priv->cgroup, param->value.ui) < 0 ||
-                    virCgroupGetBlkioWeight(priv->cgroup, &def->blkio.weight) < 0)
-                    ret = -1;
-            } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) ||
-                       STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS) ||
-                       STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS) ||
-                       STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS) ||
-                       STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
-                virCgroupPtr cgroup = priv->cgroup;
-                size_t ndevices;
-                virBlkioDevicePtr devices = NULL;
-                size_t j;
-
-                if (virDomainDriverParseBlkioDeviceStr(param->value.s,
-                                                       param->field,
-                                                       &devices,
-                                                       &ndevices) < 0) {
-                    ret = -1;
-                    continue;
-                }
-
-                if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceWeight(cgroup, devices[j].path,
-                                                            &devices[j].weight) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceReadIops(cgroup, devices[j].path,
-                                                              &devices[j].riops) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceWriteIops(cgroup, devices[j].path,
-                                                               &devices[j].wiops) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceReadBps(cgroup, devices[j].path,
-                                                             &devices[j].rbps) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else if (STREQ(param->field, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
-                    for (j = 0; j < ndevices; j++) {
-                        if (virCgroupSetupBlkioDeviceWriteBps(cgroup, devices[j].path,
-                                                              &devices[j].wbps) < 0) {
-                            ret = -1;
-                            break;
-                        }
-                    }
-                } else {
-                    virReportError(VIR_ERR_INVALID_ARG, _("Unknown blkio parameter %s"),
-                                   param->field);
-                    ret = -1;
-                    virBlkioDeviceArrayClear(devices, ndevices);
-                    VIR_FREE(devices);
-
-                    continue;
-                }
-
-                if (j != ndevices ||
-                    virDomainDriverMergeBlkioDevice(&def->blkio.devices,
-                                                    &def->blkio.ndevices,
-                                                    devices, ndevices,
-                                                    param->field) < 0)
-                    ret = -1;
-                virBlkioDeviceArrayClear(devices, ndevices);
-                VIR_FREE(devices);
-            }
-        }
+        ret = virDomainCgroupSetupDomainBlkioParameters(priv->cgroup, def,
+                                                        params, nparams);
 
         if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
             goto endjob;
-- 
2.24.1


Re: [PATCH v2 11/14] domain_cgroup.c: add virDomainCgroupSetupDomainBlkioParameters()
Posted by Ján Tomko 4 years, 9 months ago
On Mon, Feb 17, 2020 at 04:29:18PM -0500, Daniel Henrique Barboza wrote:
>After the introduction of virDomainDriverMergeBlkioDevice() in a
>previous patch, it is now clear that lxcDomainSetBlkioParameters() and
>qemuDomainSetBlkioParameters() uses the same loop to set cgroup
>blkio parameter of a domain.
>
>Avoid the repetition by adding a new helper called
>virDomainCgroupSetupDomainBlkioParameters().
>
>Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>---
> po/POTFILES.in                 |   1 +
> src/hypervisor/domain_cgroup.c | 101 +++++++++++++++++++++++++++++++++
> src/hypervisor/domain_cgroup.h |   4 ++
> src/libvirt_private.syms       |   1 +
> src/lxc/lxc_driver.c           |  87 +---------------------------
> src/qemu/qemu_driver.c         |  88 +---------------------------
> 6 files changed, 113 insertions(+), 169 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano