Rewrite the code to improve maintainability and also re-do construction of error messages which are assembled from non-translatable parts. Closes: https://gitlab.com/libvirt/libvirt/-/issues/455 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/vircgroupv1.c | 264 ++++++++++++++++++----------------------- 1 file changed, 117 insertions(+), 147 deletions(-) diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index c0cac2a6b6..77c7e209ce 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1051,189 +1051,159 @@ virCgroupV1GetBlkioWeight(virCgroup *group, } + static int -virCgroupV1GetBlkioIoServiced(virCgroup *group, - long long *bytes_read, - long long *bytes_write, - long long *requests_read, - long long *requests_write) +virCgroupV1GetBlkioIoServicedOne(virCgroup *group, + const char *field, + const char *devpath, + long long *dataRead, + long long *dataWrite) { - long long stats_val; - g_autofree char *str1 = NULL; - g_autofree char *str2 = NULL; - char *p1 = NULL; - char *p2 = NULL; - size_t i; + g_autofree char *serviced = NULL; + g_autofree char *filterWrite = NULL; + g_autofree char *filterRead = NULL; + unsigned long long tmpval; + char *tmp; + size_t len; - const char *value_names[] = { - "Read ", - "Write " - }; - long long *bytes_ptrs[] = { - bytes_read, - bytes_write - }; - long long *requests_ptrs[] = { - requests_read, - requests_write - }; - - *bytes_read = 0; - *bytes_write = 0; - *requests_read = 0; - *requests_write = 0; - - if (virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.io_service_bytes", &str1) < 0) - return -1; + *dataRead = 0; + *dataWrite = 0; - if (virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.io_serviced", &str2) < 0) + if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, field, &serviced) < 0) return -1; - /* sum up all entries of the same kind, from all devices */ - for (i = 0; i < G_N_ELEMENTS(value_names); i++) { - p1 = str1; - p2 = str2; - - while ((p1 = strstr(p1, value_names[i]))) { - p1 += strlen(value_names[i]); - if (virStrToLong_ll(p1, &p1, 10, &stats_val) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot parse byte %1$sstat '%2$s'"), - value_names[i], - p1); - return -1; - } + if (devpath) { + g_autofree char *devstr = NULL; - if (stats_val < 0 || - (stats_val > 0 && *bytes_ptrs[i] > (LLONG_MAX - stats_val))) - { - virReportError(VIR_ERR_OVERFLOW, - _("Sum of byte %1$sstat overflows"), - value_names[i]); - return -1; - } - *bytes_ptrs[i] += stats_val; - } + if (!(devstr = virCgroupGetBlockDevString(devpath))) + return -1; - while ((p2 = strstr(p2, value_names[i]))) { - p2 += strlen(value_names[i]); - if (virStrToLong_ll(p2, &p2, 10, &stats_val) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot parse %1$srequest stat '%2$s'"), - value_names[i], - p2); - return -1; - } + filterWrite = g_strdup_printf("%sWrite ", devstr); + filterRead = g_strdup_printf("%sRead ", devstr); - if (stats_val < 0 || - (stats_val > 0 && *requests_ptrs[i] > (LLONG_MAX - stats_val))) - { - virReportError(VIR_ERR_OVERFLOW, - _("Sum of %1$srequest stat overflows"), - value_names[i]); - return -1; - } - *requests_ptrs[i] += stats_val; + if (!strstr(serviced, filterWrite) || + !strstr(serviced, filterRead)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot find blkio cgroup stats (%1$s) for block device '%2$s' (%3$s)"), + field, devstr, devpath); + return -1; } + } else { + filterWrite = g_strdup("Write "); + filterRead = g_strdup("Read "); } - return 0; -} - - -static int -virCgroupV1GetBlkioIoDeviceServiced(virCgroup *group, - const char *path, - long long *bytes_read, - long long *bytes_write, - long long *requests_read, - long long *requests_write) -{ - g_autofree char *str1 = NULL; - g_autofree char *str2 = NULL; - g_autofree char *str3 = NULL; - char *p1 = NULL; - char *p2 = NULL; - size_t i; - - const char *value_names[] = { - "Read ", - "Write " - }; - long long *bytes_ptrs[] = { - bytes_read, - bytes_write - }; - long long *requests_ptrs[] = { - requests_read, - requests_write - }; - - if (virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.io_service_bytes", &str1) < 0) - return -1; + len = strlen(filterRead); - if (virCgroupGetValueStr(group, - VIR_CGROUP_CONTROLLER_BLKIO, - "blkio.throttle.io_serviced", &str2) < 0) - return -1; + for (tmp = strstr(serviced, filterRead); tmp; tmp = strstr(tmp, filterRead)) { + char *cur = tmp; + tmp += len; - if (!(str3 = virCgroupGetBlockDevString(path))) - return -1; + VIR_DEBUG("filter='%s' line='%.*s'", filterRead, (int) (strchr(tmp, '\n') - tmp), tmp); - if (!(p1 = strstr(str1, str3))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find byte stats for block device '%1$s'"), - str3); - return -1; - } + if (virStrToLong_ullp(tmp, &tmp, 10, &tmpval) < 0) { + char *eol; - if (!(p2 = strstr(str2, str3))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find request stats for block device '%1$s'"), - str3); - return -1; - } + if ((eol = strchr(cur, '\n'))) + *eol = '\0'; - for (i = 0; i < G_N_ELEMENTS(value_names); i++) { - if (!(p1 = strstr(p1, value_names[i]))) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find byte %1$sstats for block device '%2$s'"), - value_names[i], str3); + _("Cannot parse blkio cgroup (%1$s) entry '%2$s'"), + field, cur); return -1; } - if (virStrToLong_ll(p1 + strlen(value_names[i]), &p1, 10, bytes_ptrs[i]) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot parse %1$sstat '%2$s'"), - value_names[i], p1 + strlen(value_names[i])); + if (tmpval + *dataRead > LLONG_MAX) { + virReportError(VIR_ERR_OVERFLOW, + _("overflow in sum of statistic for blkio cgroup (%1$s) field '%2$s'"), + field, filterRead); return -1; } - if (!(p2 = strstr(p2, value_names[i]))) { + *dataRead += tmpval; + } + + len = strlen(filterWrite); + + for (tmp = strstr(serviced, filterWrite); tmp; tmp = strstr(tmp, filterWrite)) { + char *cur = tmp; + tmp += len; + + VIR_DEBUG("filter='%s' line='%.*s'", filterWrite, (int) (strchr(cur, '\n') - cur), cur); + + if (virStrToLong_ullp(tmp, &tmp, 10, &tmpval) < 0) { + char *eol; + + if ((eol = strchr(cur, '\n'))) + *eol = '\0'; + virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot find request %1$sstats for block device '%2$s'"), - value_names[i], str3); + _("Cannot parse blkio cgroup ('%1$s') entry '%2$s'"), + field, cur); return -1; } - if (virStrToLong_ll(p2 + strlen(value_names[i]), &p2, 10, requests_ptrs[i]) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot parse %1$sstat '%2$s'"), - value_names[i], p2 + strlen(value_names[i])); + if (tmpval + *dataWrite > LLONG_MAX) { + virReportError(VIR_ERR_OVERFLOW, + _("overflow in sum of statistic for blkio cgroup (%1$s) field '%2$s'"), + field, filterWrite); return -1; } + + *dataWrite += tmpval; } return 0; } +static int +virCgroupV1GetBlkioIoServicedInternal(virCgroup *group, + const char *devpath, + long long *bytes_read, + long long *bytes_write, + long long *requests_read, + long long *requests_write) +{ + if (virCgroupV1GetBlkioIoServicedOne(group, "blkio.throttle.io_service_bytes", + devpath, bytes_read, bytes_write) < 0) + return -1; + + if (virCgroupV1GetBlkioIoServicedOne(group, "blkio.throttle.io_serviced", + devpath, requests_read, requests_write) < 0) + return -1; + + return 0; +} + + +static int +virCgroupV1GetBlkioIoServiced(virCgroup *group, + long long *bytes_read, + long long *bytes_write, + long long *requests_read, + long long *requests_write) +{ + return virCgroupV1GetBlkioIoServicedInternal(group, NULL, + bytes_read, bytes_write, + requests_read, requests_write); +} + + +static int +virCgroupV1GetBlkioIoDeviceServiced(virCgroup *group, + const char *path, + long long *bytes_read, + long long *bytes_write, + long long *requests_read, + long long *requests_write) +{ + return virCgroupV1GetBlkioIoServicedInternal(group, path, + bytes_read, bytes_write, + requests_read, requests_write); +} + + static int virCgroupV1SetBlkioDeviceWeight(virCgroup *group, const char *devPath, -- 2.39.2

On a Thursday in 2023, Peter Krempa wrote: "unix" is actually translated in our Korean translation. Jano

Add a version for functions which may already need to take a printf format string. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/vsh.c | 32 ++++++++++++++++++++++---------- tools/vsh.h | 4 ++++ 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 5b672b8edf..41f55a91fa 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1865,34 +1865,46 @@ vshDebug(vshControl *ctl, int level, const char *format, ...) fflush(stdout); } + void -vshPrintExtra(vshControl *ctl, const char *format, ...) +vshPrintVa(vshControl *ctl G_GNUC_UNUSED, + const char *format, + va_list ap) { - va_list ap; g_autofree char *str = NULL; + str = g_strdup_vprintf(format, ap); + fputs(str, stdout); + fflush(stdout); +} + + +void +vshPrintExtra(vshControl *ctl, + const char *format, + ...) +{ + va_list ap; + if (ctl && ctl->quiet) return; va_start(ap, format); - str = g_strdup_vprintf(format, ap); + vshPrintVa(ctl, format, ap); va_end(ap); - fputs(str, stdout); - fflush(stdout); } void -vshPrint(vshControl *ctl G_GNUC_UNUSED, const char *format, ...) +vshPrint(vshControl *ctl, + const char *format, + ...) { va_list ap; - g_autofree char *str = NULL; va_start(ap, format); - str = g_strdup_vprintf(format, ap); + vshPrintVa(ctl, format, ap); va_end(ap); - fputs(str, stdout); - fflush(stdout); } diff --git a/tools/vsh.h b/tools/vsh.h index 51f09cd2b0..377c5947c1 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -302,6 +302,10 @@ const vshCmdOpt *vshCommandOptArgv(vshControl *ctl, const vshCmd *cmd, bool vshCommandArgvParse(vshControl *ctl, int nargs, char **argv); int vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout); +void vshPrintVa(vshControl *ctl, + const char *format, + va_list ap) + G_GNUC_PRINTF(2, 0); void vshPrint(vshControl *ctl, const char *format, ...) G_GNUC_PRINTF(2, 3); void vshPrintExtra(vshControl *ctl, const char *format, ...) -- 2.39.2

Remove construction of the event string from sub-strings marked as translatable. Without context it's impossible to translate it correctly. This slightly increases verbosity of the code but actually makes it more readable as everything is inline. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/virsh-domain-event.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/tools/virsh-domain-event.c b/tools/virsh-domain-event.c index a8a321590c..c522d79dd7 100644 --- a/tools/virsh-domain-event.c +++ b/tools/virsh-domain-event.c @@ -254,20 +254,6 @@ virshDomainEventDiskChangeToString(int reason) return str ? _(str) : _("unknown"); } -VIR_ENUM_DECL(virshDomainEventTrayChange); -VIR_ENUM_IMPL(virshDomainEventTrayChange, - VIR_DOMAIN_EVENT_TRAY_CHANGE_LAST, - N_("opened"), - N_("closed")); - -static const char * -virshDomainEventTrayChangeToString(int reason) -{ - const char *str = virshDomainEventTrayChangeTypeToString(reason); - return str ? _(str) : _("unknown"); -} - - struct virshDomainEventCallback { const char *name; virConnectDomainEventGenericCallback cb; @@ -511,13 +497,23 @@ virshEventTrayChangePrint(virConnectPtr conn G_GNUC_UNUSED, int reason, void *opaque) { - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + switch ((virDomainEventTrayChangeReason) reason) { + case VIR_DOMAIN_EVENT_TRAY_CHANGE_OPEN: + virshEventPrintf(opaque, _("event 'tray-change' for domain '%1$s' disk %2$s: opened\n"), + virDomainGetName(dom), alias); + break; - virBufferAsprintf(&buf, _("event 'tray-change' for domain '%1$s' disk %2$s: %3$s\n"), - virDomainGetName(dom), - alias, - virshDomainEventTrayChangeToString(reason)); - virshEventPrint(opaque, &buf); + case VIR_DOMAIN_EVENT_TRAY_CHANGE_CLOSE: + virshEventPrintf(opaque, _("event 'tray-change' for domain '%1$s' disk %2$s: closed\n"), + virDomainGetName(dom), alias); + break; + + case VIR_DOMAIN_EVENT_TRAY_CHANGE_LAST: + default: + virshEventPrintf(opaque, _("event 'tray-change' for domain '%1$s' disk %2$s: unknown\n"), + virDomainGetName(dom), alias); + break; + } } static void -- 2.39.2

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/virsh-domain-event.c | 70 +++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/tools/virsh-domain-event.c b/tools/virsh-domain-event.c index 8656b0fa09..2969c22a91 100644 --- a/tools/virsh-domain-event.c +++ b/tools/virsh-domain-event.c @@ -162,20 +162,6 @@ virshDomainEventDetailToString(int event, int detail) return str ? _(str) : _("unknown"); } -VIR_ENUM_DECL(virshDomainEventIOError); -VIR_ENUM_IMPL(virshDomainEventIOError, - VIR_DOMAIN_EVENT_IO_ERROR_LAST, - N_("none"), - N_("pause"), - N_("report")); - -static const char * -virshDomainEventIOErrorToString(int action) -{ - const char *str = virshDomainEventIOErrorTypeToString(action); - return str ? _(str) : _("unknown"); -} - VIR_ENUM_DECL(virshGraphicsPhase); VIR_ENUM_IMPL(virshGraphicsPhase, VIR_DOMAIN_EVENT_GRAPHICS_LAST, @@ -398,14 +384,25 @@ virshEventIOErrorPrint(virConnectPtr conn G_GNUC_UNUSED, int action, void *opaque) { - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - - virBufferAsprintf(&buf, _("event 'io-error' for domain '%1$s': %2$s (%3$s) %4$s\n"), - virDomainGetName(dom), - srcPath, - devAlias, - virshDomainEventIOErrorToString(action)); - virshEventPrint(opaque, &buf); + switch ((virDomainEventIOErrorAction) action) { + case VIR_DOMAIN_EVENT_IO_ERROR_NONE: + virshEventPrintf(opaque, _("event 'io-error' for domain '%1$s': %2$s (%3$s) none\n"), + virDomainGetName(dom), srcPath, devAlias); + break; + case VIR_DOMAIN_EVENT_IO_ERROR_PAUSE: + virshEventPrintf(opaque, _("event 'io-error' for domain '%1$s': %2$s (%3$s) pause\n"), + virDomainGetName(dom), srcPath, devAlias); + break; + case VIR_DOMAIN_EVENT_IO_ERROR_REPORT: + virshEventPrintf(opaque, _("event 'io-error' for domain '%1$s': %2$s (%3$s) report\n"), + virDomainGetName(dom), srcPath, devAlias); + break; + case VIR_DOMAIN_EVENT_IO_ERROR_LAST: + default: + virshEventPrintf(opaque, _("event 'io-error' for domain '%1$s': %2$s (%3$s) unknown\n"), + virDomainGetName(dom), srcPath, devAlias); + break; + } } static void @@ -449,16 +446,25 @@ virshEventIOErrorReasonPrint(virConnectPtr conn G_GNUC_UNUSED, const char *reason, void *opaque) { - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - - virBufferAsprintf(&buf, _("event 'io-error-reason' for domain '%1$s': " - "%2$s (%3$s) %4$s due to %5$s\n"), - virDomainGetName(dom), - srcPath, - devAlias, - virshDomainEventIOErrorToString(action), - reason); - virshEventPrint(opaque, &buf); + switch ((virDomainEventIOErrorAction) action) { + case VIR_DOMAIN_EVENT_IO_ERROR_NONE: + virshEventPrintf(opaque, _("event 'io-error' for domain '%1$s': %2$s (%3$s) none due to %4$s\n"), + virDomainGetName(dom), srcPath, devAlias, reason); + break; + case VIR_DOMAIN_EVENT_IO_ERROR_PAUSE: + virshEventPrintf(opaque, _("event 'io-error' for domain '%1$s': %2$s (%3$s) pause due to %4$s\n"), + virDomainGetName(dom), srcPath, devAlias, reason); + break; + case VIR_DOMAIN_EVENT_IO_ERROR_REPORT: + virshEventPrintf(opaque, _("event 'io-error' for domain '%1$s': %2$s (%3$s) report due to %4$s\n"), + virDomainGetName(dom), srcPath, devAlias, reason); + break; + case VIR_DOMAIN_EVENT_IO_ERROR_LAST: + default: + virshEventPrintf(opaque, _("event 'io-error' for domain '%1$s': %2$s (%3$s) unknown due to %4$s\n"), + virDomainGetName(dom), srcPath, devAlias, reason); + break; + } } static void -- 2.39.2