[libvirt][PATCH v13 1/6] Define SGX capabilities structs

[Huang, Haibin]

> -----Original Message-----
> From: Michal Prívozník <mprivozn at redhat.com>
> Sent: Wednesday, July 20, 2022 7:27 PM
> To: Yang, Lin A <lin.a.yang at intel.com>; libvir-list at redhat.com; Huang, Haibin
> <haibin.huang at intel.com>; Ding, Jian-feng <jian-feng.ding at intel.com>
> Subject: Re: [libvirt][PATCH v13 1/6] Define SGX capabilities structs
> 
> On 7/1/22 21:14, Lin Yang wrote:
> > From: Haibin Huang <haibin.huang at intel.com>
> >
> > Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> > Signed-off-by: Haibin Huang <haibin.huang at intel.com>
> > ---
> >  src/conf/domain_capabilities.c | 10 ++++++++++
> > src/conf/domain_capabilities.h | 24 ++++++++++++++++++++++++
> >  src/libvirt_private.syms       |  1 +
> >  3 files changed, 35 insertions(+)
> >
> > diff --git a/src/conf/domain_capabilities.c
> > b/src/conf/domain_capabilities.c index 895e8d00e8..27f3fb8d36 100644
> > --- a/src/conf/domain_capabilities.c
> > +++ b/src/conf/domain_capabilities.c
> > @@ -76,6 +76,16 @@ virSEVCapabilitiesFree(virSEVCapability *cap)  }
> >
> >
> > +void
> > +virSGXCapabilitiesFree(virSGXCapability *cap) {
> > +    if (!cap)
> > +        return;
> > +
> 
> This leaks cap->pSections.
[Haibin] OK
> 
> > +    g_free(cap);
> > +}
> > +
> > +
> >  static void
> >  virDomainCapsDispose(void *obj)
> >  {
> > diff --git a/src/conf/domain_capabilities.h
> > b/src/conf/domain_capabilities.h index f2eed80b15..dac1098e98 100644
> > --- a/src/conf/domain_capabilities.h
> > +++ b/src/conf/domain_capabilities.h
> > @@ -192,6 +192,24 @@ struct _virSEVCapability {
> >      unsigned int max_es_guests;
> >  };
> >
> > +typedef struct _virSection virSection; typedef virSection
> > +*virSectionPtr;
> 
> No, if we can help it I'd rather avoid introducing virXXXPtr typedef.
> We've worked hard to get rid of them and I don't quite see a reason to
> reintroduce them.
> 
> > +struct _virSection {
> > +    unsigned long long size;
> > +    unsigned int node;
> 
> While it's true that QEMU with its current code does not ever report a
> negative number for 'node', at the QAPI level it's declared as signed integer
> and thus we ought to reflect that.
> 
[Haibin] OK
> > +};
> > +
> > +typedef struct _virSGXCapability virSGXCapability; typedef
> > +virSGXCapability *virSGXCapabilityPtr; struct _virSGXCapability {
> > +    bool flc;
> > +    bool sgx1;
> > +    bool sgx2;
> > +    unsigned long long section_size;
> > +    size_t nSections;
> > +    virSectionPtr pSections;
> 
> We tend to use: 'nitems' and 'items', or 'nsections' and 'sections' in cases like
> this.
[Haibin] OK
> 
> > +};
> > +
> 
> Michal