Re: [PATCH] Add query-tdx-capabilities

[Markus Armbruster <armbru@redhat.com>]

Cc: machine core maintainers for an opinion on query-machines.

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Fri, Jan 9, 2026 at 4:27 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>> > On Fri, Jan 09, 2026 at 11:29:47AM +0100, Markus Armbruster wrote:
>> >> Daniel P. Berrangé <berrange@redhat.com> writes:
>> >>
>> >> > On Fri, Jan 09, 2026 at 11:01:27AM +0100, Markus Armbruster wrote:
>> >> >> Daniel P. Berrangé <berrange@redhat.com> writes:
>> >> >>
>> >> >> > On Fri, Jan 09, 2026 at 10:30:32AM +0100, Markus Armbruster wrote:
>> >> >> >> Daniel P. Berrangé <berrange@redhat.com> writes:
>> >> >> >>
>> >> >> >> > On Tue, Jan 06, 2026 at 10:36:20PM +0400, marcandre.lureau@redhat.com wrote:
>> >> >> >> >> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >> >> >> >>
>> >> >> >> >> Return an empty TdxCapability struct, for extensibility and matching
>> >> >> >> >> query-sev-capabilities return type.
>> >> >> >> >>
>> >> >> >> >> Fixes: https://issues.redhat.com/browse/RHEL-129674
>> >> >> >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

[...]

>> >> >> Do management applications need to know more than "this combination of
>> >> >> host + KVM + QEMU can do SEV, yes / no?
>> >> >>
>> >> >> If yes, what do they need?  "No" split up into serval "No, because X"?
>> >> >
>> >> > When libvirt runs  query-sev-capabilities it does not care about the
>> >> > reason for it being unsupported.   Any "GenericError" is considered
>> >> > to mark the lack of host support, and no fine grained checks are
>> >> > performed on the err msg.
>> >> >
>> >> > If query-sev-capabilities succeeds (indicating SEV is supported), then
>> >> > all the returned info is exposed to mgmt apps in the libvirt domain
>> >> > capabilities XML document.
>> >>
>> >> So query-sev-capabilities is good enough as is?
>> >
>> > IIUC, essentially all QEMU errors that could possibly be seen with
>> > query-sev-capabilities are "GenericError" these days, except for
>> > the small possibility of "CommandNotFound".
>> >
>> > The two scenarios with lack of SEV support are covered by GenericError
>> > but I'm concerned that other things that should be considered fatal
>> > will also fall under GenericError.
>> >
>> > eg take a look at qmp_dispatch() and see countless places where we can
>> > return GenericError which ought to be treated as fatal by callers.
>> >
>> > IMHO  "SEV not supported" is not conceptually an error, it is an
>> > expected informational result of query-sev-capabilities, and thus
>> > shouldn't be using the QMP error object, it should have been a
>> > boolean result field.
>>
>> I agree that errors should be used only for "abnormal" outcomes, not for
>> the "no" answer to a simple question like "is SEV available, and if yes,
>> what are its capabilities?"
>>
>> I further agree that encoding "no" as GenericError runs the risk of
>> conflating "no" with other errors.  Since query-sev itself can fail just
>> one way, these can only come from the QMP core.  For the core's syntax
>> and type errors, the risk is only theoretical: just don't do that.
>> Errors triggered by state, like the one in qmp_command_available(), are
>> a bit more worrying.  I think they're easy enough to avoid if you're
>> aware, but "if you're aware" is admittedly rittle.
>>
>> Anyway, that's what we have.  Badly designed, but it seems to be
>> workable.
>>
>> Is the bad enough to justify revising the interface?  I can't see how to
>> do that compatibly.
>>
>> Is it bad enough to justify new interfaces for similar things to be
>> dissimilar?
>>
>
> Maybe query-{sev,tdx,*}-capabilities should only be called when the
> host is actually capable, thus throwing an Error is fine.
>
> What about a new "query-confidential-guest-supports" command that
> checks the host capability and returns ["sev", "tdx", "pef"...] then ?

Some similarity to query-accelerators.  Feels reasonable.

> Or maybe this should be provided at the MachineInfo level instead
> (query-machines).

Also reasonable, I think.  Machine core maintainers, got an opinion?