[libvirt] [PATCH 0/2] Improve error messages about blkio values
There was one message used for all parsing errors in the blkio code. This series fixes it in both qemu and lxc drivers where the code is used. Martin Kletzander (2): qemu: improve error message for invalid blkiotune settings lxc: improve error message for invalid blkiotune settings src/lxc/lxc_driver.c | 29 +++++++++++++++++++---------- src/qemu/qemu_driver.c | 29 +++++++++++++++++++---------- 2 files changed, 38 insertions(+), 20 deletions(-) -- 2.1.2
Before: $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,-1 error: Unable to change blkio parameters error: invalid argument: unable to parse blkio device 'device_read_bytes_sec' '/dev/sda,-1' After: $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,-1 error: Unable to change blkio parameters error: invalid argument: invalid value '-1' for parameter 'device_read_bytes_sec' of device '/dev/sda' Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1131306 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 373daab..50bfdcb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8083,7 +8083,7 @@ qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, /* A valid string must have even number of fields, hence an odd * number of commas. */ if (!(nsep & 1)) - goto error; + goto parse_error; ndevices = (nsep + 1) / 2; @@ -8098,7 +8098,7 @@ qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, /* device path */ p = strchr(p, ','); if (!p) - goto error; + goto parse_error; if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0) goto cleanup; @@ -8108,21 +8108,23 @@ qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { if (virStrToLong_uip(temp, &p, 10, &result[i].weight) < 0) - goto error; + goto number_error; } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { if (virStrToLong_uip(temp, &p, 10, &result[i].riops) < 0) - goto error; + goto number_error; } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { if (virStrToLong_uip(temp, &p, 10, &result[i].wiops) < 0) - goto error; + goto number_error; } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { if (virStrToLong_ullp(temp, &p, 10, &result[i].rbps) < 0) - goto error; + goto number_error; } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { if (virStrToLong_ullp(temp, &p, 10, &result[i].wbps) < 0) - goto error; + goto number_error; } else { - goto error; + virReportError(VIR_ERR_INVALID_ARG, + _("unknown parameter '%s'"), type); + goto cleanup; } i++; @@ -8130,7 +8132,7 @@ qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, if (*p == '\0') break; else if (*p != ',') - goto error; + goto parse_error; temp = p + 1; } @@ -8142,10 +8144,17 @@ qemuDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, return 0; - error: + 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); -- 2.1.2
Before: $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,-1 error: Unable to change blkio parameters error: invalid argument: unable to parse blkio device 'device_read_bytes_sec' '/dev/sda,-1' After: $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,-1 error: Unable to change blkio parameters error: invalid argument: invalid value '-1' for parameter 'device_read_bytes_sec' of device '/dev/sda' Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1131306 Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/lxc/lxc_driver.c | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 979382b..6a58d50 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2158,7 +2158,7 @@ lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, /* A valid string must have even number of fields, hence an odd * number of commas. */ if (!(nsep & 1)) - goto error; + goto parse_error; ndevices = (nsep + 1) / 2; @@ -2173,7 +2173,7 @@ lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, /* device path */ p = strchr(p, ','); if (!p) - goto error; + goto parse_error; if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0) goto cleanup; @@ -2183,21 +2183,23 @@ lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT)) { if (virStrToLong_uip(temp, &p, 10, &result[i].weight) < 0) - goto error; + goto number_error; } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_IOPS)) { if (virStrToLong_uip(temp, &p, 10, &result[i].riops) < 0) - goto error; + goto number_error; } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_IOPS)) { if (virStrToLong_uip(temp, &p, 10, &result[i].wiops) < 0) - goto error; + goto number_error; } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_READ_BPS)) { if (virStrToLong_ullp(temp, &p, 10, &result[i].rbps) < 0) - goto error; + goto number_error; } else if (STREQ(type, VIR_DOMAIN_BLKIO_DEVICE_WRITE_BPS)) { if (virStrToLong_ullp(temp, &p, 10, &result[i].wbps) < 0) - goto error; + goto number_error; } else { - goto error; + virReportError(VIR_ERR_INVALID_ARG, + _("unknown parameter '%s'"), type); + goto cleanup; } i++; @@ -2205,7 +2207,7 @@ lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, if (*p == '\0') break; else if (*p != ',') - goto error; + goto parse_error; temp = p + 1; } @@ -2217,10 +2219,17 @@ lxcDomainParseBlkioDeviceStr(char *blkioDeviceStr, const char *type, return 0; - error: + 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); -- 2.1.2
-----Original Message----- From: libvir-list-bounces@redhat.com [mailto:libvir-list-bounces@redhat.com] On Behalf Of Martin Kletzander Sent: Wednesday, October 29, 2014 11:45 PM To: libvir-list@redhat.com Subject: [libvirt] [PATCH 2/2] lxc: improve error message for invalid blkiotune settings
Before: $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,-1 error: Unable to change blkio parameters error: invalid argument: unable to parse blkio device 'device_read_bytes_sec' '/dev/sda,-1'
After: $ virsh blkiotune dummy --device-read-bytes-sec /dev/sda,-1 error: Unable to change blkio parameters error: invalid argument: invalid value '-1' for parameter 'device_read_bytes_sec' of device '/dev/sda'
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1131306
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
That would be much easier for users. Reviewed-by: Chen Hanxiao <chenhanxiao@cn.fujitsu.com> Thanks, - Chen
On 29.10.2014 16:44, Martin Kletzander wrote:
There was one message used for all parsing errors in the blkio code. This series fixes it in both qemu and lxc drivers where the code is used.
Martin Kletzander (2): qemu: improve error message for invalid blkiotune settings lxc: improve error message for invalid blkiotune settings
src/lxc/lxc_driver.c | 29 +++++++++++++++++++---------- src/qemu/qemu_driver.c | 29 +++++++++++++++++++---------- 2 files changed, 38 insertions(+), 20 deletions(-)
ACK and safe for the freeze. Michal
On Thu, Oct 30, 2014 at 06:08:16PM +0100, Michal Privoznik wrote:
On 29.10.2014 16:44, Martin Kletzander wrote:
There was one message used for all parsing errors in the blkio code. This series fixes it in both qemu and lxc drivers where the code is used.
Martin Kletzander (2): qemu: improve error message for invalid blkiotune settings lxc: improve error message for invalid blkiotune settings
src/lxc/lxc_driver.c | 29 +++++++++++++++++++---------- src/qemu/qemu_driver.c | 29 +++++++++++++++++++---------- 2 files changed, 38 insertions(+), 20 deletions(-)
ACK and safe for the freeze.
No rush with these patches, it was just a coincidence that I sent them during freeze. Thanks for the review, I'll push after release ;) Martin
participants (3)
-
Chen, Hanxiao -
Martin Kletzander -
Michal Privoznik