virsh: Fix XPATH in virshDomainDeviceAliasCompleter()

[PATCH] virsh: Fix XPATH in virshDomainDeviceAliasCompleter()

Posted by Michal Privoznik 3 years, 2 months ago

The way this completer works is that it dumps XML of specified
domain and then tries to look for @name attribute of <alias/>
element. However, the XPATH it uses is not correct which results
in no aliases returned by the completer.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tools/virsh-completer-domain.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 4a3459f12a..e773af6552 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -316,14 +316,14 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
     if (virshDomainGetXML(ctl, cmd, domainXMLFlags, &xmldoc, &ctxt) < 0)
         return NULL;
 
-    naliases = virXPathNodeSet("./devices//alias/@name", ctxt, &aliases);
+    naliases = virXPathNodeSet("/domain/devices//alias[@name]", ctxt, &aliases);
     if (naliases < 0)
         return NULL;
 
     tmp = g_new0(char *, naliases + 1);
 
     for (i = 0; i < naliases; i++) {
-        if (!(tmp[i] = virXMLNodeContentString(aliases[i])))
+        if (!(tmp[i] = virXMLPropString(aliases[i], "name")))
             return NULL;
     }
 
-- 
2.26.2

Re: [PATCH] virsh: Fix XPATH in virshDomainDeviceAliasCompleter()

Posted by Peter Krempa 3 years, 2 months ago

On Fri, Jan 22, 2021 at 09:39:01 +0100, Michal Privoznik wrote:
> The way this completer works is that it dumps XML of specified
> domain and then tries to look for @name attribute of <alias/>
> element. However, the XPATH it uses is not correct which results
> in no aliases returned by the completer.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tools/virsh-completer-domain.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
> index 4a3459f12a..e773af6552 100644
> --- a/tools/virsh-completer-domain.c
> +++ b/tools/virsh-completer-domain.c
> @@ -316,14 +316,14 @@ virshDomainDeviceAliasCompleter(vshControl *ctl,
>      if (virshDomainGetXML(ctl, cmd, domainXMLFlags, &xmldoc, &ctxt) < 0)
>          return NULL;
>  
> -    naliases = virXPathNodeSet("./devices//alias/@name", ctxt, &aliases);
> +    naliases = virXPathNodeSet("/domain/devices//alias[@name]", ctxt, &aliases);

Another option would be //devices/ as start, but since we know it's a
domain XML full path is okay.

>      if (naliases < 0)
>          return NULL;
>  
>      tmp = g_new0(char *, naliases + 1);
>  
>      for (i = 0; i < naliases; i++) {
> -        if (!(tmp[i] = virXMLNodeContentString(aliases[i])))
> +        if (!(tmp[i] = virXMLPropString(aliases[i], "name")))

... and I actually prefer this.

>              return NULL;
>      }

Reviewed-by: Peter Krempa <pkrempa@redhat.com>