Re: [PATCH] Add query-tdx-capabilities

[Markus Armbruster <armbru@redhat.com>]

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>
>> >> 
>> >> [...]
>> >> 
>> >> >> > This matches the conceptual design used with query-sev-capabilities,
>> >> >> > where the lack of SEV support has to be inferred from the command
>> >> >> > returning "GenericError".
>> >> >> 
>> >> >> Such guesswork is brittle.  An interface requiring it is flawed, and
>> >> >> should be improved.
>> >> >> 
>> >> >> Our SEV interface doesn't actually require it: query-sev tells you
>> >> >> whether we have SEV.  Just run that first.
>> >> >
>> >> > Actually these commands are intended for different use cases.
>> >> >
>> >> > "query-sev" only returns info if you have launched qemu with
>> >> >
>> >> >   $QEMU -object sev-guest,id=cgs0  -machine confidential-guest-support=cgs0
>> >> >
>> >> > The goal of "query-sev-capabilities" is to allow you to determine
>> >> > if the combination of host+kvm+qemu are capable of running a guest
>> >> > with "sev-guest".
>> >> >
>> >> > IOW, query-sev-capabilities alone is what you want/need in order
>> >> > to probe host features.
>> >> >
>> >> > query-sev is for examining running guest configuration
>> >> 
>> >> The doc comments fail to explain this.  Needs fixing.
>> >> 
>> >> 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?

>> If yes, then the proposed query-tdx-capabilities should also be good
>> enough, shouldn't it?
>
> With regards,
> Daniel