I understand and thank you very much for your meticulous explanation. > -----Original Message----- > From: Michal Prívozník <mprivozn@xxxxxxxxxx> > Sent: Wednesday, February 23, 2022 8:16 PM > To: Huang, Haibin <haibin.huang@xxxxxxxxx>; libvir-list@xxxxxxxxxx; > berrange@xxxxxxxxxx; Ding, Jian-feng <jian-feng.ding@xxxxxxxxx>; Yang, Lin > A <lin.a.yang@xxxxxxxxx>; Lu, Lianhao <lianhao.lu@xxxxxxxxx> > Subject: Re: [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