On 6/3/22 09:36, Peter Krempa wrote: > On Thu, Jun 02, 2022 at 22:49:15 +0000, Yang, Lin A wrote: >> 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: > > [...] > >>>> 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? > > Usually we prefer if everything is done in one series, but obviously > that is not always possible. > > In case you want to commit partially-incomplete patches you need to > ensure that they behave sanely for anyone attempting to use it. > > This means mostly that if e.g. your code would work with qemu-6.2 and > would then break with qemu-7.0 it _must_ be kept disabled. > > I didn't go through the conversation here, but from the above it seems > that you are discussing that there are two distinct ways how to detect > the presence of the feature in qemu between qemu-6.2 and qemu-7.0. More or less, yes. The feature works differently in 6.2. And the same cmd line doesn't work in 7.0. > > If that is true then I'd simply say it's strongly preferrable to just > drop the code for qemu-6.2 and just do the necessary steps to make it > work with qemu-7.0. Adding another bit of code just to make it work with > one extra version doesn't seem to be worth it unless you have a very > good justification, because it's more code that we'll have to maintain > in the end. > Exactly. This is what I was also suggesting, to just not bother with 6.2 at all and aim at 7.0. But it looks like Yang is aiming on supporting 6.2 too. This is going to create complicated implementation for a little benefit IMO. But I'm not the one writing patches :-) Michal
