On 22.09.2016 12:43, Chen Hanxiao wrote:

From: Chen Hanxiao <chenhanxiao@gmail.com>

For one VM, it could have more than one graphical display. Such as we coud add both vnc and spice display to a VM.

This patch introduces '--all' for showing all possible type of graphical display for a active VM.

Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- tools/virsh-domain.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)

You need to update the manpage too. Without it I cannot ACK this one.

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 3829b17..7194153 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10648,6 +10648,10 @@ static const vshCmdOptDef opts_domdisplay[] = { .help = N_("select particular graphical display " "(e.g. \"vnc\", \"spice\", \"rdp\")") }, + {.name = "all", + .type = VSH_OT_BOOL, + .help = N_("show all possible graphical displays") + }, {.name = NULL} };

@@ -10671,6 +10675,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) int tmp; int flags = 0; bool params = false; + bool all = false;

You can initialize this variable right here: bool all = vshCommandOptBool(cmd, "all"));

const char *xpath_fmt = "string(/domain/devices/graphics[@type='%s']/%s)"; virSocketAddr addr;

@@ -10685,6 +10690,9 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "include-password")) flags |= VIR_DOMAIN_XML_SECURE;

+ if (vshCommandOptBool(cmd, "all")) + all = true; + if (vshCommandOptStringReq(ctl, cmd, "type", &type) < 0) goto cleanup;

@@ -10845,7 +10853,15 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)

/* We got what we came for so return successfully */ ret = true; - break; + if (!all) { + break; + } else { + VIR_FREE(xpath); + VIR_FREE(passwd); + VIR_FREE(listen_addr); + VIR_FREE(output); + vshPrint(ctl, "\n"); + }

I'd prefer if these are spread across the loop body. That is, just before a variable set, it is prepended with VIR_FREE() call, e.g. /* Print out our full URI */ VIR_FREE(output); output = virBufferContentAndReset(&buf); And so on. I like the idea! Michal