[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 1/6] virsh-domain: Refactor cmdVcpucount and fix output on inactive domains

On 04/19/13 01:27, Eric Blake wrote:
On 04/16/2013 08:00 AM, Peter Krempa wrote:
This patch factors out the vCPU count retrieval including fallback means
into vshCPUCountCollect() and removes the duplicated code to retrieve
individual counts.

+#define PRINT_COUNT(VAR, MAX, STATE) if (VAR > 0) \
+    vshPrint(ctl, "%-12s %-12s %3d\n", _(MAX), _(STATE), VAR)
+        PRINT_COUNT(conf_max, "maximum", "config");
+        PRINT_COUNT(live_max, "maximum", "live");
+        PRINT_COUNT(conf_cur, "current", "config");
+        PRINT_COUNT(live_cur, "current", "live");

Doesn't quite work with i18n.  Since MAX and STATE are not string
literals, they do not appear in the .po file.  But since "maximum" isn't
marked, it also doesn't appear.  So nothing gets translated.

You either need: _(MAX) in the macro, and N_("maximum") as argument to
the macro, or plain MAX in the macro, and _("maximum") as argument to
the macro.

Also, using the name MAX as a macro parameter risks collisions with a
common macro name of MAX(); it might be better to name the macro
parameter something less likely to collide, like WHICH.

Beyond that, it looks like your conversion does the right things.  I
assume you tested on all three of transient, persistent online, and
persistent offline guests, and with a good set of combinations of
arguments to make sure we aren't breaking back-compat for which numbers
get reported.

ACK with the macro fixed up.

Thanks, I fixed the macro and double checked that the original flag meanings match the stuff reported by old virsh and pushed this patch.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]