Re: [libvirt PATCH 1/5] qemu_capabilities: detect if confidential-guest-support is available

On Fri, Jun 11, 2021 at 15:24:09 +0200, Pavel Hrdina wrote: > In libvirt we already use `query-command-line-options` QMP command but > that is useless as it doesn't provide correct data for `-machine` > option. So we need a new and better way to get that data. > > We already use `qom-list-properties` to get options for specific machine > types so we can reuse it to get options for special `none` machine type > as a generic arch independent machine type. > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; [...] > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 2b9ab9af60..436fe40f65 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -632,6 +632,9 @@ VIR_ENUM_IMPL(virQEMUCaps, > "input-linux", > "virtio-gpu-gl-pci", > "virtio-vga-gl", > + > + /* 405 */ > + "confidential-guest-support", > ); > > > @@ -1756,6 +1759,10 @@ static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsVirt[] = { > { "iommu", QEMU_CAPS_MACHINE_VIRT_IOMMU }, > }; > > +static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsGeneric[] = { > + { "confidential-guest-support", QEMU_CAPS_MACHINE_CONFIDENTAL_GUEST_SUPPORT }, If possible please include this line and all the detection that happens from it in a separate commit including the definition of the new capability flag. > +}; > + > static virQEMUCapsObjectTypeProps virQEMUCapsMachineProps[] = { > { "pseries", virQEMUCapsMachinePropsPSeries, > G_N_ELEMENTS(virQEMUCapsMachinePropsPSeries), > @@ -1763,6 +1770,9 @@ static virQEMUCapsObjectTypeProps virQEMUCapsMachineProps[] = { > { "virt", virQEMUCapsMachinePropsVirt, > G_N_ELEMENTS(virQEMUCapsMachinePropsVirt), > -1 }, > + { "none", virQEMUCapsMachinePropsGeneric, > + G_N_ELEMENTS(virQEMUCapsMachinePropsGeneric), > + -1 }, > }; > > static void > @@ -2865,8 +2875,10 @@ virQEMUCapsProbeQMPMachineProps(virQEMUCaps *qemuCaps, > const char *canon = virQEMUCapsGetCanonicalMachine(qemuCaps, virtType, props.type); > g_autofree char *type = NULL; > > - if (!virQEMUCapsIsMachineSupported(qemuCaps, virtType, canon)) > + if (STRNEQ(canon, "none") && > + !virQEMUCapsIsMachineSupported(qemuCaps, virtType, canon)) { > continue; This looks fishy since after this patch we'll ever only skip the 'none' machine, whereas previously we'd let get virQEMUCapsIsMachineSupported called.

> + } > > /* The QOM type for machine types is the machine type name > * followed by the -machine suffix */ > diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies > index 884db53152..aa8209cc42 100644 > --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies > +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.replies > @@ -17523,9 +17523,122 @@ > "id": "libvirt-36" > } > > +{ > + "execute": "qom-list-properties", > + "arguments": { > + "typename": "none-machine" > + }, > + "id": "libvirt-37" > +} As noted above, I'd prefer if the update of replies is separated from addition of the capability.