[libvirt PATCH 0/3] conf: do not use %s as a part of a word in translatable strings
Ján Tomko (3): conf: rename 'name' in scheduler parser conf: pass elementName to virDomainThreadSchedParseHelper conf: scheduler parser: do not hardcode element name src/conf/domain_conf.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) -- 2.26.2
virDomainThreadSchedParseHelper is used for parsing both iothread and vcpu scheduling settings. Rename its 'name' attribute to make it obvious this refers to the attribute name, not the name of the element (which is currently constructed from the attribute name). Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 386b04b5b8..fd968635fe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19965,17 +19965,17 @@ virDomainEmulatorSchedParse(xmlNodePtr node, static virBitmapPtr virDomainSchedulerParse(xmlNodePtr node, - const char *name, + const char *attributeName, virProcessSchedPolicy *policy, int *priority) { virBitmapPtr ret = NULL; g_autofree char *tmp = NULL; - if (!(tmp = virXMLPropString(node, name))) { + if (!(tmp = virXMLPropString(node, attributeName))) { virReportError(VIR_ERR_XML_ERROR, _("Missing attribute '%s' in element '%sched'"), - name, name); + attributeName, attributeName); goto error; } @@ -19985,7 +19985,7 @@ virDomainSchedulerParse(xmlNodePtr node, if (virBitmapIsAllClear(ret)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("'%s' scheduler bitmap '%s' is empty"), - name, tmp); + attributeName, tmp); goto error; } @@ -20002,7 +20002,7 @@ virDomainSchedulerParse(xmlNodePtr node, static int virDomainThreadSchedParseHelper(xmlNodePtr node, - const char *name, + const char *attributeName, virDomainThreadSchedParamPtr (*func)(virDomainDefPtr, unsigned int), virDomainDefPtr def) { @@ -20012,7 +20012,7 @@ virDomainThreadSchedParseHelper(xmlNodePtr node, int priority = 0; g_autoptr(virBitmap) map = NULL; - if (!(map = virDomainSchedulerParse(node, name, &policy, &priority))) + if (!(map = virDomainSchedulerParse(node, attributeName, &policy, &priority))) return -1; while ((next = virBitmapNextSetBit(map, next)) > -1) { @@ -20022,7 +20022,7 @@ virDomainThreadSchedParseHelper(xmlNodePtr node, if (sched->policy != VIR_PROC_POLICY_NONE) { virReportError(VIR_ERR_XML_DETAIL, _("%ssched attributes 'vcpus' must not overlap"), - STREQ(name, "vcpus") ? "vcpu" : name); + STREQ(attributeName, "vcpus") ? "vcpu" : attributeName); return -1; } -- 2.26.2
Pass the scheduler element name instead of trying to reconstructing it from the attribute name. This has the benefit of not mixing '%s' with regular text in translatable strings as well as preventing the confusion when the 's' marking the plural in the element name ('vcpus') is taken as a first letter of the 'sched' suffix. Signed-off-by: Ján Tomko <jtomko@redhat.com> Fixes: 7ea55a481dd45f09b54425005362533ecc800cd2 Fixes: 99c5fe0e7c26c08103415248ffef1f5acb81ddc7 --- src/conf/domain_conf.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fd968635fe..0f12b54575 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19965,6 +19965,7 @@ virDomainEmulatorSchedParse(xmlNodePtr node, static virBitmapPtr virDomainSchedulerParse(xmlNodePtr node, + const char *elementName, const char *attributeName, virProcessSchedPolicy *policy, int *priority) @@ -19974,8 +19975,8 @@ virDomainSchedulerParse(xmlNodePtr node, if (!(tmp = virXMLPropString(node, attributeName))) { virReportError(VIR_ERR_XML_ERROR, - _("Missing attribute '%s' in element '%sched'"), - attributeName, attributeName); + _("Missing attribute '%s' in element '%s'"), + attributeName, elementName); goto error; } @@ -20002,6 +20003,7 @@ virDomainSchedulerParse(xmlNodePtr node, static int virDomainThreadSchedParseHelper(xmlNodePtr node, + const char *elementName, const char *attributeName, virDomainThreadSchedParamPtr (*func)(virDomainDefPtr, unsigned int), virDomainDefPtr def) @@ -20012,7 +20014,8 @@ virDomainThreadSchedParseHelper(xmlNodePtr node, int priority = 0; g_autoptr(virBitmap) map = NULL; - if (!(map = virDomainSchedulerParse(node, attributeName, &policy, &priority))) + if (!(map = virDomainSchedulerParse(node, elementName, attributeName, + &policy, &priority))) return -1; while ((next = virBitmapNextSetBit(map, next)) > -1) { @@ -20021,8 +20024,8 @@ virDomainThreadSchedParseHelper(xmlNodePtr node, if (sched->policy != VIR_PROC_POLICY_NONE) { virReportError(VIR_ERR_XML_DETAIL, - _("%ssched attributes 'vcpus' must not overlap"), - STREQ(attributeName, "vcpus") ? "vcpu" : attributeName); + _("'%s' attributes 'vcpus' must not overlap"), + elementName); return -1; } @@ -20038,7 +20041,9 @@ static int virDomainVcpuThreadSchedParse(xmlNodePtr node, virDomainDefPtr def) { - return virDomainThreadSchedParseHelper(node, "vcpus", + return virDomainThreadSchedParseHelper(node, + "vcpusched", + "vcpus", virDomainDefGetVcpuSched, def); } @@ -20065,7 +20070,9 @@ static int virDomainIOThreadSchedParse(xmlNodePtr node, virDomainDefPtr def) { - return virDomainThreadSchedParseHelper(node, "iothreads", + return virDomainThreadSchedParseHelper(node, + "iothreadsched", + "iothreads", virDomainDefGetIOThreadSched, def); } -- 2.26.2
When trying to parse an XML with overlapping iothread scheduler settings, the error message was rather confusing: error: iothreadssched attributes 'vcpus' must not overlap Pass the correct element name. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0f12b54575..1179a27a00 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20024,8 +20024,8 @@ virDomainThreadSchedParseHelper(xmlNodePtr node, if (sched->policy != VIR_PROC_POLICY_NONE) { virReportError(VIR_ERR_XML_DETAIL, - _("'%s' attributes 'vcpus' must not overlap"), - elementName); + _("'%s' attributes '%s' must not overlap"), + elementName, attributeName); return -1; } -- 2.26.2
On Monday, 27 July 2020 15:15:46 CEST Ján Tomko wrote:
When trying to parse an XML with overlapping iothread scheduler settings, the error message was rather confusing:
error: iothreadssched attributes 'vcpus' must not overlap
Pass the correct element name.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/conf/domain_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0f12b54575..1179a27a00 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20024,8 +20024,8 @@ virDomainThreadSchedParseHelper(xmlNodePtr node,
if (sched->policy != VIR_PROC_POLICY_NONE) { virReportError(VIR_ERR_XML_DETAIL, - _("'%s' attributes 'vcpus' must not overlap"), - elementName); + _("'%s' attributes '%s' must not overlap"), + elementName, attributeName);
While I generally agree with this kind of changes, please note that this is difficult in C: see: https://www.gnu.org/software/gettext/manual/gettext.html#c_002dformat -- Pino Toscano
On Mon, Jul 27, 2020 at 03:15:43PM +0200, Ján Tomko wrote:
Ján Tomko (3): conf: rename 'name' in scheduler parser conf: pass elementName to virDomainThreadSchedParseHelper conf: scheduler parser: do not hardcode element name
src/conf/domain_conf.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Pino Toscano