[PATCH 00/13] virsh: Enhancements for hypervisor-cpu-* commands and their documentation
This series originally started as a documentation update for hypervisor-cpu-compare and hypervisor-cpu-baseline commands with an idea to print a warning if the commands are used suboptimally. The rest is a refactor and a lot of fixes of (mostly) error messages printed by virsh. Jiri Denemark (13): docs: Clarify documentation of virsh hypervisor-cpu-compare docs: Clarify documentation of virsh hypervisor-cpu-baseline virsh: Do not format messages twice virsh: Make messages printed by vshError properly translatable virsh: Refactor vshError virsh: Introduce vshWarn virsh: Warn when hypervisor-cpu-* is used with host CPU virsh: Do not require \n in vshDebug messages virsh: Properly mark all error messages for translation virsh: Avoid using translated messages without format virsh: Drop extra newlines at the end of error messages virsh: Let prohibit_newline_at_end_of_diagnostic check pass build: Enable syntax checks for vshError and vshWarn build-aux/syntax-check.mk | 3 +- docs/manpages/virsh.rst | 47 ++++++++----- tools/virsh-domain-monitor.c | 16 ++--- tools/virsh-domain.c | 131 ++++++++++++++++++----------------- tools/virsh-host.c | 22 ++++-- tools/virsh-interface.c | 6 +- tools/virsh-network.c | 10 +-- tools/virsh-nodedev.c | 6 +- tools/virsh-nwfilter.c | 10 +-- tools/virsh-pool.c | 10 +-- tools/virsh-secret.c | 8 +-- tools/virsh-snapshot.c | 12 ++-- tools/virsh-util.c | 16 ++--- tools/virsh-volume.c | 24 +++---- tools/virsh.c | 10 +-- tools/virt-admin.c | 16 ++--- tools/vsh.c | 113 +++++++++++++++++------------- tools/vsh.h | 6 +- 18 files changed, 254 insertions(+), 212 deletions(-) -- 2.48.1
Using host CPU definition with hypervisor-cpu-compare is possible, but it provide incorrect results and thus it should not be documented the same way we describe the correct usage. Also using host-model CPU from domain capabilities was not described clearly enough. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- docs/manpages/virsh.rst | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 0eb1d6ea93..63ad619eab 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -954,15 +954,22 @@ provide on the host. (This is different from ``cpu-compare`` which compares the CPU definition with the host CPU without considering any specific hypervisor and its abilities.) -The XML *FILE* may contain either a host or guest CPU definition. The host CPU -definition is the <cpu> element and its contents as printed by the -``capabilities`` command. The guest CPU definition is the <cpu> element and its -contents from the domain XML definition or the CPU definition created from the -host CPU model found in the domain capabilities XML (printed by the -``domcapabilities`` command). In addition to the <cpu> element itself, this -command accepts full domain XML, capabilities XML, or domain capabilities XML -containing the CPU definition. For more information on guest CPU definition -see: `https://libvirt.org/formatdomain.html#elementsCPU <https://libvirt.org/formatdomain.html#cpu-model-and-topology>`__. +The XML *FILE* should contain a guest CPU definition: either the ``<cpu>`` +element and its contents from a domain XML definition or a CPU definition +created from the host CPU model found in the ``<mode name="host-model">`` +element in the domain capabilities XML (printed by the ``domcapabilities`` +command). The ``<mode name="host-model">`` element itself or even its +``<cpu>`` parent element found in domain capabilities XML is not accepted. +The element has to be transformed into an actual CPU definition. For more +information on guest CPU definition see: +`https://libvirt.org/formatdomain.html#elementsCPU <https://libvirt.org/formatdomain.html#cpu-model-and-topology>`__. + +Alternatively this command will automatically extract the CPU definition when +provided with a full domain or domain capabilities XML. + +For historical reasons the XML *FILE* may also contain a host CPU definition, +but such usage is strongly discouraged as it will most likely provide incorrect +results. The *virttype* option specifies the virtualization type (usable in the 'type' attribute of the <domain> top level element from the domain XML). *emulator* -- 2.48.1
Using host CPU definition with hypervisor-cpu-baseline is possible, but it provide incorrect results and thus it should not be documented the same way we describe the correct usage. Also using host-model CPU from domain capabilities was not described clearly enough. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- docs/manpages/virsh.rst | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 63ad619eab..06c2802b3f 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -1000,15 +1000,19 @@ As an alternative for *FILE* in case the XML would only contain a CPU model with no additional features the CPU model name itself can be passed as *model*. Exactly one of *FILE* and *model* must be used. -The XML *FILE* may contain either host or guest CPU definitions describing the -host CPU model. The host CPU definition is the <cpu> element and its contents -as printed by ``capabilities`` command. The guest CPU definition may be created -from the host CPU model found in domain capabilities XML (printed by -``domcapabilities`` command). In addition to the <cpu> elements, this command -accepts full capabilities XMLs, or domain capabilities XMLs containing the CPU -definitions. It is recommended to use only the CPU definitions from domain -capabilities, as on some architectures using the host CPU definition may either -fail or provide unexpected results. +The XML *FILE* should contain guest CPU definitions created from the host CPU +model found in the ``<mode name="host-model">`` element domain capabilities +XMLs (printed by the ``domcapabilities`` command on each host). The +``<mode name="host-model">`` elements themselves or even their ``<cpu>`` +parent elements found in domain capabilities XMLs are not accepted. The +elements have to be transformed into actual CPU definitions. + +Alternatively this command will automatically extract the CPU definitions when +provided with domain capabilities XMLs. + +For historical reasons the XML *FILE* may also contain host CPU definitions, +but such usage is strongly discouraged as it will most likely provide incorrect +results. When *FILE* contains only a single CPU definition, the command will print the same CPU with restrictions imposed by the capabilities of the hypervisor. -- 2.48.1
The same message was formatted both in vshOutputLogFile and in vshDebug and vshError functions. This patch refactor vshOutputLogFile and its callers to only format each message once. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/vsh.c | 27 +++++++++++---------------- tools/vsh.h | 4 +--- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 5f5e2f281d..1b650bdd9b 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1967,13 +1967,12 @@ vshDebug(vshControl *ctl, int level, const char *format, ...) if (level < ctl->debug) return; - va_start(ap, format); - vshOutputLogFile(ctl, level, format, ap); - va_end(ap); - va_start(ap, format); str = g_strdup_vprintf(format, ap); va_end(ap); + + vshOutputLogFile(ctl, level, str); + fputs(str, stdout); fflush(stdout); } @@ -2118,11 +2117,12 @@ vshError(vshControl *ctl, const char *format, ...) va_list ap; g_autofree char *str = NULL; - if (ctl != NULL) { - va_start(ap, format); - vshOutputLogFile(ctl, VSH_ERR_ERROR, format, ap); - va_end(ap); - } + va_start(ap, format); + str = g_strdup_vprintf(format, ap); + va_end(ap); + + if (ctl) + vshOutputLogFile(ctl, VSH_ERR_ERROR, str); /* Most output is to stdout, but if someone ran virsh 2>&1, then * printing to stderr will not interleave correctly with stdout @@ -2130,10 +2130,6 @@ vshError(vshControl *ctl, const char *format, ...) fflush(stdout); fputs(_("error: "), stderr); - va_start(ap, format); - str = g_strdup_vprintf(format, ap); - va_end(ap); - fprintf(stderr, "%s\n", NULLSTR(str)); fflush(stderr); } @@ -2337,8 +2333,7 @@ vshOpenLogFile(vshControl *ctl) * Outputting an error to log file. */ void -vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, - va_list ap) +vshOutputLogFile(vshControl *ctl, int log_level, const char *msg) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_autofree char *str = NULL; @@ -2381,7 +2376,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, break; } virBufferAsprintf(&buf, "%s ", lvl); - virBufferVasprintf(&buf, msg_format, ap); + virBufferAddStr(&buf, msg); virBufferTrim(&buf, "\n"); virBufferAddChar(&buf, '\n'); diff --git a/tools/vsh.h b/tools/vsh.h index 7de9fa2f09..1c7370dd4f 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -241,9 +241,7 @@ struct _vshCmdGrp { void vshError(vshControl *ctl, const char *format, ...) G_GNUC_PRINTF(2, 3); void vshOpenLogFile(vshControl *ctl); -void vshOutputLogFile(vshControl *ctl, int log_level, const char *format, - va_list ap) - G_GNUC_PRINTF(3, 0); +void vshOutputLogFile(vshControl *ctl, int log_level, const char *msg); void vshCloseLogFile(vshControl *ctl); int vshCommandOptInt(vshControl *ctl, const vshCmd *cmd, -- 2.48.1
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/vsh.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 1b650bdd9b..64507fe560 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2128,9 +2128,7 @@ vshError(vshControl *ctl, const char *format, ...) * printing to stderr will not interleave correctly with stdout * unless we flush between every transition between streams. */ fflush(stdout); - fputs(_("error: "), stderr); - - fprintf(stderr, "%s\n", NULLSTR(str)); + fprintf(stderr, _("error: %1$s\n"), NULLSTR(str)); fflush(stderr); } -- 2.48.1
The code is moved into a newly introduced generic vshPrintStderr and vshError changed into a tiny wrapper. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/vsh.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 64507fe560..91e2ae2067 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2111,18 +2111,18 @@ vshTTYMakeRaw(vshControl *ctl G_GNUC_UNUSED, } -void -vshError(vshControl *ctl, const char *format, ...) +static void G_GNUC_PRINTF(3, 0) +vshPrintStderr(vshControl *ctl, + int level, + const char *format, + va_list ap) { - va_list ap; g_autofree char *str = NULL; - va_start(ap, format); str = g_strdup_vprintf(format, ap); - va_end(ap); if (ctl) - vshOutputLogFile(ctl, VSH_ERR_ERROR, str); + vshOutputLogFile(ctl, level, str); /* Most output is to stdout, but if someone ran virsh 2>&1, then * printing to stderr will not interleave correctly with stdout @@ -2133,6 +2133,17 @@ vshError(vshControl *ctl, const char *format, ...) } +void +vshError(vshControl *ctl, const char *format, ...) +{ + va_list ap; + + va_start(ap, format); + vshPrintStderr(ctl, VSH_ERR_ERROR, format, ap); + va_end(ap); +} + + void vshEventLoop(void *opaque) { -- 2.48.1
This new function can be used for printing warnings about suboptimal usage. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh-secret.c | 2 +- tools/vsh.c | 13 ++++++++++++- tools/vsh.h | 2 ++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index d9435e4357..6655d3211e 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -235,7 +235,7 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd) if (base64) { /* warn users that the --base64 option passed from command line is wrong */ - vshError(ctl, _("Passing secret value as command-line argument is insecure!")); + vshWarn(ctl, _("Passing secret value as command-line argument is insecure!")); secret_val = g_strdup(base64); secret_len = strlen(secret_val); } else if (filename) { diff --git a/tools/vsh.c b/tools/vsh.c index 91e2ae2067..812fc81bde 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2144,6 +2144,17 @@ vshError(vshControl *ctl, const char *format, ...) } +void +vshWarn(vshControl *ctl, const char *format, ...) +{ + va_list ap; + + va_start(ap, format); + vshPrintStderr(ctl, VSH_ERR_WARNING, format, ap); + va_end(ap); +} + + void vshEventLoop(void *opaque) { @@ -2501,7 +2512,7 @@ vshAskReedit(vshControl *ctl, const char *msg G_GNUC_UNUSED, bool relax_avail G_GNUC_UNUSED) { - vshDebug(ctl, VSH_ERR_WARNING, "%s", _("This function is not supported on WIN32 platform")); + vshWarn(ctl, "%s", _("This function is not supported on WIN32 platform")); return 0; } #endif /* WIN32 */ diff --git a/tools/vsh.h b/tools/vsh.h index 1c7370dd4f..5970addfb6 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -240,6 +240,8 @@ struct _vshCmdGrp { void vshError(vshControl *ctl, const char *format, ...) G_GNUC_PRINTF(2, 3); +void vshWarn(vshControl *ctl, const char *format, ...) + G_GNUC_PRINTF(2, 3); void vshOpenLogFile(vshControl *ctl); void vshOutputLogFile(vshControl *ctl, int log_level, const char *msg); void vshCloseLogFile(vshControl *ctl); -- 2.48.1
While using host CPU definition from capabilities XML is allowed for historical reasons, it will likely provide incorrect results and should be avoided. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh-host.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index f4e7324f42..418bd995da 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -1093,7 +1093,8 @@ cmdURI(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) */ static char ** vshExtractCPUDefXMLs(vshControl *ctl, - const char *xmlFile) + const char *xmlFile, + bool hostCPUWarning) { g_auto(GStrv) cpus = NULL; g_autofree char *buffer = NULL; @@ -1137,6 +1138,8 @@ vshExtractCPUDefXMLs(vshControl *ctl, cpus = g_new0(char *, n + 1); for (i = 0; i < n; i++) { + ctxt->node = nodes[i]; + /* If the user provided domain capabilities XML, we need to replace * <mode ...> element with <cpu>. */ if (xmlStrEqual(nodes[i]->name, BAD_CAST "mode")) { @@ -1150,6 +1153,11 @@ vshExtractCPUDefXMLs(vshControl *ctl, } } + if (hostCPUWarning && + virXPathBoolean("boolean(./feature[not(@policy)])", ctxt) == 1) { + vshWarn(ctl, "%s", _("using host CPU definition as input may provide incorrect results")); + } + if (!(cpus[i] = virXMLNodeToString(xml, nodes[i]))) { vshSaveLibvirtError(); return NULL; @@ -1199,7 +1207,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; - if (!(cpus = vshExtractCPUDefXMLs(ctl, from))) + if (!(cpus = vshExtractCPUDefXMLs(ctl, from, false))) return false; result = virConnectCompareCPU(priv->conn, cpus[0], flags); @@ -1268,7 +1276,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(ctl, cmd, "file", &from) < 0) return false; - if (!(list = vshExtractCPUDefXMLs(ctl, from))) + if (!(list = vshExtractCPUDefXMLs(ctl, from, false))) return false; if (!(result = virConnectBaselineCPU(priv->conn, (const char **)list, @@ -1612,7 +1620,7 @@ cmdHypervisorCPUCompare(vshControl *ctl, vshCommandOptString(ctl, cmd, "machine", &machine) < 0) return false; - if (!(cpus = vshExtractCPUDefXMLs(ctl, from))) + if (!(cpus = vshExtractCPUDefXMLs(ctl, from, true))) return false; result = virConnectCompareHypervisorCPU(priv->conn, emulator, arch, @@ -1736,7 +1744,7 @@ cmdHypervisorCPUBaseline(vshControl *ctl, VSH_ALTERNATIVE_OPTIONS_EXPR("file", from, "model", model); if (from) { - if (!(list = vshExtractCPUDefXMLs(ctl, from))) + if (!(list = vshExtractCPUDefXMLs(ctl, from, true))) return false; } else { list = g_new0(char *, 2); -- 2.48.1
Having to put a newline at the end of each debug message in virsh has always felt strange. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh-domain-monitor.c | 2 +- tools/virsh-domain.c | 32 ++++++++++++++++---------------- tools/virsh-interface.c | 6 +++--- tools/virsh-network.c | 10 +++++----- tools/virsh-nwfilter.c | 10 +++++----- tools/virsh-pool.c | 10 +++++----- tools/virsh-secret.c | 2 +- tools/virsh-util.c | 8 ++++---- tools/virsh-volume.c | 20 ++++++++++---------- tools/virsh.c | 4 ++-- tools/virt-admin.c | 2 +- tools/vsh.c | 8 ++++---- 12 files changed, 57 insertions(+), 57 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 9ee9090c11..76843005b2 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1224,7 +1224,7 @@ cmdDominfo(vshControl *ctl, const vshCmd *cmd) /* Check and display whether the domain is persistent or not */ persistent = virDomainIsPersistent(dom); - vshDebug(ctl, VSH_ERR_DEBUG, "Domain persistent flag value: %d\n", + vshDebug(ctl, VSH_ERR_DEBUG, "Domain persistent flag value: %d", persistent); if (persistent < 0) vshPrint(ctl, "%-15s %s\n", _("Persistent:"), _("unknown")); diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f3da2f903f..171bc0ce17 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4254,7 +4254,7 @@ virshWatchTimeout(gpointer opaque) struct virshWatchData *data = opaque; /* suspend the domain when migration timeouts. */ - vshDebug(data->ctl, VSH_ERR_DEBUG, "watchJob: timeout\n"); + vshDebug(data->ctl, VSH_ERR_DEBUG, "watchJob: timeout"); if (data->timeout_func) (data->timeout_func)(data->ctl, data->dom, data->opaque); @@ -4266,7 +4266,7 @@ static void virshWatchSetTimeout(struct virshWatchData *data) { vshDebug(data->ctl, VSH_ERR_DEBUG, - "watchJob: setting timeout of %d secs\n", data->timeout_secs); + "watchJob: setting timeout of %d secs", data->timeout_secs); data->timeout_src = g_timeout_source_new_seconds(data->timeout_secs); g_source_set_callback(data->timeout_src, @@ -4291,7 +4291,7 @@ virshWatchProgress(gpointer opaque) pthread_sigmask(SIG_BLOCK, &sigmask, &oldsigmask); #endif /* !WIN32 */ vshDebug(data->ctl, VSH_ERR_DEBUG, "%s", - "watchJob: progress update\n"); + "watchJob: progress update"); ret = virDomainGetJobInfo(data->dom, &jobinfo); #ifndef WIN32 pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL); @@ -4308,7 +4308,7 @@ virshWatchProgress(gpointer opaque) vshTTYDisableInterrupt(data->ctl); data->jobStarted = true; vshDebug(data->ctl, VSH_ERR_DEBUG, - "watchJob: job started\n"); + "watchJob: job started"); } if (data->jobStarted) { @@ -4317,7 +4317,7 @@ virshWatchProgress(gpointer opaque) virshWatchSetTimeout(data); } else if (!data->verbose) { vshDebug(data->ctl, VSH_ERR_DEBUG, - "watchJob: disabling callback\n"); + "watchJob: disabling callback"); return G_SOURCE_REMOVE; } } @@ -4339,7 +4339,7 @@ virshWatchInterrupt(GIOChannel *source G_GNUC_UNUSED, gsize nread = 0; vshDebug(data->ctl, VSH_ERR_DEBUG, - "watchJob: stdin data %d\n", condition); + "watchJob: stdin data %d", condition); if (condition & G_IO_IN) { g_io_channel_read_chars(data->stdin_ioc, &retchar, @@ -4348,7 +4348,7 @@ virshWatchInterrupt(GIOChannel *source G_GNUC_UNUSED, NULL); vshDebug(data->ctl, VSH_ERR_DEBUG, - "watchJob: got %zu characters\n", nread); + "watchJob: got %zu characters", nread); if (nread == 1 && vshTTYIsInterruptCharacter(data->ctl, retchar)) { virDomainAbortJob(data->dom); @@ -4407,7 +4407,7 @@ virshWatchJob(vshControl *ctl, /* don't poll on STDIN if we are not using a terminal */ if (vshTTYAvailable(ctl)) { vshDebug(ctl, VSH_ERR_DEBUG, "%s", - "watchJob: on TTY, enabling Ctrl-c processing\n"); + "watchJob: on TTY, enabling Ctrl-c processing"); #ifdef WIN32 data.stdin_ioc = g_io_channel_win32_new_fd(STDIN_FILENO); #else @@ -4429,7 +4429,7 @@ virshWatchJob(vshControl *ctl, g_main_loop_run(eventLoop); vshDebug(ctl, VSH_ERR_DEBUG, - "watchJob: job done, status %d\n", *job_err); + "watchJob: job done, status %d", *job_err); if (*job_err == 0 && verbose) /* print [100 %] */ virshPrintJobProgress(label, 0, 1); @@ -6118,7 +6118,7 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) _("Optional flags or --rawstats are not supported by the daemon")); goto cleanup; } - vshDebug(ctl, VSH_ERR_DEBUG, "detailed statistics not supported\n"); + vshDebug(ctl, VSH_ERR_DEBUG, "detailed statistics not supported"); vshResetLibvirtError(); rc = virDomainGetJobInfo(dom, &info); } @@ -11158,16 +11158,16 @@ virshMigrateTimeout(vshControl *ctl, case VIRSH_MIGRATE_TIMEOUT_DEFAULT: /* unreachable */ case VIRSH_MIGRATE_TIMEOUT_SUSPEND: vshDebug(ctl, VSH_ERR_DEBUG, - "migration timed out; suspending domain\n"); + "migration timed out; suspending domain"); if (virDomainSuspend(dom) < 0) - vshDebug(ctl, VSH_ERR_INFO, "suspending domain failed\n"); + vshDebug(ctl, VSH_ERR_INFO, "suspending domain failed"); break; case VIRSH_MIGRATE_TIMEOUT_POSTCOPY: vshDebug(ctl, VSH_ERR_DEBUG, - "migration timed out; switching to post-copy\n"); + "migration timed out; switching to post-copy"); if (virDomainMigrateStartPostCopy(dom, 0) < 0) - vshDebug(ctl, VSH_ERR_INFO, "switching to post-copy failed\n"); + vshDebug(ctl, VSH_ERR_INFO, "switching to post-copy failed"); break; } } @@ -11182,10 +11182,10 @@ virshMigrateIteration(virConnectPtr conn G_GNUC_UNUSED, if (iteration == 2) { vshDebug(ctl, VSH_ERR_DEBUG, - "iteration %d finished; switching to post-copy\n", + "iteration %d finished; switching to post-copy", iteration - 1); if (virDomainMigrateStartPostCopy(dom, 0) < 0) - vshDebug(ctl, VSH_ERR_INFO, "switching to post-copy failed\n"); + vshDebug(ctl, VSH_ERR_INFO, "switching to post-copy failed"); } } diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index fda6d55158..7e3103adf1 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -61,7 +61,7 @@ virshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; - vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s", cmd->def->name, optname, n); if (name) @@ -72,13 +72,13 @@ virshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, /* try it by NAME */ if (!is_mac && (flags & VIRSH_BYNAME)) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as interface NAME\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as interface NAME", cmd->def->name, optname); iface = virInterfaceLookupByName(priv->conn, n); /* try it by MAC */ } else if (is_mac && (flags & VIRSH_BYMAC)) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as interface MAC\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as interface MAC", cmd->def->name, optname); iface = virInterfaceLookupByMACString(priv->conn, n); } diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 6fcc7fd8ee..da396c0002 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -68,7 +68,7 @@ virshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; - vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s", cmd->def->name, optname, n); if (name) @@ -76,13 +76,13 @@ virshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, /* try it by UUID */ if ((flags & VIRSH_BYUUID) && strlen(n) == VIR_UUID_STRING_BUFLEN-1) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as network UUID\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as network UUID", cmd->def->name, optname); network = virNetworkLookupByUUIDString(priv->conn, n); } /* try it by NAME */ if (!network && (flags & VIRSH_BYNAME)) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as network NAME\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as network NAME", cmd->def->name, optname); network = virNetworkLookupByName(priv->conn, n); } @@ -106,13 +106,13 @@ virshCommandOptNetworkPort(vshControl *ctl, const vshCmd *cmd, if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; - vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s", cmd->def->name, optname, n); if (name) *name = n; - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as network UUID\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as network UUID", cmd->def->name, optname); port = virNetworkPortLookupByUUIDString(net, n); diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 56133d6566..3745abb67e 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -41,7 +41,7 @@ virshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; - vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s", cmd->def->name, optname, n); if (name) @@ -49,13 +49,13 @@ virshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, /* try it by UUID */ if ((flags & VIRSH_BYUUID) && strlen(n) == VIR_UUID_STRING_BUFLEN-1) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as nwfilter UUID\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as nwfilter UUID", cmd->def->name, optname); nwfilter = virNWFilterLookupByUUIDString(priv->conn, n); } /* try it by NAME */ if (!nwfilter && (flags & VIRSH_BYNAME)) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as nwfilter NAME\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as nwfilter NAME", cmd->def->name, optname); nwfilter = virNWFilterLookupByName(priv->conn, n); } @@ -450,13 +450,13 @@ virshCommandOptNWFilterBindingBy(vshControl *ctl, if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; - vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s", cmd->def->name, optname, n); if (name) *name = n; - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as nwfilter binding port dev\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as nwfilter binding port dev", cmd->def->name, optname); binding = virNWFilterBindingLookupByPortDev(priv->conn, n); diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 4cd7b7ba0b..33b130564e 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -186,7 +186,7 @@ virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, if (cmd->skipChecks && !n) return NULL; - vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s", cmd->def->name, optname, n); if (name) @@ -194,13 +194,13 @@ virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, /* try it by UUID */ if ((flags & VIRSH_BYUUID) && strlen(n) == VIR_UUID_STRING_BUFLEN-1) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as pool UUID\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as pool UUID", cmd->def->name, optname); pool = virStoragePoolLookupByUUIDString(priv->conn, n); } /* try it by NAME */ if (!pool && (flags & VIRSH_BYNAME)) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as pool NAME\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as pool NAME", cmd->def->name, optname); pool = virStoragePoolLookupByName(priv->conn, n); } @@ -1211,7 +1211,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) /* Retrieve the persistence status of the pool */ if (details) { persistent = virStoragePoolIsPersistent(list->pools[i]); - vshDebug(ctl, VSH_ERR_DEBUG, "Persistent flag value: %d\n", + vshDebug(ctl, VSH_ERR_DEBUG, "Persistent flag value: %d", persistent); if (persistent < 0) poolInfoTexts[i].persistent = g_strdup(_("unknown")); @@ -1545,7 +1545,7 @@ cmdPoolInfo(vshControl *ctl, const vshCmd *cmd) /* Check and display whether the pool is persistent or not */ persistent = virStoragePoolIsPersistent(pool); - vshDebug(ctl, VSH_ERR_DEBUG, "Pool persistent flag value: %d\n", + vshDebug(ctl, VSH_ERR_DEBUG, "Pool persistent flag value: %d", persistent); if (persistent < 0) vshPrint(ctl, "%-15s %s\n", _("Persistent:"), _("unknown")); diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index 6655d3211e..7c81c10845 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -45,7 +45,7 @@ virshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, const char **name) return NULL; vshDebug(ctl, VSH_ERR_DEBUG, - "%s: found option <%s>: %s\n", cmd->def->name, optname, n); + "%s: found option <%s>: %s", cmd->def->name, optname, n); if (name != NULL) *name = n; diff --git a/tools/virsh-util.c b/tools/virsh-util.c index ab350f0326..55ab71f26a 100644 --- a/tools/virsh-util.c +++ b/tools/virsh-util.c @@ -39,7 +39,7 @@ virshLookupDomainInternal(vshControl *ctl, /* try it by ID */ if (flags & VIRSH_BYID) { if (virStrToLong_i(name, NULL, 10, &id) == 0 && id >= 0) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <domain> looks like ID\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <domain> looks like ID", cmdname); dom = virDomainLookupByID(priv->conn, id); } @@ -48,14 +48,14 @@ virshLookupDomainInternal(vshControl *ctl, /* try it by UUID */ if (!dom && (flags & VIRSH_BYUUID) && strlen(name) == VIR_UUID_STRING_BUFLEN-1) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <domain> trying as domain UUID\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <domain> trying as domain UUID", cmdname); dom = virDomainLookupByUUIDString(priv->conn, name); } /* try it by NAME */ if (!dom && (flags & VIRSH_BYNAME)) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <domain> trying as domain NAME\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <domain> trying as domain NAME", cmdname); dom = virDomainLookupByName(priv->conn, name); } @@ -90,7 +90,7 @@ virshCommandOptDomainBy(vshControl *ctl, if (vshCommandOptString(ctl, cmd, optname, &n) < 0) return NULL; - vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s", cmd->def->name, optname, n); if (name) diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 8805b8c06b..430961fef2 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -109,7 +109,7 @@ virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, } } - vshDebug(ctl, VSH_ERR_DEBUG, "%s: found option <%s>: %s\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: found option <%s>: %s", cmd->def->name, optname, n); if (name) @@ -117,19 +117,19 @@ virshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, /* try it by name */ if (pool && (flags & VIRSH_BYNAME)) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as vol name\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as vol name", cmd->def->name, optname); vol = virStorageVolLookupByName(pool, n); } /* try it by key */ if (!vol && (flags & VIRSH_BYUUID)) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as vol key\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as vol key", cmd->def->name, optname); vol = virStorageVolLookupByKey(priv->conn, n); } /* try it by path */ if (!vol && (flags & VIRSH_BYUUID)) { - vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as vol path\n", + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as vol path", cmd->def->name, optname); vol = virStorageVolLookupByPath(priv->conn, n); } @@ -293,36 +293,36 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) /* Lookup snapshot backing volume. Try the backing-vol * parameter as a name */ vshDebug(ctl, VSH_ERR_DEBUG, - "%s: Look up backing store volume '%s' as name\n", + "%s: Look up backing store volume '%s' as name", cmd->def->name, snapshotStrVol); snapVol = virStorageVolLookupByName(pool, snapshotStrVol); if (snapVol) vshDebug(ctl, VSH_ERR_DEBUG, - "%s: Backing store volume found using '%s' as name\n", + "%s: Backing store volume found using '%s' as name", cmd->def->name, snapshotStrVol); if (snapVol == NULL) { /* Snapshot backing volume not found by name. Try the * backing-vol parameter as a key */ vshDebug(ctl, VSH_ERR_DEBUG, - "%s: Look up backing store volume '%s' as key\n", + "%s: Look up backing store volume '%s' as key", cmd->def->name, snapshotStrVol); snapVol = virStorageVolLookupByKey(priv->conn, snapshotStrVol); if (snapVol) vshDebug(ctl, VSH_ERR_DEBUG, - "%s: Backing store volume found using '%s' as key\n", + "%s: Backing store volume found using '%s' as key", cmd->def->name, snapshotStrVol); } if (snapVol == NULL) { /* Snapshot backing volume not found by key. Try the * backing-vol parameter as a path */ vshDebug(ctl, VSH_ERR_DEBUG, - "%s: Look up backing store volume '%s' as path\n", + "%s: Look up backing store volume '%s' as path", cmd->def->name, snapshotStrVol); snapVol = virStorageVolLookupByPath(priv->conn, snapshotStrVol); if (snapVol) vshDebug(ctl, VSH_ERR_DEBUG, - "%s: Backing store volume found using '%s' as path\n", + "%s: Backing store volume found using '%s' as path", cmd->def->name, snapshotStrVol); } if (snapVol == NULL) { diff --git a/tools/virsh.c b/tools/virsh.c index a958f2b82f..6abae3dae9 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -165,7 +165,7 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) goto cleanup; } vshDebug(ctl, VSH_ERR_INFO, "%s", - _("Failed to setup keepalive on connection\n")); + _("Failed to setup keepalive on connection")); vshResetLibvirtError(); } @@ -769,7 +769,7 @@ virshParseArgv(vshControl *ctl, int argc, char **argv) /* parse command */ ctl->imode = false; if (argc - optind == 1) { - vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]); + vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"", argv[optind]); return vshCommandStringParse(ctl, argv[optind], NULL); } else { return vshCommandArgvParse(ctl, argc - optind, argv + optind); diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 325b7aa827..16c9b2173a 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1332,7 +1332,7 @@ vshAdmParseArgv(vshControl *ctl, int argc, char **argv) /* parse command */ ctl->imode = false; if (argc - optind == 1) { - vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]); + vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"", argv[optind]); return vshCommandStringParse(ctl, argv[optind], NULL); } else { return vshCommandArgvParse(ctl, argc - optind, argv + optind); diff --git a/tools/vsh.c b/tools/vsh.c index 812fc81bde..fb00cc40a6 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -542,7 +542,7 @@ vshCmdOptAssign(vshControl *ctl, case VSH_OT_BOOL: /* nothing to do */ if (report) { - vshDebug(ctl, VSH_ERR_INFO, "%s: %s(bool)\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: %s(bool)", cmd->def->name, opt->def->name); } break; @@ -550,7 +550,7 @@ vshCmdOptAssign(vshControl *ctl, case VSH_OT_STRING: case VSH_OT_INT: if (report) { - vshDebug(ctl, VSH_ERR_INFO, "%s: %s(optdata): %s\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: %s(optdata): %s", cmd->def->name, opt->def->name, NULLSTR(val)); } @@ -559,7 +559,7 @@ vshCmdOptAssign(vshControl *ctl, case VSH_OT_ARGV: if (report) { - vshDebug(ctl, VSH_ERR_INFO, "%s: %s(argv: %zu): %s\n", + vshDebug(ctl, VSH_ERR_INFO, "%s: %s(argv: %zu): %s", cmd->def->name, opt->def->name, opt->nargv, NULLSTR(val)); } @@ -1973,7 +1973,7 @@ vshDebug(vshControl *ctl, int level, const char *format, ...) vshOutputLogFile(ctl, level, str); - fputs(str, stdout); + fprintf(stderr, "%s\n", str); fflush(stdout); } -- 2.48.1
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh-domain-monitor.c | 6 ++--- tools/virsh-domain.c | 12 ++++----- tools/virsh-host.c | 2 +- tools/virsh-nodedev.c | 6 ++--- tools/virsh-secret.c | 2 +- tools/virsh.c | 4 +-- tools/virt-admin.c | 4 +-- tools/vsh.c | 50 +++++++++++++++++++----------------- 8 files changed, 45 insertions(+), 41 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 76843005b2..150e6ec806 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -452,7 +452,7 @@ cmdDomblkinfo(vshControl *ctl, const vshCmd *cmd) all = vshCommandOptBool(cmd, "all"); if (!all && vshCommandOptStringQuiet(ctl, cmd, "device", &device) <= 0) { - vshError(ctl, "command 'domblkinfo' requires <device> option"); + vshError(ctl, "%s", _("command 'domblkinfo' requires <device> option")); return false; } @@ -604,7 +604,7 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) target = virXPathString("string(./target/@dev)", ctxt); if (!target) { - vshError(ctl, "unable to query block list"); + vshError(ctl, "%s", _("unable to query block list")); return false; } @@ -616,7 +616,7 @@ cmdDomblklist(vshControl *ctl, const vshCmd *cmd) if (!(namespace = virXPathString("string(./source/@namespace)", ctxt)) || !(addrNode = virXPathNode("./source/address", ctxt)) || virPCIDeviceAddressParseXML(addrNode, &addr) < 0) { - vshError(ctl, "Unable to query NVMe disk address"); + vshError(ctl, "%s", _("Unable to query NVMe disk address")); return false; } diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 171bc0ce17..e20888f2c2 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -7915,7 +7915,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) goto failed_stats; if (cpu >= max_id) { - vshError(ctl, "Start CPU %d is out of range (min: 0, max: %d)", + vshError(ctl, _("Start CPU %1$d is out of range (min: 0, max: %2$d)"), cpu, max_id - 1); goto cleanup; } @@ -9787,7 +9787,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) resultjson = virJSONValueFromString(result); if (returnval && !resultjson) { - vshError(ctl, "failed to parse JSON returned by qemu"); + vshError(ctl, "%s", _("failed to parse JSON returned by qemu")); return false; } } @@ -9800,7 +9800,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) if (returnval) { if (!(formatjson = virJSONValueObjectGet(resultjson, "return"))) { - vshError(ctl, "'return' member missing"); + vshError(ctl, "%s", _("'return' member missing")); return false; } } else { @@ -10859,7 +10859,7 @@ doMigrate(void *opaque) if (flags & VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES && !(flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC))) { - vshError(ctl, "'--copy-storage-synchronous-writes' requires one of '--copy-storage-all', '--copy-storage-inc'"); + vshError(ctl, "%s", _("'--copy-storage-synchronous-writes' requires one of '--copy-storage-all', '--copy-storage-inc'")); goto out; } @@ -10918,7 +10918,7 @@ doMigrate(void *opaque) g_autofree char **val = NULL; if (!(flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC))) { - vshError(ctl, "'--migrate-disks' requires one of '--copy-storage-all', '--copy-storage-inc'"); + vshError(ctl, "%s", _("'--migrate-disks' requires one of '--copy-storage-all', '--copy-storage-inc'")); goto out; } @@ -10939,7 +10939,7 @@ doMigrate(void *opaque) g_autofree char **val = NULL; if (!(flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC))) { - vshError(ctl, "'--migrate-disks-detect-zeroes' requires one of '--copy-storage-all', '--copy-storage-inc'"); + vshError(ctl, "%s", _("'--migrate-disks-detect-zeroes' requires one of '--copy-storage-all', '--copy-storage-inc'")); goto out; } diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 418bd995da..1b992e70f6 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -455,7 +455,7 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd) if (rv == 0) { vshError(ctl, - "Could not get count of free %uKiB pages, no data returned", + _("Could not get count of free %1$uKiB pages, no data returned"), *pagesize); goto cleanup; } diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index e759b9f629..7ae6127513 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -129,7 +129,7 @@ vshFindNodeDevice(vshControl *ctl, const char *value) } if (!dev) { - vshError(ctl, "%s '%s'", _("Could not find matching device"), value); + vshError(ctl, _("Could not find matching device '%1$s'"), value); return NULL; } @@ -969,8 +969,8 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd) if (device_value) { if (!(dev = virNodeDeviceLookupByName(priv->conn, device_value))) { - vshError(ctl, "%s '%s'", - _("Could not find matching device"), device_value); + vshError(ctl, _("Could not find matching device '%1$s'"), + device_value); goto cleanup; } } diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index 7c81c10845..3a4160107b 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -320,7 +320,7 @@ cmdSecretGetValue(vshControl *ctl, const vshCmd *cmd) if (plain) { if (fwrite(value, 1, value_size, stdout) != value_size) { virSecureErase(value, value_size); - vshError(ctl, "failed to write secret"); + vshError(ctl, "%s", _("failed to write secret")); return false; } } else { diff --git a/tools/virsh.c b/tools/virsh.c index 6abae3dae9..090fdb2017 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -657,8 +657,8 @@ virshParseArgv(vshControl *ctl, int argc, char **argv) break; case 'd': if (virStrToLong_i(optarg, NULL, 10, &debug) < 0) { - vshError(ctl, _("option %1$s takes a numeric argument"), - longindex == -1 ? "-d" : "--debug"); + const char *optStr = longindex == -1 ? "-d" : "--debug"; + vshError(ctl, _("option %1$s takes a numeric argument"), optStr); exit(EXIT_FAILURE); } if (debug < VSH_ERR_DEBUG || debug > VSH_ERR_ERROR) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 16c9b2173a..fe2868d379 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1271,8 +1271,8 @@ vshAdmParseArgv(vshControl *ctl, int argc, char **argv) break; case 'd': if (virStrToLong_i(optarg, NULL, 10, &debug) < 0) { - vshError(ctl, _("option %1$s takes a numeric argument"), - longindex == -1 ? "-d" : "--debug"); + const char *optStr = longindex == -1 ? "-d" : "--debug"; + vshError(ctl, _("option %1$s takes a numeric argument"), optStr); exit(EXIT_FAILURE); } if (debug < VSH_ERR_DEBUG || debug > VSH_ERR_ERROR) diff --git a/tools/vsh.c b/tools/vsh.c index fb00cc40a6..fd6156ea76 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -280,34 +280,34 @@ vshCmddefCheckInternals(vshControl *ctl, const vshCmdDef *alias; if (!(alias = vshCmddefSearch(cmd->alias))) { - vshError(ctl, "command alias '%s' is pointing to a non-existent command '%s'", + vshError(ctl, _("command alias '%1$s' is pointing to a non-existent command '%2$s'"), cmd->name, cmd->alias); return -1; } if (alias->alias) { - vshError(ctl, "command alias '%s' is pointing to another command alias '%s'", + vshError(ctl, _("command alias '%1$s' is pointing to another command alias '%2$s'"), cmd->name, cmd->alias); return -1; } if (cmd->handler) { - vshError(ctl, "command '%s' has handler set", cmd->name); + vshError(ctl, _("command '%1$s' has handler set"), cmd->name); return -1; } if (cmd->opts) { - vshError(ctl, "command '%s' has options set", cmd->name); + vshError(ctl, _("command '%1$s' has options set"), cmd->name); return -1; } if (cmd->info) { - vshError(ctl, "command '%s' has info set", cmd->name); + vshError(ctl, _("command '%1$s' has info set"), cmd->name); return -1; } if (cmd->flags != 0) { - vshError(ctl, "command '%s' has multiple flags set", cmd->name); + vshError(ctl, _("command '%1$s' has multiple flags set"), cmd->name); return -1; } @@ -317,7 +317,7 @@ vshCmddefCheckInternals(vshControl *ctl, /* Each command has to provide a non-empty help string. */ if (!cmd->info || !cmd->info->help || !*cmd->info->help) { - vshError(ctl, "command '%s' lacks help", cmd->name); + vshError(ctl, _("command '%1$s' lacks help"), cmd->name); return -1; } @@ -348,7 +348,7 @@ vshCmddefCheckInternals(vshControl *ctl, /* allow at most one optional positional option */ if (opt->positional && !opt->required) { if (seenOptionalPositionalOption) { - vshError(ctl, "multiple optional positional arguments (%s, %s) of command '%s' are not allowed", + vshError(ctl, _("multiple optional positional arguments (%1$s, %2$s) of command '%3$s' are not allowed"), seenOptionalPositionalOption, opt->name, cmd->name); return -1; } @@ -358,45 +358,45 @@ vshCmddefCheckInternals(vshControl *ctl, /* all optional positional arguments must be defined after the required ones */ if (seenOptionalPositionalOption && opt->positional && opt->required) { - vshError(ctl, "required positional argument '%s' declared after an optional positional argument '%s' of command '%s'", + vshError(ctl, _("required positional argument '%1$s' declared after an optional positional argument '%2$s' of command '%3$s'"), opt->name, seenOptionalPositionalOption, cmd->name); return -1; } /* Mandate no completer flags if no completer is specified */ if (opt->completer_flags != 0 && !opt->completer) { - vshError(ctl, "completer_flags of argument '%s' of command '%s' must be 0 if no completer is used", + vshError(ctl, _("completer_flags of argument '%1$s' of command '%2$s' must be 0 if no completer is used"), opt->name, cmd->name); return -1; } if (opt->unwanted_positional && opt->positional) { - vshError(ctl, "unwanted_positional flag of argument '%s' of command '%s' must not be used together with positional", + vshError(ctl, _("unwanted_positional flag of argument '%1$s' of command '%2$s' must not be used together with positional"), opt->name, cmd->name); return -1; } switch (opt->type) { case VSH_OT_NONE: - vshError(ctl, "invalid type 'NONE' of option '%s' of command '%s'", + vshError(ctl, _("invalid type 'NONE' of option '%1$s' of command '%2$s'"), opt->name, cmd->name); return -1; case VSH_OT_BOOL: if (opt->completer) { - vshError(ctl, "bool parameter '%s' of command '%s' has completer set", + vshError(ctl, _("bool parameter '%1$s' of command '%2$s' has completer set"), opt->name, cmd->name); return -1; } if (opt->positional || opt->unwanted_positional) { - vshError(ctl, "boolean parameter '%s' of command '%s' must not be positional", + vshError(ctl, _("boolean parameter '%1$s' of command '%2$s' must not be positional"), opt->name, cmd->name); return -1; } if (opt->required) { - vshError(ctl, "parameter '%s' of command '%s' misused 'required' flag", + vshError(ctl, _("parameter '%1$s' of command '%2$s' misused 'required' flag"), opt->name, cmd->name); return -1; /* bool can't be mandatory */ } @@ -413,7 +413,7 @@ vshCmddefCheckInternals(vshControl *ctl, opt->unwanted_positional || opt->completer || !opt->help) { - vshError(ctl, "parameter '%s' of command '%s' has incorrect alias option", + vshError(ctl, _("parameter '%1$s' of command '%2$s' has incorrect alias option"), opt->name, cmd->name); return -1; } @@ -429,13 +429,13 @@ vshCmddefCheckInternals(vshControl *ctl, if (p) { /* If alias comes with value, replacement must not be bool */ if (cmd->opts[j].type == VSH_OT_BOOL) { - vshError(ctl, "alias '%s' of command '%s' has mismatched alias type", + vshError(ctl, _("alias '%1$s' of command '%2$s' has mismatched alias type"), opt->name, cmd->name); return -1; } } if (!cmd->opts[j].name) { - vshError(ctl, "alias '%s' of command '%s' has missing alias option", + vshError(ctl, _("alias '%1$s' of command '%2$s' has missing alias option"), opt->name, cmd->name); return -1; } @@ -444,7 +444,7 @@ vshCmddefCheckInternals(vshControl *ctl, case VSH_OT_ARGV: if (cmd->opts[i + 1].name) { - vshError(ctl, "parameter '%s' of command '%s' must be listed last", + vshError(ctl, _("parameter '%1$s' of command '%2$s' must be listed last"), opt->name, cmd->name); return -1; } @@ -453,7 +453,7 @@ vshCmddefCheckInternals(vshControl *ctl, case VSH_OT_INT: case VSH_OT_STRING: if (opt->positional && seenOptionalOption) { - vshError(ctl, "parameter '%s' of command '%s' must be listed before optional parameters", + vshError(ctl, _("parameter '%1$s' of command '%2$s' must be listed before optional parameters"), opt->name, cmd->name); return -1; } @@ -2424,9 +2424,13 @@ vshCloseLogFile(vshControl *ctl) { /* log file close */ if (VIR_CLOSE(ctl->log_fd) < 0) { - vshError(ctl, _("%1$s: failed to write log file: %2$s"), - ctl->logfile ? ctl->logfile : "?", - g_strerror(errno)); + if (ctl->logfile) { + vshError(ctl, _("%1$s: failed to write log file: %2$s"), + ctl->logfile, g_strerror(errno)); + } else { + vshError(ctl, _("failed to write log file: %1$s"), + g_strerror(errno)); + } } g_clear_pointer(&ctl->logfile, g_free); -- 2.48.1
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh-domain-monitor.c | 8 ++-- tools/virsh-domain.c | 72 ++++++++++++++++++------------------ tools/virsh-host.c | 2 +- tools/virsh-secret.c | 4 +- tools/virsh-snapshot.c | 2 +- tools/virsh-util.c | 8 ++-- tools/virsh-volume.c | 4 +- tools/virsh.c | 2 +- tools/virt-admin.c | 10 ++--- 9 files changed, 56 insertions(+), 56 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 150e6ec806..fa1c05ac77 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -783,7 +783,7 @@ cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd) iface); if ((ninterfaces = virXPathNodeSet(xpath, ctxt, &interfaces)) < 0) { - vshError(ctl, _("Failed to extract interface information")); + vshError(ctl, "%s", _("Failed to extract interface information")); return false; } @@ -795,7 +795,7 @@ cmdDomIfGetLink(vshControl *ctl, const vshCmd *cmd) return false; } else if (ninterfaces > 1) { - vshError(ctl, _("multiple matching interfaces found")); + vshError(ctl, "%s", _("multiple matching interfaces found")); return false; } @@ -1386,7 +1386,7 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd) if (doSet || now || rtcSync) { if (now && ((seconds = time(NULL)) == (time_t) -1)) { - vshError(ctl, _("Unable to get current time")); + vshError(ctl, "%s", _("Unable to get current time")); return false; } @@ -2288,7 +2288,7 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) return false; if ((ifaces_count = virDomainInterfaceAddresses(dom, &ifaces, source, 0)) < 0) { - vshError(ctl, _("Failed to query for interfaces addresses")); + vshError(ctl, "%s", _("Failed to query for interfaces addresses")); goto cleanup; } diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index e20888f2c2..2db88a1a7d 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -540,7 +540,7 @@ cmdAttachDiskFormatAddress(vshControl *ctl, struct virshAddress diskAddr; if (virshAddressParse(straddr, multifunction, &diskAddr) < 0) { - vshError(ctl, _("Invalid address.")); + vshError(ctl, "%s", _("Invalid address.")); return -1; } @@ -689,7 +689,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) } if ((type == VIRSH_ATTACH_DISK_SOURCE_TYPE_NETWORK) != !!source_protocol) { - vshError(ctl, _("--source-protocol option requires --sourcetype network")); + vshError(ctl, "%s", _("--source-protocol option requires --sourcetype network")); return false; } @@ -992,7 +992,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) if (virshParseRateStr(ctl, inboundStr, &inbound) < 0) return false; if (!inbound.average && !inbound.floor) { - vshError(ctl, _("either inbound average or floor is mandatory")); + vshError(ctl, "%s", _("either inbound average or floor is mandatory")); return false; } } @@ -1000,11 +1000,11 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) if (virshParseRateStr(ctl, outboundStr, &outbound) < 0) return false; if (outbound.average == 0) { - vshError(ctl, _("outbound average is mandatory")); + vshError(ctl, "%s", _("outbound average is mandatory")); return false; } if (outbound.floor) { - vshError(ctl, _("outbound floor is unsupported yet")); + vshError(ctl, "%s", _("outbound floor is unsupported yet")); return false; } } @@ -1048,7 +1048,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) case VIR_DOMAIN_NET_TYPE_VHOSTUSER: if (sourceMode < 0) { - vshError(ctl, _("source-mode is mandatory")); + vshError(ctl, "%s", _("source-mode is mandatory")); return false; } virBufferAsprintf(&buf, "<source type='unix' path='%s' mode='%s'/>\n", @@ -3144,7 +3144,7 @@ cmdDomIfSetLink(vshControl *ctl, const vshCmd *cmd) return false; if ((nnodes = virXPathNodeSet("/domain/devices/interface", ctxt, &nodes)) <= 0) { - vshError(ctl, _("Failed to extract interface information or no interfaces found")); + vshError(ctl, "%s", _("Failed to extract interface information or no interfaces found")); return false; } @@ -3177,7 +3177,7 @@ cmdDomIfSetLink(vshControl *ctl, const vshCmd *cmd) /* try to find <link> element or create new one */ if (!(linkNode = virXPathNode("./link", ctxt))) { if (!(linkNode = xmlNewChild(ifaceNode, NULL, BAD_CAST "link", NULL))) { - vshError(ctl, _("failed to create XML node")); + vshError(ctl, "%s", _("failed to create XML node")); return false; } } @@ -3188,13 +3188,13 @@ cmdDomIfSetLink(vshControl *ctl, const vshCmd *cmd) stateAttr = xmlNewProp(linkNode, BAD_CAST "state", BAD_CAST state); if (!stateAttr) { - vshError(ctl, _("Failed to create or modify the state XML attribute")); + vshError(ctl, "%s", _("Failed to create or modify the state XML attribute")); return false; } if (!(xml_buf = virXMLNodeToString(xml, ifaceNode))) { vshSaveLibvirtError(); - vshError(ctl, _("Failed to create XML")); + vshError(ctl, "%s", _("Failed to create XML")); return false; } @@ -3204,7 +3204,7 @@ cmdDomIfSetLink(vshControl *ctl, const vshCmd *cmd) } if (virDomainUpdateDeviceFlags(dom, xml_buf, flags) < 0) { - vshError(ctl, _("Failed to update interface link state")); + vshError(ctl, "%s", _("Failed to update interface link state")); return false; } @@ -3297,7 +3297,7 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) if ((!inbound.average && (inbound.burst || inbound.peak)) && !inbound.floor) { - vshError(ctl, _("either inbound average or floor is mandatory")); + vshError(ctl, "%s", _("either inbound average or floor is mandatory")); goto cleanup; } @@ -3335,12 +3335,12 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) goto cleanup; } if (outbound.average == 0 && (outbound.burst || outbound.peak)) { - vshError(ctl, _("outbound average is mandatory")); + vshError(ctl, "%s", _("outbound average is mandatory")); goto cleanup; } if (outbound.floor) { - vshError(ctl, _("outbound floor is unsupported yet")); + vshError(ctl, "%s", _("outbound floor is unsupported yet")); goto cleanup; } @@ -3659,7 +3659,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) ignore_value(vshCommandOptStringQuiet(ctl, cmd, "storage", &vol_string)); if (!(vol_string || remove_all_storage) && wipe_storage) { - vshError(ctl, + vshError(ctl, "%s", _("'--wipe-storage' requires '--storage <string>' or '--remove-all-storage'")); return false; } @@ -3744,13 +3744,13 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) /* Stash domain description for later use */ if (vol_string || remove_all_storage) { if (running) { - vshError(ctl, + vshError(ctl, "%s", _("Storage volume deletion is supported only on stopped domains")); goto cleanup; } if (vol_string && remove_all_storage) { - vshError(ctl, + vshError(ctl, "%s", _("Specified both --storage and --remove-all-storage")); goto cleanup; } @@ -3923,7 +3923,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) vols[i].target, vols[i].source); fflush(stdout); if (virStorageVolWipe(vols[i].vol, 0) < 0) { - vshError(ctl, _("Failed! Volume not removed.")); + vshError(ctl, "%s", _("Failed! Volume not removed.")); ret = false; continue; } else { @@ -7206,7 +7206,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) return false; if (count == 0) { - vshError(ctl, _("Can't set 0 processors for a VM")); + vshError(ctl, "%s", _("Can't set 0 processors for a VM")); return false; } @@ -7272,7 +7272,7 @@ cmdGuestvcpus(vshControl *ctl, const vshCmd *cmd) return false; if (cpulist && !(enable || disable)) { - vshError(ctl, _("One of options --enable or --disable is required by option --cpulist")); + vshError(ctl, "%s", _("One of options --enable or --disable is required by option --cpulist")); return false; } @@ -7470,7 +7470,7 @@ cmdIOThreadInfo(vshControl *ctl, const vshCmd *cmd) return false; if ((rc = virDomainGetIOThreadInfo(dom, &info, flags)) < 0) { - vshError(ctl, _("Unable to get domain IOThreads information")); + vshError(ctl, "%s", _("Unable to get domain IOThreads information")); goto cleanup; } niothreads = rc; @@ -7751,7 +7751,7 @@ cmdIOThreadSet(vshControl *ctl, const vshCmd *cmd) return false; if (npar == 0) { - vshError(ctl, _("Not enough arguments passed, nothing to set")); + vshError(ctl, "%s", _("Not enough arguments passed, nothing to set")); return false; } @@ -8593,7 +8593,7 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd) for (opt = vshCommandOptArgv(cmd, "keycode"); opt && *opt; opt++) { if (count == VIR_DOMAIN_SEND_KEY_MAX_KEYS) { - vshError(ctl, _("too many keycodes")); + vshError(ctl, "%s", _("too many keycodes")); return false; } @@ -8922,10 +8922,10 @@ virshGetUpdatedMemoryXML(char **updatedMemoryXML, vshSaveLibvirtError(); return -1; } else if (nmems == 0) { - vshError(ctl, _("no memory device found")); + vshError(ctl, "%s", _("no memory device found")); return -1; } else if (nmems > 1) { - vshError(ctl, _("multiple memory devices found, use --alias or --node to select one")); + vshError(ctl, "%s", _("multiple memory devices found, use --alias or --node to select one")); return -1; } @@ -8944,7 +8944,7 @@ virshGetUpdatedMemoryXML(char **updatedMemoryXML, requestedSizeNode = virXPathNode("./target/requested", ctxt); if (!requestedSizeNode) { - vshError(ctl, _("virtio-mem device is missing <requested/>")); + vshError(ctl, "%s", _("virtio-mem device is missing <requested/>")); return -1; } @@ -11730,7 +11730,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) return false; if (!virDomainIsActive(dom)) { - vshError(ctl, _("Domain is not running")); + vshError(ctl, "%s", _("Domain is not running")); return false; } @@ -11767,7 +11767,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) if (type) vshError(ctl, _("No graphical display with type '%1$s' found"), type); else - vshError(ctl, _("No graphical display found")); + vshError(ctl, "%s", _("No graphical display found")); } return ret; @@ -11800,7 +11800,7 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd) /* Check if the domain is active and don't rely on -1 for this */ if (!virDomainIsActive(dom)) { - vshError(ctl, _("Domain is not running")); + vshError(ctl, "%s", _("Domain is not running")); return false; } @@ -11810,7 +11810,7 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd) /* Get the VNC port */ if (virXPathInt("string(/domain/devices/graphics[@type='vnc']/@port)", ctxt, &port)) { - vshError(ctl, _("Failed to get VNC port. Is this domain using VNC?")); + vshError(ctl, "%s", _("Failed to get VNC port. Is this domain using VNC?")); return false; } @@ -12454,7 +12454,7 @@ virshUpdateDiskXML(xmlNodePtr disk_node, source_block = false; new_source = NULL; } else if (!new_source) { - vshError(NULL, _("New disk media source was not specified")); + vshError(NULL, "%s", _("New disk media source was not specified")); return NULL; } @@ -12861,7 +12861,7 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd) return false; if (virDomainFSTrim(dom, mountPoint, minimum, flags) < 0) { - vshError(ctl, _("Unable to invoke fstrim")); + vshError(ctl, "%s", _("Unable to invoke fstrim")); return false; } @@ -12898,7 +12898,7 @@ cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd) nmountpoints = g_strv_length((GStrv) mountpoints); if ((count = virDomainFSFreeze(dom, mountpoints, nmountpoints, 0)) < 0) { - vshError(ctl, _("Unable to freeze filesystems")); + vshError(ctl, "%s", _("Unable to freeze filesystems")); return false; } @@ -12936,7 +12936,7 @@ cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd) nmountpoints = g_strv_length((GStrv) mountpoints); if ((count = virDomainFSThaw(dom, mountpoints, nmountpoints, 0)) < 0) { - vshError(ctl, _("Unable to thaw filesystems")); + vshError(ctl, "%s", _("Unable to thaw filesystems")); return false; } @@ -12970,7 +12970,7 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd) rc = virDomainGetFSInfo(dom, &info, 0); if (rc < 0) { - vshError(ctl, _("Unable to get filesystem information")); + vshError(ctl, "%s", _("Unable to get filesystem information")); goto cleanup; } ninfos = rc; @@ -13252,7 +13252,7 @@ cmdSetUserSSHKeys(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_AUTHORIZED_SSH_KEYS_SET_APPEND; if (!from) { - vshError(ctl, _("Option --file is required")); + vshError(ctl, "%s", _("Option --file is required")); return false; } } diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 1b992e70f6..9e8f542c96 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -1146,7 +1146,7 @@ vshExtractCPUDefXMLs(vshControl *ctl, xmlNodeSetName(nodes[i], (const xmlChar *)"cpu"); while (nodes[i]->properties) { if (xmlRemoveProp(nodes[i]->properties) < 0) { - vshError(ctl, + vshError(ctl, "%s", _("Cannot extract CPU definition from domain capabilities XML")); return NULL; } diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index 3a4160107b..68b14a5276 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -235,7 +235,7 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd) if (base64) { /* warn users that the --base64 option passed from command line is wrong */ - vshWarn(ctl, _("Passing secret value as command-line argument is insecure!")); + vshWarn(ctl, "%s", _("Passing secret value as command-line argument is insecure!")); secret_val = g_strdup(base64); secret_len = strlen(secret_val); } else if (filename) { @@ -257,7 +257,7 @@ cmdSecretSetValue(vshControl *ctl, const vshCmd *cmd) secret_len = strlen(secret_val); plain = true; } else { - vshError(ctl, _("Input secret value is missing")); + vshError(ctl, "%s", _("Input secret value is missing")); return false; } diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index e0be8e7582..2821cd9e26 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1140,7 +1140,7 @@ virshSnapshotListCollect(vshControl *ctl, virDomainPtr dom, if (count < 0) { if (!last_error) - vshError(ctl, _("failed to collect snapshot list")); + vshError(ctl, "%s", _("failed to collect snapshot list")); goto cleanup; } diff --git a/tools/virsh-util.c b/tools/virsh-util.c index 55ab71f26a..a4ae5be08c 100644 --- a/tools/virsh-util.c +++ b/tools/virsh-util.c @@ -383,14 +383,14 @@ virshDomainGetXMLFromDom(vshControl *ctl, g_autofree char *desc = NULL; if (!(desc = virDomainGetXMLDesc(dom, flags))) { - vshError(ctl, _("Failed to get domain description xml")); + vshError(ctl, "%s", _("Failed to get domain description xml")); return -1; } *xml = virXMLParseStringCtxt(desc, _("(domain_definition)"), ctxt); if (!(*xml)) { - vshError(ctl, _("Failed to parse domain description xml")); + vshError(ctl, "%s", _("Failed to parse domain description xml")); return -1; } @@ -408,14 +408,14 @@ virshNetworkGetXMLFromNet(vshControl *ctl, g_autofree char *desc = NULL; if (!(desc = virNetworkGetXMLDesc(net, flags))) { - vshError(ctl, _("Failed to get network description xml")); + vshError(ctl, "%s", _("Failed to get network description xml")); return -1; } *xml = virXMLParseStringCtxt(desc, _("(network_definition)"), ctxt); if (!(*xml)) { - vshError(ctl, _("Failed to parse network description xml")); + vshError(ctl, "%s", _("Failed to parse network description xml")); return -1; } diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 430961fef2..7b1847d7ae 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -676,7 +676,7 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd) flags |= VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM; if (!(st = virStreamNew(priv->conn, 0))) { - vshError(ctl, _("cannot create a new stream")); + vshError(ctl, "%s", _("cannot create a new stream")); return false; } @@ -793,7 +793,7 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd) cbData.isBlock = !!S_ISBLK(sb.st_mode); if (!(st = virStreamNew(priv->conn, 0))) { - vshError(ctl, _("cannot create a new stream")); + vshError(ctl, "%s", _("cannot create a new stream")); goto cleanup; } diff --git a/tools/virsh.c b/tools/virsh.c index 090fdb2017..800e280c7a 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -757,7 +757,7 @@ virshParseArgv(vshControl *ctl, int argc, char **argv) vshError(ctl, _("unsupported option '%1$s'. See --help."), argv[optind - 1]); exit(EXIT_FAILURE); default: - vshError(ctl, _("unknown option")); + vshError(ctl, "%s", _("unknown option")); exit(EXIT_FAILURE); } longindex = -1; diff --git a/tools/virt-admin.c b/tools/virt-admin.c index fe2868d379..2d63098444 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -975,14 +975,14 @@ cmdDaemonLogFilters(vshControl *ctl, const vshCmd *cmd) const char *filters = NULL; if ((vshCommandOptString(ctl, cmd, "filters", &filters) < 0 || virAdmConnectSetLoggingFilters(priv->conn, filters, 0) < 0)) { - vshError(ctl, _("Unable to change daemon logging settings")); + vshError(ctl, "%s", _("Unable to change daemon logging settings")); return false; } } else { g_autofree char *filters = NULL; if (virAdmConnectGetLoggingFilters(priv->conn, &filters, 0) < 0) { - vshError(ctl, _("Unable to get daemon logging filters information")); + vshError(ctl, "%s", _("Unable to get daemon logging filters information")); return false; } @@ -1024,13 +1024,13 @@ cmdDaemonLogOutputs(vshControl *ctl, const vshCmd *cmd) const char *outputs = NULL; if ((vshCommandOptString(ctl, cmd, "outputs", &outputs) < 0 || virAdmConnectSetLoggingOutputs(priv->conn, outputs, 0) < 0)) { - vshError(ctl, _("Unable to change daemon logging settings")); + vshError(ctl, "%s", _("Unable to change daemon logging settings")); return false; } } else { g_autofree char *outputs = NULL; if (virAdmConnectGetLoggingOutputs(priv->conn, &outputs, 0) < 0) { - vshError(ctl, _("Unable to get daemon logging outputs information")); + vshError(ctl, "%s", _("Unable to get daemon logging outputs information")); return false; } @@ -1320,7 +1320,7 @@ vshAdmParseArgv(vshControl *ctl, int argc, char **argv) vshError(ctl, _("unsupported option '%1$s'. See --help."), argv[optind - 1]); exit(EXIT_FAILURE); default: - vshError(ctl, _("unknown option")); + vshError(ctl, "%s", _("unknown option")); exit(EXIT_FAILURE); } longindex = -1; -- 2.48.1
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh-domain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2db88a1a7d..c8c341b3d1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2648,7 +2648,7 @@ virshBlockJobInfo(vshControl *ctl, if (!raw) { speed <<= 20; if (speed >> 20 != info.bandwidth) { - vshError(ctl, _("overflow in converting %1$ld MiB/s to bytes\n"), + vshError(ctl, _("overflow in converting %1$ld MiB/s to bytes"), info.bandwidth); return false; } @@ -3831,7 +3831,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) if (!vol.vol) { vshError(ctl, - _("Storage volume '%1$s'(%2$s) is not managed by libvirt. Remove it manually.\n"), + _("Storage volume '%1$s'(%2$s) is not managed by libvirt. Remove it manually."), target, source); vshResetLibvirtError(); continue; @@ -3848,7 +3848,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) for (i = 0; i < nvol_list; i++) { if (vol_list[i]) { vshError(ctl, - _("Volume '%1$s' was not found in domain's definition.\n"), + _("Volume '%1$s' was not found in domain's definition."), vol_list[i]); found = true; } -- 2.48.1
The prohibit_newline_at_end_of_diagnostic syntax check is confused when another unrelated translatable message with a newline is too close to the function it is supposed to check. Refactoring the code to make the two strings further apart seems like the easiest solution. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tools/virsh-domain.c | 9 +++++---- tools/virsh-snapshot.c | 10 ++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c8c341b3d1..cc5ae60536 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -12307,12 +12307,13 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) return ret; cleanup: - if (!ret) { - vshError(ctl, "%s", _("Failed to detach interface")); - } else { + if (ret) { vshPrintExtra(ctl, "%s", _("Interface detached successfully\n")); + return true; } - return ret; + + vshError(ctl, "%s", _("Failed to detach interface")); + return false; } diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 2821cd9e26..dbd849bb3b 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1758,11 +1758,13 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) result = virDomainRevertToSnapshot(snapshot, flags); } - if (result < 0) + if (result < 0) { vshError(ctl, _("Failed to revert snapshot %1$s"), name); - else - vshPrintExtra(ctl, _("Domain snapshot %1$s reverted\n"), name); - return result >= 0; + return false; + } + + vshPrintExtra(ctl, _("Domain snapshot %1$s reverted\n"), name); + return true; } /* -- 2.48.1
To make sure both error and warning messages printed by virsh are properly marked for translation. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- build-aux/syntax-check.mk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index bd3dd6cb54..d414e033df 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -401,6 +401,8 @@ msg_gen_function += virReportError msg_gen_function += virReportErrorHelper msg_gen_function += virReportSystemError msg_gen_function += virLastErrorPrefixMessage +msg_gen_function += vshError +msg_gen_function += vshWarn # Uncomment the following and run "ninja test" to see diagnostics # that are not yet marked for translation, but that need to be rewritten @@ -408,7 +410,6 @@ msg_gen_function += virLastErrorPrefixMessage # msg_gen_function += fprintf # msg_gen_function += testError # msg_gen_function += vshPrint -# msg_gen_function += vshError space = $(null) $(null) func_re= ($(subst $(space),|,$(msg_gen_function))) -- 2.48.1
On 2/20/25 16:26, Jiri Denemark wrote:
This series originally started as a documentation update for hypervisor-cpu-compare and hypervisor-cpu-baseline commands with an idea to print a warning if the commands are used suboptimally. The rest is a refactor and a lot of fixes of (mostly) error messages printed by virsh.
Jiri Denemark (13): docs: Clarify documentation of virsh hypervisor-cpu-compare docs: Clarify documentation of virsh hypervisor-cpu-baseline virsh: Do not format messages twice virsh: Make messages printed by vshError properly translatable virsh: Refactor vshError virsh: Introduce vshWarn virsh: Warn when hypervisor-cpu-* is used with host CPU virsh: Do not require \n in vshDebug messages virsh: Properly mark all error messages for translation virsh: Avoid using translated messages without format virsh: Drop extra newlines at the end of error messages virsh: Let prohibit_newline_at_end_of_diagnostic check pass build: Enable syntax checks for vshError and vshWarn
build-aux/syntax-check.mk | 3 +- docs/manpages/virsh.rst | 47 ++++++++----- tools/virsh-domain-monitor.c | 16 ++--- tools/virsh-domain.c | 131 ++++++++++++++++++----------------- tools/virsh-host.c | 22 ++++-- tools/virsh-interface.c | 6 +- tools/virsh-network.c | 10 +-- tools/virsh-nodedev.c | 6 +- tools/virsh-nwfilter.c | 10 +-- tools/virsh-pool.c | 10 +-- tools/virsh-secret.c | 8 +-- tools/virsh-snapshot.c | 12 ++-- tools/virsh-util.c | 16 ++--- tools/virsh-volume.c | 24 +++---- tools/virsh.c | 10 +-- tools/virt-admin.c | 16 ++--- tools/vsh.c | 113 +++++++++++++++++------------- tools/vsh.h | 6 +- 18 files changed, 254 insertions(+), 212 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Jiri Denemark -
Michal Prívozník