[libvirt] [PATCH 0/3] query-cpu-model-baseline QMP command

Chris Venteicher posted 3 patches 5 years, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180416060658.31760-1-cventeic@redhat.com
Test syntax-check passed
src/qemu/qemu_monitor.c      |  16 ++++
src/qemu/qemu_monitor.h      |   5 ++
src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++++++++++++------
src/qemu/qemu_monitor_json.h |   7 ++
4 files changed, 185 insertions(+), 22 deletions(-)
[libvirt] [PATCH 0/3] query-cpu-model-baseline QMP command
Posted by Chris Venteicher 5 years, 11 months ago
Implementation of libvirt functions to support the
QEMU query-cpu-model-baseline QMP command.

This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999

Signed-off-by: Chris Venteicher <cventeic@redhat.com>

Chris Venteicher (3):
  qemu_monitor_json: Populate CPUModelInfo struct from json
  qemu_monitor_json: Build Json CPU Model Info
  qemu_monitor: query-cpu-model-baseline QMP command

 src/qemu/qemu_monitor.c      |  16 ++++
 src/qemu/qemu_monitor.h      |   5 ++
 src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++++++++++++------
 src/qemu/qemu_monitor_json.h |   7 ++
 4 files changed, 185 insertions(+), 22 deletions(-)

-- 
2.14.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] query-cpu-model-baseline QMP command
Posted by Collin Walling 5 years, 11 months ago
On 04/16/2018 02:06 AM, Chris Venteicher wrote:
> Implementation of libvirt functions to support the
> QEMU query-cpu-model-baseline QMP command.
> 
> This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
> 
> Signed-off-by: Chris Venteicher <cventeic@redhat.com>
> 
> Chris Venteicher (3):
>   qemu_monitor_json: Populate CPUModelInfo struct from json
>   qemu_monitor_json: Build Json CPU Model Info
>   qemu_monitor: query-cpu-model-baseline QMP command
> 
>  src/qemu/qemu_monitor.c      |  16 ++++
>  src/qemu/qemu_monitor.h      |   5 ++
>  src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++++++++++++------
>  src/qemu/qemu_monitor_json.h |   7 ++
>  4 files changed, 185 insertions(+), 22 deletions(-)
> 
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] query-cpu-model-baseline QMP command
Posted by Chris Venteicher 5 years, 11 months ago

----- Original Message -----
> From: "Collin Walling" <walling@linux.ibm.com>
> To: "Chris Venteicher" <cventeic@redhat.com>, libvir-list@redhat.com
> Sent: Monday, April 16, 2018 6:26:07 PM
> Subject: Re: [libvirt] [PATCH 0/3] query-cpu-model-baseline QMP command
> 
> On 04/16/2018 02:06 AM, Chris Venteicher wrote:
> > Implementation of libvirt functions to support the
> > QEMU query-cpu-model-baseline QMP command.
> > 
> > This is part of resolution of:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1511999
> > 
> > Signed-off-by: Chris Venteicher <cventeic@redhat.com>
> > 
> > Chris Venteicher (3):
> >   qemu_monitor_json: Populate CPUModelInfo struct from json
> >   qemu_monitor_json: Build Json CPU Model Info
> >   qemu_monitor: query-cpu-model-baseline QMP command
> > 
> >  src/qemu/qemu_monitor.c      |  16 ++++
> >  src/qemu/qemu_monitor.h      |   5 ++
> >  src/qemu/qemu_monitor_json.c | 179
> >  +++++++++++++++++++++++++++++++++++++------
> >  src/qemu/qemu_monitor_json.h |   7 ++
> >  4 files changed, 185 insertions(+), 22 deletions(-)
> > 
> 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
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 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.)

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.

That seem good to you?
Chris

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] query-cpu-model-baseline QMP command
Posted by Collin Walling 5 years, 11 months ago
[...]

>> 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