RE: [libvirt][PATCH RESEND v10 1/5] qemu: provide support to query the SGX capability
I understand and thank you very much for your meticulous explanation.
-----Original Message-----
From: Michal Prívozník <mprivozn(a)redhat.com>
Sent: Wednesday, February 23, 2022 8:16 PM
To: Huang, Haibin <haibin.huang(a)intel.com>; libvir-list(a)redhat.com;
berrange(a)redhat.com; Ding, Jian-feng <jian-feng.ding(a)intel.com>; Yang, Lin
A <lin.a.yang(a)intel.com>; Lu, Lianhao <lianhao.lu(a)intel.com>
Subject: Re: [libvirt][PATCH RESEND v10 1/5] qemu: provide support to query
the SGX capability
On 2/23/22 04:41, Huang, Haibin wrote:
> I accept all comments. But I didn't your point for this comment .
>>> +static void
>>> +virQEMUCapsFormatSGXInfo(virQEMUCaps *qemuCaps, virBuffer *buf)
{
>>> + virSGXCapabilityPtr sgx =
>>> +virQEMUCapsGetSGXCapabilities(qemuCaps);
>>> +
>>> + virBufferAddLit(buf, "<sgx>\n");
>>> + virBufferAdjustIndent(buf, 2);
>>> + virBufferAsprintf(buf, "<flc>%s</flc>\n",
sgx->flc ? "yes" :
>>> + "no");
>>
>> ... which is in contradiction with the way we format it.
> [Haibin] sorry, I don't get your point, can you give me the details
>
> Does you mean that change virBufferAsprintf(buf,
"<flc>%s</flc>\n",
> sgx->flc ? "yes" : "no"); to virBufferAsprintf(buf,
"<flc>%u</flc>\n",
> sgx->flc);
No. You can keep this particual virBufferAsprintf() as is. What I have problem
with is the parsing side. So if this virQEMUCapsFormatSGXInfo() function is
called, if produces the following XML:
<sgx>
<flc>no</flc>
<epc_size>1024</epc_size>
</sgx>
(I've made the values up)
This corresponds to: sgx->flc = false; sgx->epc_size = 1024;
But when the XML is parsed, in virQEMUCapsParseSGXInfo(), it would se the
following values:
sgx->flc = true; sgx->epc_size = 2014;
Notice the change in ->flc? This is because
virXPathBoolean("boolean(./sgx/flc)") does not evaluate the value of
<flc/>
element, but only whether the xpath exists. Therefore, even if flc's element
value is "no", it does exist and as such is evaluated to true.
Hope this cleans things up.
Michal