On 3/19/25 4:15 AM, Peter Krempa via Devel wrote:
> 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.
Thanks for the detailed explanation. I merely copied a the emulator
parameter from the other virsh hypervisor-cpu-* commands, but did not
understand the significance of this option. I will omit it in v2.
--
Regards,
Collin
