On Wed, Sep 14, 2016 at 10:26:51 +0800, Dou Liyang wrote:
> Hi Peter,
>
> At 09/14/2016 12:27 AM, Peter Krempa wrote:
> > Return whether a vcpu entry is hotpluggable or online so that upper
> > layers don't have to infer the information from other data.
> >
> > Advantage is that this code can be tested by unit tests.
> > ---
> > src/qemu/qemu_monitor.c | 11 +++++
> > src/qemu/qemu_monitor.h | 4 ++
> > .../qemumonitorjson-cpuinfo-ppc64-basic.data | 48 ++++++++++++++++++++++
> > .../qemumonitorjson-cpuinfo-ppc64-hotplug-1.data | 48 ++++++++++++++++++++++
> > .../qemumonitorjson-cpuinfo-ppc64-hotplug-2.data | 48 ++++++++++++++++++++++
> > .../qemumonitorjson-cpuinfo-ppc64-hotplug-4.data | 48 ++++++++++++++++++++++
> > .../qemumonitorjson-cpuinfo-ppc64-no-threads.data | 32 +++++++++++++++
> > ...emumonitorjson-cpuinfo-x86-basic-pluggable.data | 16 ++++++++
> > .../qemumonitorjson-cpuinfo-x86-full.data | 22 ++++++++++
> > tests/qemumonitorjsontest.c | 3 ++
> > 10 files changed, 280 insertions(+)
> >
> > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> > index 4489997..e700b15 100644
> > --- a/src/qemu/qemu_monitor.c
> > +++ b/src/qemu/qemu_monitor.c
> > @@ -1773,6 +1773,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl
> > int order = 1;
> > size_t totalvcpus = 0;
> > size_t mastervcpu; /* this iterator is used for iterating hotpluggable entities */
> > + size_t slavevcpu; /* this corresponds to subentries of a hotpluggable entry */
> > size_t anyvcpu; /* this iterator is used for any vcpu entry in the result */
> > size_t i;
> > size_t j;
> > @@ -1816,6 +1817,10 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl
> > * logical vcpus in the guest */
> > mastervcpu = 0;
> > for (i = 0; i < nhotplugvcpus; i++) {
> > + vcpus[mastervcpu].online = !!hotplugvcpus[i].qom_path;
> > + vcpus[mastervcpu].hotpluggable = !!hotplugvcpus[i].alias;
> > + if (!vcpus[mastervcpu].online)
> > + vcpus[mastervcpu].hotpluggable = true;
>
> I think if the vcpu don't have an alias, we mark it as non-hotpluggable.
> so, the code above may not fit well.
The hotplugvcpus.alias field is NULL in the following two cases:
1) vcpu is online but was selected as non-hotpluggable
2) the vcpu is offline
The hotpluggable field in libvirt is covering both the hotplug and
hotunplug case, thus a vcpu shall be marked as hotpluggable if:
1) it's offline
2) has the correct alias.
> I guess you may mean that:
>
> vcpus[mastervcpu].online = !!hotplugvcpus[i].qom_path;
> vcpus[mastervcpu].hotpluggable = false;
> if (!vcpus[mastervcpu].online && (!!hotplugvcpus[i].alias))
> vcpus[mastervcpu].hotpluggable = true;
This is wrong as online vcpus would not be marked as hotpluggable (which
by the way in the libvirt context also means that it can be unplugged).
>
> OR
>
> vcpus[mastervcpu].online = !!hotplugvcpus[i].qom_path;
>
> if (vcpus[mastervcpu].online)
> vcpus[mastervcpu].hotpluggable = false;
This is wrong in the same regards.
> else
> vcpus[mastervcpu].hotpluggable = !!hotplugvcpus[i].alias;
Also offline vcpu won't ever have an alias, since it's offline.
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list