Hi On Fri, Jan 9, 2026 at 4:26 PM Markus Armbruster <[email protected]> wrote: > > Daniel P. Berrangé <[email protected]> writes: > > > On Fri, Jan 09, 2026 at 11:29:47AM +0100, Markus Armbruster wrote: > >> Daniel P. Berrangé <[email protected]> writes: > >> > >> > On Fri, Jan 09, 2026 at 11:01:27AM +0100, Markus Armbruster wrote: > >> >> Daniel P. Berrangé <[email protected]> writes: > >> >> > >> >> > On Fri, Jan 09, 2026 at 10:30:32AM +0100, Markus Armbruster wrote: > >> >> >> Daniel P. Berrangé <[email protected]> writes: > >> >> >> > >> >> >> > On Tue, Jan 06, 2026 at 10:36:20PM +0400, > >> >> >> > [email protected] wrote: > >> >> >> >> From: Marc-André Lureau <[email protected]> > >> >> >> >> > >> >> >> >> 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 <[email protected]> > >> >> > >> >> [...] > >> >> > >> >> >> > 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? >
Markus, as our QAPI maintainer, you have more weight on the decision. Should the query return an union ( "unavailable": "reason..", "available": TdxCapability ) or the proposed patch is okay? > >> If yes, then the proposed query-tdx-capabilities should also be good > >> enough, shouldn't it? > > > > With regards, > > Daniel >
