[...] >> Hi Chris, >> >> I'm working on a neighboring item with implementing cpu-comparison. Since >> there >> are some similarities with both comparison and baseline, I'd like for us to >> work together on driving both features forward. >> >> Thus far, I've created the qemu_monitor_json interface, tied it into the qemu >> driver, and can issue virsh cpu-compare <file.xml> successfully. (Though it's >> no where near perfect yet ;) ) >> >> I've already noted some differences between our JSON interaces (namely, yours >> works with qemuMonitorCPUModelInfoPtrs, and mine works with virCPUDefPtrs). >> >> Ultimately, I think your patches are in good shape. >> >> Let's discuss further on my comparison patches posted here: >> >> https://www.redhat.com/archives/libvir-list/2018-April/msg01375.html >> >> I'm open to any suggestions / design ideas you have. >> >> -- >> Respectfully, >> - Collin Walling >> >> > > Hi Collin, > > Not sure if you received my previous reply on this... It's possible it got > stuck in my mail box. > > Seems like we have got our wires crossed on this... > I also have an implementation going in parallel for virsh cpu-comparison I > have been working under Bug 1511996 - Implement "virsh cpu-compare" for s390x. > > As you mentioned the requirements for both are similar in that > 1) Both functions work with cpu model info > 2) Both functions require a qemu instance to be spun up during execution. > > I took a look at your code for cpu-comparison. > > 1) Seems like it will be good to compare and contrast how both of us spin up > the qemu instance for best practice
I look forward to seeing your approach on this. > 2) Looks like you translate between XML --- virCpuDef --- qemu qmp JSON > I ended up just going straight from XML --- qemu qmp JSON > > Think there is an open question about which approach is best. I did XML -> CpuDef -> JSON because my approach utilized the CpuDef found in virCaps (which I stole from qemuCaps, but it looks like this is not the way we should being do this). It looked cleaner to pass two cpudefs to the monitor function. I think it'll be more clear to both of us which approach is better once we see how the host / hypervisor CPU will be stored. > > I have only pushed the qmp JSON functions to libvirt-list so far. > (In fact I just pushed v2 if you have a moment to look again.) I'll take a look at them. > > However I have the rest of the code for baseline, including launching the > qemu instance, working and nearly ready to push. > > I guess at this point I think it might be best to just finish pushing the > rest of the code for baseline and see what the feedback is and to make it > available for you to look at. > > I am especially interested in getting feedback from community on skipping the > virCpuDef step in the XML to JSON translations. > > Then we could revisit and address any delta between baseline and > cpu-comparison. > > If straight XML to JSON is sufficient we might want to leverage more of what > I have. > If the virCpuDef step is needed then leverage more of yours. > > Either way try and make the QEMU spin up as clean as possible. Agreed. > > That seem good to you? Perfectly fine with me :) At this point, it seems like my role in this is better suited as a reviewer -- which is fine. I'll be keeping a close eye on the forthcoming patches and provide my 2cents. > Chris > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > -- Respectfully, - Collin Walling -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
