On Mon, Jul 23, 2012 at 1:51 PM, Martin Kletzander <mkletzan@xxxxxxxxxx> wrote:
> The 'domdisplay' command didn't properly evaluate '--include-password'
> option.
> ---
> tools/virsh.c | 35 +++++++++++++++++++++++------------
> 1 files changed, 23 insertions(+), 12 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 5888d6c..e0765ba 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -66,6 +66,7 @@
> #include "virtypedparam.h"
> #include "intprops.h"
> #include "conf/virdomainlist.h"
> +#include "datatypes.h"
Why exactly is this necessary? No new types are being used that aren't
used throughout this whole file.
>
> static char *progname;
>
> @@ -13882,7 +13883,16 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> - doc = virDomainGetXMLDesc(dom, 0);
> + if (!vshCommandOptBool(cmd, "include-password"))
> + doc = virDomainGetXMLDesc(dom, 0);
> + else {
> + if (ctl->conn->flags & VIR_DOMAIN_XML_SECURE) {
> + vshError(ctl, _("Cannot get password with read-only connection"));
> + goto cleanup;
> + }
> + doc = virDomainGetXMLDesc(dom, VIR_DOMAIN_XML_SECURE);
> + }
> +
I would do like all the other commands and define unsigned int flags =
0; at the top and if include-password is set just do flags |=
VIR_DOMAIN_XML_SECURE; and always call virDomainGetXMLDesc() with
flags passed. Look at cmdSnapshotCurrent for an example.
> if (!doc)
> goto cleanup;
>
> @@ -13944,19 +13954,20 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd)
> if (tmp)
> tls_port = 0;
>
> - if (vshCommandOptBool(cmd, "daemon")) {
> - /* Create our XPATH lookup for the SPICE password */
> - virAsprintf(&xpath, "string(/domain/devices/graphics"
> + /* Create our XPATH lookup for the SPICE password */
> + virAsprintf(&xpath, "string(/domain/devices/graphics"
> "[@type='%s']/@passwd)", scheme[iter]);
> - if (!xpath) {
> - virReportOOMError();
> - goto cleanup;
> - }
> -
> - /* Attempt to get the SPICE password */
> - passwd = virXPathString(xpath, ctxt);
> - VIR_FREE(xpath);
> + if (!xpath) {
> + virReportOOMError();
> + goto cleanup;
> }
> +
> + /* Attempt to get the SPICE password
> + *
> + * This will return NULL automatically if the
> + * virDomainGetXMLDesc wasn't secure */
> + passwd = virXPathString(xpath, ctxt);
> + VIR_FREE(xpath);
> }
>
> /* Build up the full URI, starting with the scheme */
> --
> 1.7.8.6
>
Getting rid of the conditional on include-password (which was
incorrectly stated as 'daemon') doesn't seem correct. Leave the logic
as is and just fix 'daemon' to say 'include-password'.
--
Doug Goldstein
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list