[PATCH v2 09/14] src/hypervisor: introduce domain_driver.c

Daniel Henrique Barboza posted 14 patches 4 years, 9 months ago
[PATCH v2 09/14] src/hypervisor: introduce domain_driver.c
Posted by Daniel Henrique Barboza 4 years, 9 months ago
lxcDomainMergeBlkioDevice() and qemuDomainMergeBlkioDevice()
are the same functions. This duplicated code can't be put in
the existing domain_cgroup.c since it's not cgroup related.

This patch introduces a new src/hypervisor/domain_driver.c to
host this more generic code that can be shared between virt
drivers. This new file is then used to create a new helper
called virDomainDeivceMergeBlkioDevice() to eliminate the code
repetition mentioned above. Callers in LXC and QEMU files
were updated.

This change is a preliminary step for more code reduction of
cgroup related code inside lxcDomainSetBlkioParameters() and
qemuDomainSetBlkioParameters().

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/hypervisor/Makefile.inc.am |  2 +
 src/hypervisor/domain_driver.c | 96 ++++++++++++++++++++++++++++++++++
 src/hypervisor/domain_driver.h | 29 ++++++++++
 src/libvirt_private.syms       |  4 ++
 src/lxc/lxc_driver.c           | 84 ++++-------------------------
 src/qemu/qemu_driver.c         | 83 ++++-------------------------
 6 files changed, 149 insertions(+), 149 deletions(-)
 create mode 100644 src/hypervisor/domain_driver.c
 create mode 100644 src/hypervisor/domain_driver.h

diff --git a/src/hypervisor/Makefile.inc.am b/src/hypervisor/Makefile.inc.am
index 961b4e2b95..02cf2c7cb1 100644
--- a/src/hypervisor/Makefile.inc.am
+++ b/src/hypervisor/Makefile.inc.am
@@ -3,6 +3,8 @@
 HYPERVISOR_SOURCES = \
 	hypervisor/domain_cgroup.h \
 	hypervisor/domain_cgroup.c \
+	hypervisor/domain_driver.h \
+	hypervisor/domain_driver.c \
 	$(NULL)
 
 noinst_LTLIBRARIES += libvirt_hypervisor.la
diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
new file mode 100644
index 0000000000..c999458c25
--- /dev/null
+++ b/src/hypervisor/domain_driver.c
@@ -0,0 +1,96 @@
+/*
+ * domain_driver.c: general functions shared between hypervisor drivers
+ *
+ * Copyright IBM Corp. 2020
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+
+#include "domain_driver.h"
+#include "viralloc.h"
+
+#define VIR_FROM_THIS VIR_FROM_DOMAIN
+
+
+/* Modify dest_array to reflect all blkio device changes described in
+ * src_array.  */
+int
+virDomainDriverMergeBlkioDevice(virBlkioDevicePtr *dest_array,
+                                size_t *dest_size,
+                                virBlkioDevicePtr src_array,
+                                size_t src_size,
+                                const char *type)
+{
+    size_t i, j;
+    virBlkioDevicePtr dest, src;
+
+    for (i = 0; i < src_size; i++) {
+        bool found = false;
+
+        src = &src_array[i];
+        for (j = 0; j < *dest_size; j++) {
+            dest = &(*dest_array)[j];
+            if (STREQ(src->path, dest->path)) {
+                found = true;
+
+                if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
+                    dest->weight = src->weight;
+                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
+                    dest->riops = src->riops;
+                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
+                    dest->wiops = src->wiops;
+                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
+                    dest->rbps = src->rbps;
+                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
+                    dest->wbps = src->wbps;
+                } else {
+                    virReportError(VIR_ERR_INVALID_ARG, _("Unknown parameter %s"),
+                                   type);
+                    return -1;
+                }
+                break;
+            }
+        }
+        if (!found) {
+            if (!src->weight && !src->riops && !src->wiops && !src->rbps && !src->wbps)
+                continue;
+            if (VIR_EXPAND_N(*dest_array, *dest_size, 1) < 0)
+                return -1;
+            dest = &(*dest_array)[*dest_size - 1];
+
+            if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
+                dest->weight = src->weight;
+            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
+                dest->riops = src->riops;
+            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
+                dest->wiops = src->wiops;
+            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
+                dest->rbps = src->rbps;
+            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
+                dest->wbps = src->wbps;
+            } else {
+                *dest_size = *dest_size - 1;
+                return -1;
+            }
+
+            dest->path = src->path;
+            src->path = NULL;
+        }
+    }
+
+    return 0;
+}
diff --git a/src/hypervisor/domain_driver.h b/src/hypervisor/domain_driver.h
new file mode 100644
index 0000000000..2fb4148dff
--- /dev/null
+++ b/src/hypervisor/domain_driver.h
@@ -0,0 +1,29 @@
+/*
+ * domain_driver.h: general functions shared between hypervisor drivers
+ *
+ * Copyright IBM Corp. 2020
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "domain_conf.h"
+
+int virDomainDriverMergeBlkioDevice(virBlkioDevicePtr *dest_array,
+                                    size_t *dest_size,
+                                    virBlkioDevicePtr src_array,
+                                    size_t src_size,
+                                    const char *type);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2f958b1c6f..c6b9afe259 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1394,6 +1394,10 @@ virDomainCgroupSetupBlkio;
 virDomainCgroupSetupMemtune;
 
 
+# hypervisor/domain_cgroup.h
+virDomainDriverMergeBlkioDevice;
+
+
 # libvirt_internal.h
 virConnectSupportsFeature;
 virDomainMigrateBegin3;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index d6d0f031a5..08c744a69b 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_driver.h"
 #include "domain_nwfilter.h"
 #include "virinitctl.h"
 #include "virnetdev.h"
@@ -2214,75 +2215,6 @@ lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type,
     return -1;
 }
 
-static int
-lxcDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array,
-                          size_t *dest_size,
-                          virBlkioDevicePtr src_array,
-                          size_t src_size,
-                          const char *type)
-{
-    size_t i, j;
-    virBlkioDevicePtr dest, src;
-
-    for (i = 0; i < src_size; i++) {
-        bool found = false;
-
-        src = &src_array[i];
-        for (j = 0; j < *dest_size; j++) {
-            dest = &(*dest_array)[j];
-            if (STREQ(src->path, dest->path)) {
-                found = true;
-
-                if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
-                    dest->weight = src->weight;
-                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
-                    dest->riops = src->riops;
-                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
-                    dest->wiops = src->wiops;
-                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
-                    dest->rbps = src->rbps;
-                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
-                    dest->wbps = src->wbps;
-                } else {
-                    virReportError(VIR_ERR_INVALID_ARG, _("Unknown parameter %s"),
-                                   type);
-                    return -1;
-                }
-
-                break;
-            }
-        }
-        if (!found) {
-            if (!src->weight && !src->riops && !src->wiops && !src->rbps && !src->wbps)
-                continue;
-            if (VIR_EXPAND_N(*dest_array, *dest_size, 1) < 0)
-                return -1;
-            dest = &(*dest_array)[*dest_size - 1];
-
-            if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
-                dest->weight = src->weight;
-            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
-                dest->riops = src->riops;
-            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
-                dest->wiops = src->wiops;
-            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
-                dest->rbps = src->rbps;
-            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
-                dest->wbps = src->wbps;
-            } else {
-                *dest_size = *dest_size - 1;
-                return -1;
-            }
-
-            dest->path = src->path;
-            src->path = NULL;
-        }
-    }
-
-    return 0;
-}
-
-
 static int
 lxcDomainBlockStats(virDomainPtr dom,
                     const char *path,
@@ -2613,9 +2545,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
                 }
 
                 if (j != ndevices ||
-                    lxcDomainMergeBlkioDevice(&def->blkio.devices,
-                                              &def->blkio.ndevices,
-                                              devices, ndevices, param->field) < 0)
+                    virDomainDriverMergeBlkioDevice(&def->blkio.devices,
+                                                    &def->blkio.ndevices,
+                                                    devices, ndevices,
+                                                    param->field) < 0)
                     ret = -1;
                 virBlkioDeviceArrayClear(devices, ndevices);
                 VIR_FREE(devices);
@@ -2645,9 +2578,10 @@ lxcDomainSetBlkioParameters(virDomainPtr dom,
                     ret = -1;
                     continue;
                 }
-                if (lxcDomainMergeBlkioDevice(&persistentDef->blkio.devices,
-                                              &persistentDef->blkio.ndevices,
-                                              devices, ndevices, param->field) < 0)
+                if (virDomainDriverMergeBlkioDevice(&persistentDef->blkio.devices,
+                                                    &persistentDef->blkio.ndevices,
+                                                    devices, ndevices,
+                                                    param->field) < 0)
                     ret = -1;
                 virBlkioDeviceArrayClear(devices, ndevices);
                 VIR_FREE(devices);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e9feb50220..3048e8a5c7 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_driver.h"
 #include "node_device_conf.h"
 #include "virpci.h"
 #include "virusb.h"
@@ -9419,74 +9420,6 @@ qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type,
     return -1;
 }
 
-/* Modify dest_array to reflect all blkio device changes described in
- * src_array.  */
-static int
-qemuDomainMergeBlkioDevice(virBlkioDevicePtr *dest_array,
-                           size_t *dest_size,
-                           virBlkioDevicePtr src_array,
-                           size_t src_size,
-                           const char *type)
-{
-    size_t i, j;
-    virBlkioDevicePtr dest, src;
-
-    for (i = 0; i < src_size; i++) {
-        bool found = false;
-
-        src = &src_array[i];
-        for (j = 0; j < *dest_size; j++) {
-            dest = &(*dest_array)[j];
-            if (STREQ(src->path, dest->path)) {
-                found = true;
-
-                if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
-                    dest->weight = src->weight;
-                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
-                    dest->riops = src->riops;
-                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
-                    dest->wiops = src->wiops;
-                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
-                    dest->rbps = src->rbps;
-                } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
-                    dest->wbps = src->wbps;
-                } else {
-                    virReportError(VIR_ERR_INVALID_ARG, _("Unknown parameter %s"),
-                                   type);
-                    return -1;
-                }
-                break;
-            }
-        }
-        if (!found) {
-            if (!src->weight && !src->riops && !src->wiops && !src->rbps && !src->wbps)
-                continue;
-            if (VIR_EXPAND_N(*dest_array, *dest_size, 1) < 0)
-                return -1;
-            dest = &(*dest_array)[*dest_size - 1];
-
-            if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
-                dest->weight = src->weight;
-            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
-                dest->riops = src->riops;
-            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
-                dest->wiops = src->wiops;
-            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
-                dest->rbps = src->rbps;
-            } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
-                dest->wbps = src->wbps;
-            } else {
-                *dest_size = *dest_size - 1;
-                return -1;
-            }
-
-            dest->path = src->path;
-            src->path = NULL;
-        }
-    }
-
-    return 0;
-}
 
 static int
 qemuDomainSetBlkioParameters(virDomainPtr dom,
@@ -9628,9 +9561,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom,
                 }
 
                 if (j != ndevices ||
-                    qemuDomainMergeBlkioDevice(&def->blkio.devices,
-                                               &def->blkio.ndevices,
-                                               devices, ndevices, param->field) < 0)
+                    virDomainDriverMergeBlkioDevice(&def->blkio.devices,
+                                                    &def->blkio.ndevices,
+                                                    devices, ndevices,
+                                                    param->field) < 0)
                     ret = -1;
                 virBlkioDeviceArrayClear(devices, ndevices);
                 VIR_FREE(devices);
@@ -9663,9 +9597,10 @@ qemuDomainSetBlkioParameters(virDomainPtr dom,
                     ret = -1;
                     continue;
                 }
-                if (qemuDomainMergeBlkioDevice(&persistentDef->blkio.devices,
-                                               &persistentDef->blkio.ndevices,
-                                               devices, ndevices, param->field) < 0)
+                if (virDomainDriverMergeBlkioDevice(&persistentDef->blkio.devices,
+                                                    &persistentDef->blkio.ndevices,
+                                                    devices, ndevices,
+                                                    param->field) < 0)
                     ret = -1;
                 virBlkioDeviceArrayClear(devices, ndevices);
                 VIR_FREE(devices);
-- 
2.24.1


Re: [PATCH v2 09/14] src/hypervisor: introduce domain_driver.c
Posted by Ján Tomko 4 years, 9 months ago
On Mon, Feb 17, 2020 at 04:29:16PM -0500, Daniel Henrique Barboza wrote:
>lxcDomainMergeBlkioDevice() and qemuDomainMergeBlkioDevice()
>are the same functions. This duplicated code can't be put in
>the existing domain_cgroup.c since it's not cgroup related.
>
>This patch introduces a new src/hypervisor/domain_driver.c to
>host this more generic code that can be shared between virt
>drivers. This new file is then used to create a new helper
>called virDomainDeivceMergeBlkioDevice() to eliminate the code
>repetition mentioned above. Callers in LXC and QEMU files
>were updated.
>
>This change is a preliminary step for more code reduction of
>cgroup related code inside lxcDomainSetBlkioParameters() and
>qemuDomainSetBlkioParameters().
>
>Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>---
> src/hypervisor/Makefile.inc.am |  2 +
> src/hypervisor/domain_driver.c | 96 ++++++++++++++++++++++++++++++++++
> src/hypervisor/domain_driver.h | 29 ++++++++++
> src/libvirt_private.syms       |  4 ++
> src/lxc/lxc_driver.c           | 84 ++++-------------------------
> src/qemu/qemu_driver.c         | 83 ++++-------------------------
> 6 files changed, 149 insertions(+), 149 deletions(-)
> create mode 100644 src/hypervisor/domain_driver.c
> create mode 100644 src/hypervisor/domain_driver.h
>

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

Jano