RE: [libvirt][PATCH v13 1/6] Define SGX capabilities structs
-----Original Message-----
From: Michal Prívozník <mprivozn(a)redhat.com>
Sent: Wednesday, July 20, 2022 7:27 PM
To: Yang, Lin A <lin.a.yang(a)intel.com>; libvir-list(a)redhat.com; Huang, Haibin
<haibin.huang(a)intel.com>; Ding, Jian-feng <jian-feng.ding(a)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(a)intel.com>
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> Signed-off-by: Haibin Huang <haibin.huang(a)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