<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple" style="word-wrap:break-word"> <div class="WordSection1"> On 6/2/22, 11:28 AM, "Yang, Lin A" <lin.a.yang@intel.com> wrote:<o:p></o:p> > On 6/1/22, 11:37 PM, "Michal Prívozník" <mprivozn@redhat.com> wrote:<o:p></o:p> > > Worst case scenario we can do a version check. It's very suboptimal<o:p></o:p> > > because if somebody backports your patches in QEMU, libvirt will stop<o:p></o:p> > > working despite having the version check.<o:p></o:p> > > <o:p></o:p> > > Therefore, I'm more inclined to just use the newest API and well, 6.2<o:p></o:p> > > won't work. In the long run - it's just one release that has the feature<o:p></o:p> > > but libvirt can't use it versus plenty of releases (that come after 7.0)<o:p></o:p> > > which have the feature and libvirt can use it. If we go this way then<o:p></o:p> > > we'll still need version check, but the other way round:<o:p></o:p> > > <o:p></o:p> > > if (qemuCaps->version < 7000000)<o:p></o:p> > > virQEMUCapsClear(qemuCaps, QEMU_CAPS_SGX_EPC);<o:p></o:p> > > <o:p></o:p> > > Or just exit early and don't even bother detecting SGX when version is<o:p></o:p> > > not 7.0.0:<o:p></o:p> > > <o:p></o:p> > > if (qemuCaps->version < 7000000)<o:p></o:p> > > return 0;<o:p></o:p> > > <o:p></o:p> > > This has the downside that when somebody backports QEMU patches to 6.2<o:p></o:p> > > to match the QAPI of 7.0 libvirt would still refuse to use the feature.<o:p></o:p> > > But one can argue that at that point the maintainer should also patch<o:p></o:p> > > libvirt (very trivial patch to remove these two lines of condition).<o:p></o:p> > > <o:p></o:p> > > This is the reason we like QEMU to make features introspectable - we<o:p></o:p> > > could avoid all of this if we were able to detect .node attribute :-(<o:p></o:p> > > <o:p></o:p> > > Now that I look at the output of query-qmp-schema command I see that 6.2<o:p></o:p> > > returns:<o:p></o:p> > > <o:p></o:p> > > {<o:p></o:p> > > "name": "237",<o:p></o:p> > > "members": [<o:p></o:p> > > {<o:p></o:p> > > "name": "sgx",<o:p></o:p> > > "type": "bool"<o:p></o:p> > > },<o:p></o:p> > > {<o:p></o:p> > > "name": "sgx1",<o:p></o:p> > > "type": "bool"<o:p></o:p> > > },<o:p></o:p> > > {<o:p></o:p> > > "name": "sgx2",<o:p></o:p> > > "type": "bool"<o:p></o:p> > > },<o:p></o:p> > > {<o:p></o:p> > > "name": "flc",<o:p></o:p> > > "type": "bool"<o:p></o:p> > > },<o:p></o:p> > > {<o:p></o:p> > > "name": "section-size",<o:p></o:p> > > "type": "int"<o:p></o:p> > > }<o:p></o:p> > > ],<o:p></o:p> > > "meta-type": "object"<o:p></o:p> > > },<o:p></o:p> > > <o:p></o:p> > > while 7.0 returns:<o:p></o:p> > > <o:p></o:p> > > {<o:p></o:p> > > "name": "237",<o:p></o:p> > > "members": [<o:p></o:p> > > {<o:p></o:p> > > "name": "sgx",<o:p></o:p> > > "type": "bool"<o:p></o:p> > > },<o:p></o:p> > > {<o:p></o:p> > > "name": "sgx1",<o:p></o:p> > > "type": "bool"<o:p></o:p> > > },<o:p></o:p> > > {<o:p></o:p> > > "name": "sgx2",<o:p></o:p> > > "type": "bool"<o:p></o:p> > > },<o:p></o:p> > > {<o:p></o:p> > > "name": "flc",<o:p></o:p> > > "type": "bool"<o:p></o:p> > > },<o:p></o:p> > > {<o:p></o:p> > > "name": "section-size",<o:p></o:p> > > "type": "int",<o:p></o:p> > > "features": [<o:p></o:p> > > "deprecated"<o:p></o:p> > > ]<o:p></o:p> > > },<o:p></o:p> > > {<o:p></o:p> > > "name": "sections",<o:p></o:p> > > "type": "[454]"<o:p></o:p> > > }<o:p></o:p> > > ],<o:p></o:p> > > "meta-type": "object"<o:p></o:p> > > }<o:p></o:p> > > <o:p></o:p> > > <o:p></o:p> > > So maybe in the end libvirt CAN know the difference without having to do<o:p></o:p> > > any version check. We have a "dialect" of XPATH that we use to traverse<o:p></o:p> > > the QMP schema: look at the comment above virQEMUQAPISchemaPathGet().<o:p></o:p> > <o:p></o:p> > This is awesome! Thank you so much for this very informative explanation.<o:p></o:p> > QEMU 7.0.0 provides more NUMA info in SGX part, so if we see "sections" in<o:p></o:p> > QAPI schema, we can assume .node attribute is required. <o:p> </o:p> > <o:p></o:p> > Let me try this solution at first. We can support both QEMU 6.2.0 and 7.0.0 in<o:p></o:p> > V13 patches if it is doable.<o:p></o:p> <o:p> </o:p> Sorry for multiple emails here.<o:p></o:p> <o:p> </o:p> Since these patches here have been review several times, and support 7.0.0 will bring<o:p></o:p> some new commits. Each update to new commit will require git rebase and resolve<o:p></o:p> conflict for old commits. Is it possible that we add the feature to compare QAPI schema<o:p></o:p> and detect .node, but only work with 6.2.0 in this patch and return error message for<o:p></o:p> 7.0.0. After finishing this thread, we can start a new thread to support NUMA for qemu<o:p></o:p> 7.0.0. Any preference?<o:p></o:p> <o:p> </o:p> Thanks,<o:p></o:p> Lin.<o:p></o:p> <o:p> </o:p> </div> </body> </html>