[PATCH] virsh: Allow listing just domain IDs

Peter Krempa pkrempa at redhat.com
Tue Oct 20 13:01:39 UTC 2020


On Tue, Oct 20, 2020 at 12:27:27 +0200, Michal Privoznik wrote:
> Some completers for libvirt related tools might want to list
> domain IDs only. Just like the one I've implemented for
> virt-viewer [1]. I've worked around it using some awk magic,
> but if it was possible to just 'virsh list --id' then I could
> drop awk.
> 
> 1: https://www.redhat.com/archives/virt-tools-list/2019-May/msg00014.html
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> 
> I just realized that I've never sent this and it was sitting in a
> branch for 1.5 years. <facepalm/>
> 
>  tools/virsh-domain-monitor.c | 41 ++++++++++++++++++++++++------------

Missing change to docs/manpages/virsh.rst

>  1 file changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index e0491d48ac..9e66d573e6 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c

[...]

> @@ -1988,8 +1993,12 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
>  
>      VSH_EXCLUSIVE_OPTIONS("table", "name");
>      VSH_EXCLUSIVE_OPTIONS("table", "uuid");
> +    VSH_EXCLUSIVE_OPTIONS("table", "id");
> +    VSH_EXCLUSIVE_OPTIONS("all", "id");

This limits the usefulness. Any client wishing to use it would
specifically need to query just live VMs with this.

For completing something which accepts both active and inactive VM
identifiers, using this pattern would motivate callers do two calls to
get the list of VMs with ID and one without that argument, which is
inherently wrong/racy.

A solution would be that if --all --id and at least one of [--name,
--uuid] is used, id '-' is used for any inactive VM. Obviously --all
--id would print only active VMs and no '-'s

> +    VSH_EXCLUSIVE_OPTIONS("inactive", "id");
> +    VSH_EXCLUSIVE_OPTIONS("state-shutoff", "id");
>  
> -    if (!optUUID && !optName)
> +    if (!optUUID && !optName && !optID)
>          optTable = true;
>  

The rest of the code looks good.




More information about the libvir-list mailing list