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