[v1] virsh completion improvement

[libvirt] [PATCH 10/19] virsh: Only return domains that have VNC display to vncdisplay command

Posted by Lin Ma 5 years, 3 months ago

Signed-off-by: Lin Ma <lma@suse.com>
---
 tools/virsh-completer-domain.c | 3 ++-
 tools/virsh-domain.c           | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index ab81a0dcfe..305711151f 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -52,7 +52,8 @@ virshDomainNameCompleter(vshControl *ctl,
                   VIR_CONNECT_LIST_DOMAINS_SHUTOFF |
                   VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE |
                   VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT |
-                  VIR_CONNECT_LIST_DOMAINS_HAS_CHECKPOINT,
+                  VIR_CONNECT_LIST_DOMAINS_HAS_CHECKPOINT |
+                  VIR_CONNECT_LIST_DOMAINS_HAS_VNC,
                   NULL);
 
     if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 7189c8c826..5b9970fdf5 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11759,7 +11759,8 @@ static const vshCmdInfo info_vncdisplay[] = {
 };
 
 static const vshCmdOptDef opts_vncdisplay[] = {
-    VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
+    VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+                                 VIR_CONNECT_LIST_DOMAINS_HAS_VNC),
     {.name = NULL}
 };
 
-- 
2.26.0

Re: [libvirt] [PATCH 10/19] virsh: Only return domains that have VNC display to vncdisplay command

Posted by Michal Privoznik 5 years, 3 months ago

On 11/2/20 9:26 AM, Lin Ma wrote:
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
>   tools/virsh-completer-domain.c | 3 ++-
>   tools/virsh-domain.c           | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
> index ab81a0dcfe..305711151f 100644
> --- a/tools/virsh-completer-domain.c
> +++ b/tools/virsh-completer-domain.c
> @@ -52,7 +52,8 @@ virshDomainNameCompleter(vshControl *ctl,
>                     VIR_CONNECT_LIST_DOMAINS_SHUTOFF |
>                     VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE |
>                     VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT |
> -                  VIR_CONNECT_LIST_DOMAINS_HAS_CHECKPOINT,
> +                  VIR_CONNECT_LIST_DOMAINS_HAS_CHECKPOINT |
> +                  VIR_CONNECT_LIST_DOMAINS_HAS_VNC,
>                     NULL);
>   
>       if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 7189c8c826..5b9970fdf5 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -11759,7 +11759,8 @@ static const vshCmdInfo info_vncdisplay[] = {
>   };
>   
>   static const vshCmdOptDef opts_vncdisplay[] = {
> -    VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
> +    VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE |
> +                                 VIR_CONNECT_LIST_DOMAINS_HAS_VNC),
>       {.name = NULL}
>   };
>   
> 

As Peter says, this too big overkill. If we'd go the full length then we 
will need HAS_.* to cover each device/element in domain XML. I'm not in 
favor of such approach.

Michal