RE: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS) support

-----Original Message----- From: Daniel P. Berrangé <berrange(a)redhat.com&gt; Subject: Re: [PATCH rfcv4 08/13] Add Intel TDX Quote Generation Service(QGS) support On Fri, May 24, 2024 at 02:21:23PM +0800, Zhenzhong Duan wrote: > Add element "quoteGenerationService" to tdx launch security type. > Currently it contains only one sub-element "SocketAddress". > > "SocketAddress" is modelized according to QEMU QAPI, supporting > inet, unix, vsock and fd type and variant attributes depending > on type. > > XML example: > > <launchSecurity type='tdx'> > <policy>0x0</policy> > <mrConfigId>xxx</mrConfigId> > <mrOwner>xxx</mrOwner> > <mrOwnerConfig>xxx</mrOwnerConfig> > <quoteGenerationService> > <SocketAddress type='vsock' cid='xxx' port='xxx'/> Libvirt doesn't usually have initial capitals in any XML elements/attrs. I think everything from <SocketAddress> could be put on the <quoteGenerationService> element directly.

> </quoteGenerationService> > </launchSecurity> > > QEMU command line example: > qemu-system-x86_64 \ > -object '{"qom-type":"tdx-guest","id":"lsec0","sept-ve- disable":false,"mrconfigid":"xxx","mrowner":"xxx","mrownerconfig":"xxx","q uote-generation-socket":{"type":"vsock","cid":"xxx","port":"xxx"}}' \ > -machine pc-q35-6.0,confidential-guest-support=lsec0 > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com&gt; > --- > src/conf/domain_conf.c | 272 +++++++++++++++++++++++++++++- > src/conf/domain_conf.h | 61 +++++++ > src/conf/schemas/domaincommon.rng | 106 ++++++++++++ > src/qemu/qemu_command.c | 106 ++++++++++++ > 4 files changed, 544 insertions(+), 1 deletion(-) > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index bb4973fce8..15cdb3e0e6 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -2852,6 +2852,55 @@ struct _virDomainKeyWrapDef { > virTristateSwitch dea; > }; > > +typedef enum { > + VIR_DOMAIN_SOCKET_ADDRESS_NONE, > + VIR_DOMAIN_SOCKET_ADDRESS_INET, > + VIR_DOMAIN_SOCKET_ADDRESS_UNIX, > + VIR_DOMAIN_SOCKET_ADDRESS_VSOCK, > + VIR_DOMAIN_SOCKET_ADDRESS_FD, > + > + VIR_DOMAIN_SOCKET_ADDRESS_LAST > +} virDomainSocketAddress; > + > +typedef struct _InetSocketAddress InetSocketAddress; > +typedef struct _UnixSocketAddress UnixSocketAddress; > +typedef struct _VsockSocketAddress VsockSocketAddress; > +typedef struct _FdSocketAddress FdSocketAddress; > + > +struct _InetSocketAddress { > + char *host; > + char *port; > + bool has_numeric; > + virTristateBool numeric; > + bool has_to; > + unsigned int to; > + bool has_ipv4; > + virTristateBool ipv4; > + bool has_ipv6; > + virTristateBool ipv6; > + bool has_keep_alive; > + virTristateBool keep_alive; > + bool has_mptcp; > + virTristateBool mptcp; > +}; > + > +struct _UnixSocketAddress { > + char *path; > + bool has_abstract; > + virTristateBool abstract; > + bool has_tight; > + virTristateBool tight; > +}; All of these "has_XXX" fields are redundant. Only 'has_to' is ever set, and it is never read after that, so that's a dead store.