Re: [PATCH] virsh: Introduce new hypervisor-cpu-models command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 18, 2025 at 18:13:57 +0100, Ján Tomko via Devel wrote:
> On a Thursday in 2025, Collin Walling wrote:
> > From: David Judkovics <djudkovi(a)linux.ibm.com>
> > 
> > Add new virsh command 'hypervisor-cpu-models'.  Command pulls from the
> > existing domcapabilities XML and uses xpath to parse CPU model strings.
> > By default, only models reported as usable by the hypervisor on the
> > host system are printed.  User may specify "--all" to also print
> > models which are not supported on the host.
> > 
> > Signed-off-by: David Judkovics <djudkovi@xxxxxxxxxxxxx>
> > Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>
> > Signed-off-by: Collin Walling <walling@xxxxxxxxxxxxx>
> > ---

[...]

> > diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> > index 9e8f542c96..2884067bbb 100644
> > --- a/tools/virsh-host.c
> > +++ b/tools/virsh-host.c
> > @@ -1766,6 +1766,74 @@ cmdHypervisorCPUBaseline(vshControl *ctl,
> > }
> > 
> > 
> > +/*
> > + * "hypervisor-cpu-models" command
> > + */
> > +static const vshCmdInfo info_hypervisor_cpu_models = {
> > +    .help = N_("Hypervisor reported CPU models"),
> > +    .desc = N_("Get the CPU models reported by the hypervisor."),
> > +};
> > +
> > +static const vshCmdOptDef opts_hypervisor_cpu_models[] = {
> > +    {.name = "virttype",
> > +     .type = VSH_OT_STRING,
> > +     .completer = virshDomainVirtTypeCompleter,
> > +     .help = N_("virtualization type (/domain/@type)"),
> > +    },
> > +    {.name = "emulator",
> > +     .type = VSH_OT_STRING,
> > +     .unwanted_positional = true,
> 
> The use of this option is discouraged, see:

I'd say forbidden. The idea of the flag was that no new code would ever
use it.

In fact in waranted cases, where the options were conflicting or new
options were added in between existing options I even changed old code
to require the options instead of filling them to unwanted fields.

> commit a67f737ddfc59b0f5b3905bd1c0935ced82de3d0
> Author:     Peter Krempa <pkrempa@xxxxxxxxxx>
> AuthorDate: 2024-03-14 17:17:19 +0100
> Commit:     Peter Krempa <pkrempa@xxxxxxxxxx>
> CommitDate: 2024-04-02 14:24:30 +0200
> 
>     virt-admin: Annodate 'unwanted_positional' arguments
> 
> https://gitlab.com/libvirt/libvirt/-/commit/a67f737ddfc59b0f5b3905bd1c0935ced82de3d0

Commits 348010ac937fd9a71d81cd3d4154dd46bdbb6a87 and
a7b10919e7d371b9e76ccec2956ba74773de6943 illustrate the problem further:

Specifically some of the arguments would be handled differently
regardless of documentation based on position.

In case of some other commands new options which were parsed this way
were added before pre-existing options thus would break how the
arguments were filled.

We do not want that to happen again.

Commands now can have at most one positional optional option, any
further optional argument must use explicit option string.




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux