Re: [PATCH v2 10/14] domain_driver.c: add virDomainDriverParseBlkioDeviceStr()

[Ján Tomko <jtomko@xxxxxxxxxx>]

On Mon, Feb 17, 2020 at 04:29:17PM -0500, Daniel Henrique Barboza wrote:
> lxcDomainParseBlkioDeviceStr() and qemuDomainParseBlkioDeviceStr()
> are the same function. Avoid code repetition by putting the code
> in a new helper.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
> ---
> po/POTFILES.in                 |   1 +
> src/hypervisor/domain_driver.c | 112 +++++++++++++++++++++++++++++
> src/hypervisor/domain_driver.h |   3 +
> src/libvirt_private.syms       |   1 +
> src/lxc/lxc_driver.c           | 122 +++----------------------------
> src/qemu/qemu_driver.c         | 126 +++------------------------------
> 6 files changed, 133 insertions(+), 232 deletions(-)
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index dba0d3a12e..aa5c1fb6c7 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_driver.c
> @SRCDIR@/src/interface/interface_backend_netcf.c
> @SRCDIR@/src/interface/interface_backend_udev.c
> @SRCDIR@/src/internal.h

This belongs in the previous patch, it already introduced a
virReportError.

> diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
> index c999458c25..bbfadb3d9b 100644
> --- a/src/hypervisor/domain_driver.c
> +++ b/src/hypervisor/domain_driver.c
> @@ -22,6 +22,7 @@
>
> #include "domain_driver.h"
> #include "viralloc.h"
> +#include "virstring.h"
>
> #define VIR_FROM_THIS VIR_FROM_DOMAIN
>
> @@ -94,3 +95,114 @@ virDomainDriverMergeBlkioDevice(virBlkioDevicePtr *dest_array,
>
>     return 0;
> }
> +
> +
> +/* blkioDeviceStr 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
> + */
> +int
> +virDomainDriverParseBlkioDeviceStr(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;
> +
> +        result[i].path = g_strndup(temp, p - temp);
> +
> +        /* value */
> +        temp = p + 1;
> +
> +        if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) {
> +            if (virStrToLong_uip(temp, &p, 10, &result[i].weight) < 0)
> +                goto number_error;
> +        } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) {
> +            if (virStrToLong_uip(temp, &p, 10, &result[i].riops) < 0)
> +                goto number_error;
> +        } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) {
> +            if (virStrToLong_uip(temp, &p, 10, &result[i].wiops) < 0)
> +                goto number_error;
> +        } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) {
> +            if (virStrToLong_ullp(temp, &p, 10, &result[i].rbps) < 0)
> +                goto number_error;
> +        } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) {
> +            if (virStrToLong_ullp(temp, &p, 10, &result[i].wbps) < 0)
> +                goto number_error;
> +        } else {
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                          _("unknown parameter '%s'"), type);

Indentation is off.

> +            goto cleanup;
> +        }
> +

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature