Re: [libvirt] [PATCH 1/2] domain_conf: move DomainParseBlkioDeviceStr out of QEMU and LXC drivers
On Thu, Jul 11, 2019 at 09:50:06 +0200, Pavel Hrdina wrote: > On Thu, Jul 11, 2019 at 08:52:09AM +0200, Peter Krempa wrote: > > On Wed, Jul 10, 2019 at 17:44:31 +0200, Ilias Stamatis wrote: > > > The qemuDomainParseBlkioDeviceStr and lxcDomainParseBlkioDeviceSts > > > functions residing in the QEMU and LXC drivers respectively are > > > completely identical. > > > > > > By moving the code to src/conf we avoid code duplication and we make the > > > function available to other drivers that might need to call it such as > > > the test driver. > > > > > > Signed-off-by: Ilias Stamatis > > > --- > > > src/conf/domain_conf.c | 115 +++ > > > src/conf/domain_conf.h | 6 ++ > > > src/libvirt_private.syms | 1 + > > > src/lxc/lxc_driver.c | 112 +- > > > src/qemu/qemu_driver.c | 126 +++ > > > 5 files changed, 132 insertions(+), 228 deletions(-) > > > > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > > > index 3323c9a5b1..e10390189c 100644 > > > --- a/src/conf/domain_conf.c > > > +++ b/src/conf/domain_conf.c > > > @@ -30219,6 +30219,121 @@ virDomainDefGetShortName(const virDomainDef > > > *def) > > > > > > #undef VIR_DOMAIN_SHORT_NAME_MAX > > > > > > + > > > +/* blkioDeviceStr 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 > > > + */ > > > +int > > > +virDomainParseBlkioDeviceStr(char *blkioDeviceStr, > > > > The domain_conf.c module is meant to collect stuff regarding the XML > > parser and configuration-related methods. > > > > This is a worker method which does not have to do anything with config. > > > > Please find a proper place in src/util/ for it. > > In order to add it into src/util we need to remove dependency on > virBlkioDevicePtr from the code as in util we should not include > anything from other parts of libvirt. Hmpf right. Okay then. I retract my comment then. In the end domain_conf.c is already a dumping pile of random code anyways. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] domain_conf: move DomainParseBlkioDeviceStr out of QEMU and LXC drivers
On Thu, Jul 11, 2019 at 08:52:09AM +0200, Peter Krempa wrote: > On Wed, Jul 10, 2019 at 17:44:31 +0200, Ilias Stamatis wrote: > > The qemuDomainParseBlkioDeviceStr and lxcDomainParseBlkioDeviceSts > > functions residing in the QEMU and LXC drivers respectively are > > completely identical. > > > > By moving the code to src/conf we avoid code duplication and we make the > > function available to other drivers that might need to call it such as > > the test driver. > > > > Signed-off-by: Ilias Stamatis > > --- > > src/conf/domain_conf.c | 115 +++ > > src/conf/domain_conf.h | 6 ++ > > src/libvirt_private.syms | 1 + > > src/lxc/lxc_driver.c | 112 +- > > src/qemu/qemu_driver.c | 126 +++ > > 5 files changed, 132 insertions(+), 228 deletions(-) > > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > > index 3323c9a5b1..e10390189c 100644 > > --- a/src/conf/domain_conf.c > > +++ b/src/conf/domain_conf.c > > @@ -30219,6 +30219,121 @@ virDomainDefGetShortName(const virDomainDef *def) > > > > #undef VIR_DOMAIN_SHORT_NAME_MAX > > > > + > > +/* blkioDeviceStr 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 > > + */ > > +int > > +virDomainParseBlkioDeviceStr(char *blkioDeviceStr, > > The domain_conf.c module is meant to collect stuff regarding the XML > parser and configuration-related methods. > > This is a worker method which does not have to do anything with config. > > Please find a proper place in src/util/ for it. In order to add it into src/util we need to remove dependency on virBlkioDevicePtr from the code as in util we should not include anything from other parts of libvirt. Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] domain_conf: move DomainParseBlkioDeviceStr out of QEMU and LXC drivers
On Wed, Jul 10, 2019 at 17:44:31 +0200, Ilias Stamatis wrote: > The qemuDomainParseBlkioDeviceStr and lxcDomainParseBlkioDeviceSts > functions residing in the QEMU and LXC drivers respectively are > completely identical. > > By moving the code to src/conf we avoid code duplication and we make the > function available to other drivers that might need to call it such as > the test driver. > > Signed-off-by: Ilias Stamatis > --- > src/conf/domain_conf.c | 115 +++ > src/conf/domain_conf.h | 6 ++ > src/libvirt_private.syms | 1 + > src/lxc/lxc_driver.c | 112 +- > src/qemu/qemu_driver.c | 126 +++ > 5 files changed, 132 insertions(+), 228 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 3323c9a5b1..e10390189c 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -30219,6 +30219,121 @@ virDomainDefGetShortName(const virDomainDef *def) > > #undef VIR_DOMAIN_SHORT_NAME_MAX > > + > +/* blkioDeviceStr 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 > + */ > +int > +virDomainParseBlkioDeviceStr(char *blkioDeviceStr, The domain_conf.c module is meant to collect stuff regarding the XML parser and configuration-related methods. This is a worker method which does not have to do anything with config. Please find a proper place in src/util/ for it. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] domain_conf: move DomainParseBlkioDeviceStr out of QEMU and LXC drivers
The qemuDomainParseBlkioDeviceStr and lxcDomainParseBlkioDeviceSts
functions residing in the QEMU and LXC drivers respectively are
completely identical.
By moving the code to src/conf we avoid code duplication and we make the
function available to other drivers that might need to call it such as
the test driver.
Signed-off-by: Ilias Stamatis
---
src/conf/domain_conf.c | 115 +++
src/conf/domain_conf.h | 6 ++
src/libvirt_private.syms | 1 +
src/lxc/lxc_driver.c | 112 +-
src/qemu/qemu_driver.c | 126 +++
5 files changed, 132 insertions(+), 228 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3323c9a5b1..e10390189c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30219,6 +30219,121 @@ virDomainDefGetShortName(const virDomainDef *def)
#undef VIR_DOMAIN_SHORT_NAME_MAX
+
+/* blkioDeviceStr 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
+ */
+int
+virDomainParseBlkioDeviceStr(char *blkioDeviceStr,
+ const char *type,
+ virBlkioDevicePtr *dev,
+ size_t *size)
+{
+char *temp;
+int ndevices = 0;
+int nsep = 0;
+size_t i;
+virBlkioDevicePtr result = NULL;
+
+*dev = NULL;
+*size = 0;
+
+if (STREQ(blkioDeviceStr, ""))
+return 0;
+
+temp = blkioDeviceStr;
+while (temp) {
+temp = strchr(temp, ',');
+if (temp) {
+temp++;
+nsep++;
+}
+}
+
+/* A valid string must have even number of fields, hence an odd
+ * number of commas. */
+if (!(nsep & 1))
+goto parse_error;
+
+ndevices = (nsep + 1) / 2;
+
+if (VIR_ALLOC_N(result, ndevices) < 0)
+return -1;
+
+i = 0;
+temp = blkioDeviceStr;
+while (temp) {
+char *p = temp;
+
+/* device path */
+p = strchr(p, ',');
+if (!p)
+goto parse_error;
+
+if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0)
+goto cleanup;
+
+/* value */
+temp = p + 1;
+
+if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
+if (virStrToLong_uip(temp, , 10, [i].weight) < 0)
+goto number_error;
+} else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
+if (virStrToLong_uip(temp, , 10, [i].riops) < 0)
+goto number_error;
+} else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
+if (virStrToLong_uip(temp, , 10, [i].wiops) < 0)
+goto number_error;
+} else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
+if (virStrToLong_ullp(temp, , 10, [i].rbps) < 0)
+goto number_error;
+} else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
+if (virStrToLong_ullp(temp, , 10, [i].wbps) < 0)
+goto number_error;
+} else {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("unknown parameter '%s'"), type);
+goto cleanup;
+}
+
+i++;
+
+if (*p == '\0')
+break;
+else if (*p != ',')
+goto parse_error;
+temp = p + 1;
+}
+
+if (!i)
+VIR_FREE(result);
+
+*dev = result;
+*size = i;
+
+return 0;
+
+ parse_error:
+virReportError(VIR_ERR_INVALID_ARG,
+ _("unable to parse blkio device '%s' '%s'"),
+ type, blkioDeviceStr);
+goto cleanup;
+
+ number_error:
+virReportError(VIR_ERR_INVALID_ARG,
+ _("invalid value '%s' for parameter '%s' of device '%s'"),
+ temp, type, result[i].path);
+
+ cleanup:
+if (result) {
+virBlkioDeviceArrayClear(result, ndevices);
+VIR_FREE(result);
+}
+return -1;
+}
+
+
int
virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def,
virTypedParameterPtr params,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c1b5fc1337..f31193b8d6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3535,6 +3535,12 @@ bool virDomainDefHasMemballoon(const virDomainDef *def)
ATTRIBUTE_NONNULL(1);
char *virDomainDefGetShortName(const virDomainDef *def) ATTRIBUTE_NONNULL(1);
+int
+virDomainParseBlkioDeviceStr(char *blkioDeviceStr,
+ const char *type,
+ virBlkioDevicePtr *dev,
+ size_t *size);
+
int
virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def,
virTypedParameterPtr params,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0545c08428..5de3e6483f 100644
--- a/src/libvirt_private.syms
+++
