[libvirt] [PATCH 0/4] Fix incorrect vcpu data refresh on daemon restart after vcpu hotplug

See patches 3 and 4 for explanation. Peter Krempa (4): qemu: monitor: Use a more obvious iterator name qemu: monitor: qemuMonitorGetCPUInfoHotplug: Add iterator 'anycpu' qemu: monitor: Add vcpu state information to monitor data qemu: domain: Don't infer vcpu state src/qemu/qemu_domain.c | 14 +++--- src/qemu/qemu_monitor.c | 51 ++++++++++++++-------- 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 ++ 11 files changed, 306 insertions(+), 28 deletions(-) -- 2.10.0

The algorithm that matches data from query-cpus and query-hotpluggable-cpus is quite complex. Start using descriptive iterator names to avoid confusion. --- src/qemu/qemu_monitor.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1fdee3a..b8da637 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1772,6 +1772,7 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl char *tmp; int order = 1; size_t totalvcpus = 0; + size_t mastervcpu; /* this iterator is used for iterating hotpluggable entities */ size_t i; size_t j; @@ -1812,19 +1813,19 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl /* transfer appropriate data from the hotpluggable list to corresponding * entries. the entries returned by qemu may in fact describe multiple * logical vcpus in the guest */ - j = 0; + mastervcpu = 0; for (i = 0; i < nhotplugvcpus; i++) { - vcpus[j].socket_id = hotplugvcpus[i].socket_id; - vcpus[j].core_id = hotplugvcpus[i].core_id; - vcpus[j].thread_id = hotplugvcpus[i].thread_id; - vcpus[j].vcpus = hotplugvcpus[i].vcpus; - VIR_STEAL_PTR(vcpus[j].qom_path, hotplugvcpus[i].qom_path); - VIR_STEAL_PTR(vcpus[j].alias, hotplugvcpus[i].alias); - VIR_STEAL_PTR(vcpus[j].type, hotplugvcpus[i].type); - vcpus[j].id = hotplugvcpus[i].enable_id; - - /* skip over vcpu entries covered by this hotpluggable entry */ - j += hotplugvcpus[i].vcpus; + vcpus[mastervcpu].socket_id = hotplugvcpus[i].socket_id; + vcpus[mastervcpu].core_id = hotplugvcpus[i].core_id; + vcpus[mastervcpu].thread_id = hotplugvcpus[i].thread_id; + vcpus[mastervcpu].vcpus = hotplugvcpus[i].vcpus; + VIR_STEAL_PTR(vcpus[mastervcpu].qom_path, hotplugvcpus[i].qom_path); + VIR_STEAL_PTR(vcpus[mastervcpu].alias, hotplugvcpus[i].alias); + VIR_STEAL_PTR(vcpus[mastervcpu].type, hotplugvcpus[i].type); + vcpus[mastervcpu].id = hotplugvcpus[i].enable_id; + + /* calculate next master vcpu (hotpluggable unit) entry */ + mastervcpu += hotplugvcpus[i].vcpus; } /* match entries from query cpus to the output array taking into account -- 2.10.0

On Tue, Sep 13, 2016 at 06:27:43PM +0200, Peter Krempa wrote:
The algorithm that matches data from query-cpus and query-hotpluggable-cpus is quite complex. Start using descriptive iterator names to avoid confusion. --- src/qemu/qemu_monitor.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
ACK Pavel

Add separate iterator for iterating all the entries --- src/qemu/qemu_monitor.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b8da637..4489997 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 anyvcpu; /* this iterator is used for any vcpu entry in the result */ size_t i; size_t j; @@ -1832,27 +1833,27 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl * multi-vcpu objects */ for (j = 0; j < ncpuentries; j++) { /* find the correct entry or beginning of group of entries */ - for (i = 0; i < maxvcpus; i++) { - if (cpuentries[j].qom_path && vcpus[i].qom_path && - STREQ(cpuentries[j].qom_path, vcpus[i].qom_path)) + for (anyvcpu = 0; anyvcpu < maxvcpus; anyvcpu++) { + if (cpuentries[j].qom_path && vcpus[anyvcpu].qom_path && + STREQ(cpuentries[j].qom_path, vcpus[anyvcpu].qom_path)) break; } - if (i == maxvcpus) { + if (anyvcpu == maxvcpus) { VIR_DEBUG("too many query-cpus entries for a given " "query-hotpluggable-cpus entry"); return -1; } - if (vcpus[i].vcpus != 1) { + if (vcpus[anyvcpu].vcpus != 1) { /* find a possibly empty vcpu thread for core granularity systems */ - for (; i < maxvcpus; i++) { - if (vcpus[i].tid == 0) + for (; anyvcpu < maxvcpus; anyvcpu++) { + if (vcpus[anyvcpu].tid == 0) break; } } - vcpus[i].tid = cpuentries[j].tid; + vcpus[anyvcpu].tid = cpuentries[j].tid; } return 0; -- 2.10.0

On Tue, Sep 13, 2016 at 06:27:44PM +0200, Peter Krempa wrote:
Add separate iterator for iterating all the entries --- src/qemu/qemu_monitor.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
ACK Pavel

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; vcpus[mastervcpu].socket_id = hotplugvcpus[i].socket_id; vcpus[mastervcpu].core_id = hotplugvcpus[i].core_id; vcpus[mastervcpu].thread_id = hotplugvcpus[i].thread_id; @@ -1825,6 +1830,12 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl VIR_STEAL_PTR(vcpus[mastervcpu].type, hotplugvcpus[i].type); vcpus[mastervcpu].id = hotplugvcpus[i].enable_id; + /* copy state information to slave vcpus */ + for (slavevcpu = mastervcpu + 1; slavevcpu < mastervcpu + hotplugvcpus[i].vcpus; slavevcpu++) { + vcpus[slavevcpu].online = vcpus[mastervcpu].online; + vcpus[slavevcpu].hotpluggable = vcpus[mastervcpu].hotpluggable; + } + /* calculate next master vcpu (hotpluggable unit) entry */ mastervcpu += hotplugvcpus[i].vcpus; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 255fff2..615ab3e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -422,6 +422,10 @@ struct _qemuMonitorCPUInfo { pid_t tid; int id; /* order of enabling of the given cpu */ + /* state data */ + bool online; + bool hotpluggable; + /* topology info for hotplug purposes. Hotplug of given vcpu impossible if * all entries are -1 */ int socket_id; diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data index 9fc8148..ae7c2f4 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data @@ -1,40 +1,88 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='21925' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='8' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='21926' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='21927' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='21928' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='21930' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='21931' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='21932' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='21933' [vcpu libvirt-id='8'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='8' vcpus='8' [vcpu libvirt-id='9'] + online=no + hotpluggable=yes [vcpu libvirt-id='10'] + online=no + hotpluggable=yes [vcpu libvirt-id='11'] + online=no + hotpluggable=yes [vcpu libvirt-id='12'] + online=no + hotpluggable=yes [vcpu libvirt-id='13'] + online=no + hotpluggable=yes [vcpu libvirt-id='14'] + online=no + hotpluggable=yes [vcpu libvirt-id='15'] + online=no + hotpluggable=yes [vcpu libvirt-id='16'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='16' vcpus='8' [vcpu libvirt-id='17'] + online=no + hotpluggable=yes [vcpu libvirt-id='18'] + online=no + hotpluggable=yes [vcpu libvirt-id='19'] + online=no + hotpluggable=yes [vcpu libvirt-id='20'] + online=no + hotpluggable=yes [vcpu libvirt-id='21'] + online=no + hotpluggable=yes [vcpu libvirt-id='22'] + online=no + hotpluggable=yes [vcpu libvirt-id='23'] + online=no + hotpluggable=yes diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data index b0139b5..5c0a6af 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data @@ -1,24 +1,42 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='21925' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='8' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='21926' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='21927' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='21928' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='21930' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='21931' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='21932' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='21933' [vcpu libvirt-id='8'] + online=yes + hotpluggable=yes thread-id='22131' qemu-id='2' type='host-spapr-cpu-core' @@ -26,26 +44,56 @@ qom_path='/machine/peripheral/vcpu0' topology: core='8' vcpus='8' [vcpu libvirt-id='9'] + online=yes + hotpluggable=yes thread-id='22132' [vcpu libvirt-id='10'] + online=yes + hotpluggable=yes thread-id='22133' [vcpu libvirt-id='11'] + online=yes + hotpluggable=yes thread-id='22134' [vcpu libvirt-id='12'] + online=yes + hotpluggable=yes thread-id='22135' [vcpu libvirt-id='13'] + online=yes + hotpluggable=yes thread-id='22136' [vcpu libvirt-id='14'] + online=yes + hotpluggable=yes thread-id='22137' [vcpu libvirt-id='15'] + online=yes + hotpluggable=yes thread-id='22138' [vcpu libvirt-id='16'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='16' vcpus='8' [vcpu libvirt-id='17'] + online=no + hotpluggable=yes [vcpu libvirt-id='18'] + online=no + hotpluggable=yes [vcpu libvirt-id='19'] + online=no + hotpluggable=yes [vcpu libvirt-id='20'] + online=no + hotpluggable=yes [vcpu libvirt-id='21'] + online=no + hotpluggable=yes [vcpu libvirt-id='22'] + online=no + hotpluggable=yes [vcpu libvirt-id='23'] + online=no + hotpluggable=yes diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data index ea4b099..ba4044e 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data @@ -1,24 +1,42 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='21925' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='8' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='21926' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='21927' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='21928' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='21930' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='21931' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='21932' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='21933' [vcpu libvirt-id='8'] + online=yes + hotpluggable=yes thread-id='22131' qemu-id='2' type='host-spapr-cpu-core' @@ -26,20 +44,36 @@ qom_path='/machine/peripheral/vcpu0' topology: core='8' vcpus='8' [vcpu libvirt-id='9'] + online=yes + hotpluggable=yes thread-id='22132' [vcpu libvirt-id='10'] + online=yes + hotpluggable=yes thread-id='22133' [vcpu libvirt-id='11'] + online=yes + hotpluggable=yes thread-id='22134' [vcpu libvirt-id='12'] + online=yes + hotpluggable=yes thread-id='22135' [vcpu libvirt-id='13'] + online=yes + hotpluggable=yes thread-id='22136' [vcpu libvirt-id='14'] + online=yes + hotpluggable=yes thread-id='22137' [vcpu libvirt-id='15'] + online=yes + hotpluggable=yes thread-id='22138' [vcpu libvirt-id='16'] + online=yes + hotpluggable=yes thread-id='22223' qemu-id='3' type='host-spapr-cpu-core' @@ -47,16 +81,30 @@ qom_path='/machine/peripheral/vcpu1' topology: core='16' vcpus='8' [vcpu libvirt-id='17'] + online=yes + hotpluggable=yes thread-id='22224' [vcpu libvirt-id='18'] + online=yes + hotpluggable=yes thread-id='22225' [vcpu libvirt-id='19'] + online=yes + hotpluggable=yes thread-id='22226' [vcpu libvirt-id='20'] + online=yes + hotpluggable=yes thread-id='22227' [vcpu libvirt-id='21'] + online=yes + hotpluggable=yes thread-id='22228' [vcpu libvirt-id='22'] + online=yes + hotpluggable=yes thread-id='22229' [vcpu libvirt-id='23'] + online=yes + hotpluggable=yes thread-id='22230' diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data index 22a425d..d2c56ef 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data @@ -1,24 +1,42 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='21925' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='8' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='21926' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='21927' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='21928' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='21930' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='21931' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='21932' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='21933' [vcpu libvirt-id='8'] + online=yes + hotpluggable=yes thread-id='23170' qemu-id='3' type='host-spapr-cpu-core' @@ -26,20 +44,36 @@ qom_path='/machine/peripheral/vcpu0' topology: core='8' vcpus='8' [vcpu libvirt-id='9'] + online=yes + hotpluggable=yes thread-id='23171' [vcpu libvirt-id='10'] + online=yes + hotpluggable=yes thread-id='23172' [vcpu libvirt-id='11'] + online=yes + hotpluggable=yes thread-id='23173' [vcpu libvirt-id='12'] + online=yes + hotpluggable=yes thread-id='23174' [vcpu libvirt-id='13'] + online=yes + hotpluggable=yes thread-id='23175' [vcpu libvirt-id='14'] + online=yes + hotpluggable=yes thread-id='23176' [vcpu libvirt-id='15'] + online=yes + hotpluggable=yes thread-id='23177' [vcpu libvirt-id='16'] + online=yes + hotpluggable=yes thread-id='22741' qemu-id='2' type='host-spapr-cpu-core' @@ -47,16 +81,30 @@ qom_path='/machine/peripheral/vcpu1' topology: core='16' vcpus='8' [vcpu libvirt-id='17'] + online=yes + hotpluggable=yes thread-id='22742' [vcpu libvirt-id='18'] + online=yes + hotpluggable=yes thread-id='22743' [vcpu libvirt-id='19'] + online=yes + hotpluggable=yes thread-id='22744' [vcpu libvirt-id='20'] + online=yes + hotpluggable=yes thread-id='22745' [vcpu libvirt-id='21'] + online=yes + hotpluggable=yes thread-id='22746' [vcpu libvirt-id='22'] + online=yes + hotpluggable=yes thread-id='22747' [vcpu libvirt-id='23'] + online=yes + hotpluggable=yes thread-id='22748' diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data index d7ab77b..c2f541b 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data @@ -1,72 +1,104 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='35232' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='1' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='35233' qemu-id='2' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[2]' topology: core='8' vcpus='1' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='35234' qemu-id='3' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[3]' topology: core='16' vcpus='1' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='35235' qemu-id='4' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[4]' topology: core='24' vcpus='1' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='35236' qemu-id='5' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[5]' topology: core='32' vcpus='1' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='35237' qemu-id='6' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[6]' topology: core='40' vcpus='1' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='35238' qemu-id='7' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[7]' topology: core='48' vcpus='1' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='35239' qemu-id='8' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[8]' topology: core='56' vcpus='1' [vcpu libvirt-id='8'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='64' vcpus='1' [vcpu libvirt-id='9'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='72' vcpus='1' [vcpu libvirt-id='10'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='80' vcpus='1' [vcpu libvirt-id='11'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='88' vcpus='1' [vcpu libvirt-id='12'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='96' vcpus='1' [vcpu libvirt-id='13'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='104' vcpus='1' [vcpu libvirt-id='14'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='112' vcpus='1' [vcpu libvirt-id='15'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='120' vcpus='1' diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data index a367a09..67dfc01 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data @@ -1,39 +1,55 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='518291' qemu-id='1' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[0]' topology: socket='0' core='0' thread='0' vcpus='1' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='518292' qemu-id='2' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[2]' topology: socket='0' core='0' thread='1' vcpus='1' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='518294' qemu-id='3' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[3]' topology: socket='0' core='1' thread='0' vcpus='1' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='518295' qemu-id='4' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[4]' topology: socket='0' core='1' thread='1' vcpus='1' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='518296' qemu-id='5' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[5]' topology: socket='1' core='0' thread='0' vcpus='1' [vcpu libvirt-id='5'] + online=no + hotpluggable=yes type='qemu64-x86_64-cpu' topology: socket='1' core='0' thread='1' vcpus='1' [vcpu libvirt-id='6'] + online=no + hotpluggable=yes type='qemu64-x86_64-cpu' topology: socket='1' core='1' thread='0' vcpus='1' [vcpu libvirt-id='7'] + online=no + hotpluggable=yes type='qemu64-x86_64-cpu' topology: socket='1' core='1' thread='1' vcpus='1' diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data index a6c1069..dba3745 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data @@ -1,10 +1,14 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='895040' qemu-id='1' type='Broadwell-x86_64-cpu' qom_path='/machine/unattached/device[0]' topology: socket='0' core='0' thread='0' vcpus='1' [vcpu libvirt-id='1'] + online=yes + hotpluggable=yes thread-id='895056' qemu-id='2' type='Broadwell-x86_64-cpu' @@ -12,6 +16,8 @@ qom_path='/machine/peripheral/vcpu1' topology: socket='1' core='0' thread='0' vcpus='1' [vcpu libvirt-id='2'] + online=yes + hotpluggable=yes thread-id='895057' qemu-id='3' type='Broadwell-x86_64-cpu' @@ -19,6 +25,8 @@ qom_path='/machine/peripheral/vcpu2' topology: socket='2' core='0' thread='0' vcpus='1' [vcpu libvirt-id='3'] + online=yes + hotpluggable=yes thread-id='895058' qemu-id='4' type='Broadwell-x86_64-cpu' @@ -26,6 +34,8 @@ qom_path='/machine/peripheral/vcpu3' topology: socket='3' core='0' thread='0' vcpus='1' [vcpu libvirt-id='4'] + online=yes + hotpluggable=yes thread-id='895059' qemu-id='5' type='Broadwell-x86_64-cpu' @@ -33,6 +43,8 @@ qom_path='/machine/peripheral/vcpu4' topology: socket='4' core='0' thread='0' vcpus='1' [vcpu libvirt-id='5'] + online=yes + hotpluggable=yes thread-id='895060' qemu-id='6' type='Broadwell-x86_64-cpu' @@ -40,6 +52,8 @@ qom_path='/machine/peripheral/vcpu5' topology: socket='5' core='0' thread='0' vcpus='1' [vcpu libvirt-id='6'] + online=yes + hotpluggable=yes thread-id='895061' qemu-id='7' type='Broadwell-x86_64-cpu' @@ -47,6 +61,8 @@ qom_path='/machine/peripheral/vcpu6' topology: socket='6' core='0' thread='0' vcpus='1' [vcpu libvirt-id='7'] + online=yes + hotpluggable=yes thread-id='895062' qemu-id='8' type='Broadwell-x86_64-cpu' @@ -54,6 +70,8 @@ qom_path='/machine/peripheral/vcpu7' topology: socket='7' core='0' thread='0' vcpus='1' [vcpu libvirt-id='8'] + online=yes + hotpluggable=yes thread-id='895063' qemu-id='9' type='Broadwell-x86_64-cpu' @@ -61,6 +79,8 @@ qom_path='/machine/peripheral/vcpu8' topology: socket='8' core='0' thread='0' vcpus='1' [vcpu libvirt-id='9'] + online=yes + hotpluggable=yes thread-id='895064' qemu-id='10' type='Broadwell-x86_64-cpu' @@ -68,6 +88,8 @@ qom_path='/machine/peripheral/vcpu9' topology: socket='9' core='0' thread='0' vcpus='1' [vcpu libvirt-id='10'] + online=yes + hotpluggable=yes thread-id='895065' qemu-id='11' type='Broadwell-x86_64-cpu' diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index d3ec3b1..9e195d7 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2357,6 +2357,9 @@ testQemuMonitorCPUInfoFormat(qemuMonitorCPUInfoPtr vcpus, virBufferAsprintf(&buf, "[vcpu libvirt-id='%zu']\n", i); virBufferAdjustIndent(&buf, 4); + virBufferAsprintf(&buf, "online=%s\n", vcpu->online ? "yes" : "no"); + virBufferAsprintf(&buf, "hotpluggable=%s\n", vcpu->hotpluggable ? "yes" : "no"); + if (vcpu->tid) virBufferAsprintf(&buf, "thread-id='%llu'\n", (unsigned long long) vcpu->tid); -- 2.10.0

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. 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; OR vcpus[mastervcpu].online = !!hotplugvcpus[i].qom_path; if (vcpus[mastervcpu].online) vcpus[mastervcpu].hotpluggable = false; else vcpus[mastervcpu].hotpluggable = !!hotplugvcpus[i].alias; Just for suggest. :) Thanks, Dou
vcpus[mastervcpu].socket_id = hotplugvcpus[i].socket_id; vcpus[mastervcpu].core_id = hotplugvcpus[i].core_id; vcpus[mastervcpu].thread_id = hotplugvcpus[i].thread_id; @@ -1825,6 +1830,12 @@ qemuMonitorGetCPUInfoHotplug(struct qemuMonitorQueryHotpluggableCpusEntry *hotpl VIR_STEAL_PTR(vcpus[mastervcpu].type, hotplugvcpus[i].type); vcpus[mastervcpu].id = hotplugvcpus[i].enable_id;
+ /* copy state information to slave vcpus */ + for (slavevcpu = mastervcpu + 1; slavevcpu < mastervcpu + hotplugvcpus[i].vcpus; slavevcpu++) { + vcpus[slavevcpu].online = vcpus[mastervcpu].online; + vcpus[slavevcpu].hotpluggable = vcpus[mastervcpu].hotpluggable; + } + /* calculate next master vcpu (hotpluggable unit) entry */ mastervcpu += hotplugvcpus[i].vcpus; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 255fff2..615ab3e 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -422,6 +422,10 @@ struct _qemuMonitorCPUInfo { pid_t tid; int id; /* order of enabling of the given cpu */
+ /* state data */ + bool online; + bool hotpluggable; + /* topology info for hotplug purposes. Hotplug of given vcpu impossible if * all entries are -1 */ int socket_id; diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data index 9fc8148..ae7c2f4 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic.data @@ -1,40 +1,88 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='21925' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='8' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='21926' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='21927' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='21928' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='21930' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='21931' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='21932' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='21933' [vcpu libvirt-id='8'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='8' vcpus='8' [vcpu libvirt-id='9'] + online=no + hotpluggable=yes [vcpu libvirt-id='10'] + online=no + hotpluggable=yes [vcpu libvirt-id='11'] + online=no + hotpluggable=yes [vcpu libvirt-id='12'] + online=no + hotpluggable=yes [vcpu libvirt-id='13'] + online=no + hotpluggable=yes [vcpu libvirt-id='14'] + online=no + hotpluggable=yes [vcpu libvirt-id='15'] + online=no + hotpluggable=yes [vcpu libvirt-id='16'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='16' vcpus='8' [vcpu libvirt-id='17'] + online=no + hotpluggable=yes [vcpu libvirt-id='18'] + online=no + hotpluggable=yes [vcpu libvirt-id='19'] + online=no + hotpluggable=yes [vcpu libvirt-id='20'] + online=no + hotpluggable=yes [vcpu libvirt-id='21'] + online=no + hotpluggable=yes [vcpu libvirt-id='22'] + online=no + hotpluggable=yes [vcpu libvirt-id='23'] + online=no + hotpluggable=yes diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data index b0139b5..5c0a6af 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-1.data @@ -1,24 +1,42 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='21925' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='8' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='21926' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='21927' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='21928' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='21930' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='21931' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='21932' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='21933' [vcpu libvirt-id='8'] + online=yes + hotpluggable=yes thread-id='22131' qemu-id='2' type='host-spapr-cpu-core' @@ -26,26 +44,56 @@ qom_path='/machine/peripheral/vcpu0' topology: core='8' vcpus='8' [vcpu libvirt-id='9'] + online=yes + hotpluggable=yes thread-id='22132' [vcpu libvirt-id='10'] + online=yes + hotpluggable=yes thread-id='22133' [vcpu libvirt-id='11'] + online=yes + hotpluggable=yes thread-id='22134' [vcpu libvirt-id='12'] + online=yes + hotpluggable=yes thread-id='22135' [vcpu libvirt-id='13'] + online=yes + hotpluggable=yes thread-id='22136' [vcpu libvirt-id='14'] + online=yes + hotpluggable=yes thread-id='22137' [vcpu libvirt-id='15'] + online=yes + hotpluggable=yes thread-id='22138' [vcpu libvirt-id='16'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='16' vcpus='8' [vcpu libvirt-id='17'] + online=no + hotpluggable=yes [vcpu libvirt-id='18'] + online=no + hotpluggable=yes [vcpu libvirt-id='19'] + online=no + hotpluggable=yes [vcpu libvirt-id='20'] + online=no + hotpluggable=yes [vcpu libvirt-id='21'] + online=no + hotpluggable=yes [vcpu libvirt-id='22'] + online=no + hotpluggable=yes [vcpu libvirt-id='23'] + online=no + hotpluggable=yes diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data index ea4b099..ba4044e 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-2.data @@ -1,24 +1,42 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='21925' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='8' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='21926' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='21927' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='21928' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='21930' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='21931' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='21932' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='21933' [vcpu libvirt-id='8'] + online=yes + hotpluggable=yes thread-id='22131' qemu-id='2' type='host-spapr-cpu-core' @@ -26,20 +44,36 @@ qom_path='/machine/peripheral/vcpu0' topology: core='8' vcpus='8' [vcpu libvirt-id='9'] + online=yes + hotpluggable=yes thread-id='22132' [vcpu libvirt-id='10'] + online=yes + hotpluggable=yes thread-id='22133' [vcpu libvirt-id='11'] + online=yes + hotpluggable=yes thread-id='22134' [vcpu libvirt-id='12'] + online=yes + hotpluggable=yes thread-id='22135' [vcpu libvirt-id='13'] + online=yes + hotpluggable=yes thread-id='22136' [vcpu libvirt-id='14'] + online=yes + hotpluggable=yes thread-id='22137' [vcpu libvirt-id='15'] + online=yes + hotpluggable=yes thread-id='22138' [vcpu libvirt-id='16'] + online=yes + hotpluggable=yes thread-id='22223' qemu-id='3' type='host-spapr-cpu-core' @@ -47,16 +81,30 @@ qom_path='/machine/peripheral/vcpu1' topology: core='16' vcpus='8' [vcpu libvirt-id='17'] + online=yes + hotpluggable=yes thread-id='22224' [vcpu libvirt-id='18'] + online=yes + hotpluggable=yes thread-id='22225' [vcpu libvirt-id='19'] + online=yes + hotpluggable=yes thread-id='22226' [vcpu libvirt-id='20'] + online=yes + hotpluggable=yes thread-id='22227' [vcpu libvirt-id='21'] + online=yes + hotpluggable=yes thread-id='22228' [vcpu libvirt-id='22'] + online=yes + hotpluggable=yes thread-id='22229' [vcpu libvirt-id='23'] + online=yes + hotpluggable=yes thread-id='22230' diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data index 22a425d..d2c56ef 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-hotplug-4.data @@ -1,24 +1,42 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='21925' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='8' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='21926' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='21927' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='21928' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='21930' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='21931' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='21932' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='21933' [vcpu libvirt-id='8'] + online=yes + hotpluggable=yes thread-id='23170' qemu-id='3' type='host-spapr-cpu-core' @@ -26,20 +44,36 @@ qom_path='/machine/peripheral/vcpu0' topology: core='8' vcpus='8' [vcpu libvirt-id='9'] + online=yes + hotpluggable=yes thread-id='23171' [vcpu libvirt-id='10'] + online=yes + hotpluggable=yes thread-id='23172' [vcpu libvirt-id='11'] + online=yes + hotpluggable=yes thread-id='23173' [vcpu libvirt-id='12'] + online=yes + hotpluggable=yes thread-id='23174' [vcpu libvirt-id='13'] + online=yes + hotpluggable=yes thread-id='23175' [vcpu libvirt-id='14'] + online=yes + hotpluggable=yes thread-id='23176' [vcpu libvirt-id='15'] + online=yes + hotpluggable=yes thread-id='23177' [vcpu libvirt-id='16'] + online=yes + hotpluggable=yes thread-id='22741' qemu-id='2' type='host-spapr-cpu-core' @@ -47,16 +81,30 @@ qom_path='/machine/peripheral/vcpu1' topology: core='16' vcpus='8' [vcpu libvirt-id='17'] + online=yes + hotpluggable=yes thread-id='22742' [vcpu libvirt-id='18'] + online=yes + hotpluggable=yes thread-id='22743' [vcpu libvirt-id='19'] + online=yes + hotpluggable=yes thread-id='22744' [vcpu libvirt-id='20'] + online=yes + hotpluggable=yes thread-id='22745' [vcpu libvirt-id='21'] + online=yes + hotpluggable=yes thread-id='22746' [vcpu libvirt-id='22'] + online=yes + hotpluggable=yes thread-id='22747' [vcpu libvirt-id='23'] + online=yes + hotpluggable=yes thread-id='22748' diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data index d7ab77b..c2f541b 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads.data @@ -1,72 +1,104 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='35232' qemu-id='1' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[1]' topology: core='0' vcpus='1' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='35233' qemu-id='2' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[2]' topology: core='8' vcpus='1' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='35234' qemu-id='3' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[3]' topology: core='16' vcpus='1' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='35235' qemu-id='4' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[4]' topology: core='24' vcpus='1' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='35236' qemu-id='5' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[5]' topology: core='32' vcpus='1' [vcpu libvirt-id='5'] + online=yes + hotpluggable=no thread-id='35237' qemu-id='6' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[6]' topology: core='40' vcpus='1' [vcpu libvirt-id='6'] + online=yes + hotpluggable=no thread-id='35238' qemu-id='7' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[7]' topology: core='48' vcpus='1' [vcpu libvirt-id='7'] + online=yes + hotpluggable=no thread-id='35239' qemu-id='8' type='host-spapr-cpu-core' qom_path='/machine/unattached/device[8]' topology: core='56' vcpus='1' [vcpu libvirt-id='8'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='64' vcpus='1' [vcpu libvirt-id='9'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='72' vcpus='1' [vcpu libvirt-id='10'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='80' vcpus='1' [vcpu libvirt-id='11'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='88' vcpus='1' [vcpu libvirt-id='12'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='96' vcpus='1' [vcpu libvirt-id='13'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='104' vcpus='1' [vcpu libvirt-id='14'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='112' vcpus='1' [vcpu libvirt-id='15'] + online=no + hotpluggable=yes type='host-spapr-cpu-core' topology: core='120' vcpus='1' diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data index a367a09..67dfc01 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data @@ -1,39 +1,55 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='518291' qemu-id='1' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[0]' topology: socket='0' core='0' thread='0' vcpus='1' [vcpu libvirt-id='1'] + online=yes + hotpluggable=no thread-id='518292' qemu-id='2' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[2]' topology: socket='0' core='0' thread='1' vcpus='1' [vcpu libvirt-id='2'] + online=yes + hotpluggable=no thread-id='518294' qemu-id='3' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[3]' topology: socket='0' core='1' thread='0' vcpus='1' [vcpu libvirt-id='3'] + online=yes + hotpluggable=no thread-id='518295' qemu-id='4' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[4]' topology: socket='0' core='1' thread='1' vcpus='1' [vcpu libvirt-id='4'] + online=yes + hotpluggable=no thread-id='518296' qemu-id='5' type='qemu64-x86_64-cpu' qom_path='/machine/unattached/device[5]' topology: socket='1' core='0' thread='0' vcpus='1' [vcpu libvirt-id='5'] + online=no + hotpluggable=yes type='qemu64-x86_64-cpu' topology: socket='1' core='0' thread='1' vcpus='1' [vcpu libvirt-id='6'] + online=no + hotpluggable=yes type='qemu64-x86_64-cpu' topology: socket='1' core='1' thread='0' vcpus='1' [vcpu libvirt-id='7'] + online=no + hotpluggable=yes type='qemu64-x86_64-cpu' topology: socket='1' core='1' thread='1' vcpus='1' diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data index a6c1069..dba3745 100644 --- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data +++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data @@ -1,10 +1,14 @@ [vcpu libvirt-id='0'] + online=yes + hotpluggable=no thread-id='895040' qemu-id='1' type='Broadwell-x86_64-cpu' qom_path='/machine/unattached/device[0]' topology: socket='0' core='0' thread='0' vcpus='1' [vcpu libvirt-id='1'] + online=yes + hotpluggable=yes thread-id='895056' qemu-id='2' type='Broadwell-x86_64-cpu' @@ -12,6 +16,8 @@ qom_path='/machine/peripheral/vcpu1' topology: socket='1' core='0' thread='0' vcpus='1' [vcpu libvirt-id='2'] + online=yes + hotpluggable=yes thread-id='895057' qemu-id='3' type='Broadwell-x86_64-cpu' @@ -19,6 +25,8 @@ qom_path='/machine/peripheral/vcpu2' topology: socket='2' core='0' thread='0' vcpus='1' [vcpu libvirt-id='3'] + online=yes + hotpluggable=yes thread-id='895058' qemu-id='4' type='Broadwell-x86_64-cpu' @@ -26,6 +34,8 @@ qom_path='/machine/peripheral/vcpu3' topology: socket='3' core='0' thread='0' vcpus='1' [vcpu libvirt-id='4'] + online=yes + hotpluggable=yes thread-id='895059' qemu-id='5' type='Broadwell-x86_64-cpu' @@ -33,6 +43,8 @@ qom_path='/machine/peripheral/vcpu4' topology: socket='4' core='0' thread='0' vcpus='1' [vcpu libvirt-id='5'] + online=yes + hotpluggable=yes thread-id='895060' qemu-id='6' type='Broadwell-x86_64-cpu' @@ -40,6 +52,8 @@ qom_path='/machine/peripheral/vcpu5' topology: socket='5' core='0' thread='0' vcpus='1' [vcpu libvirt-id='6'] + online=yes + hotpluggable=yes thread-id='895061' qemu-id='7' type='Broadwell-x86_64-cpu' @@ -47,6 +61,8 @@ qom_path='/machine/peripheral/vcpu6' topology: socket='6' core='0' thread='0' vcpus='1' [vcpu libvirt-id='7'] + online=yes + hotpluggable=yes thread-id='895062' qemu-id='8' type='Broadwell-x86_64-cpu' @@ -54,6 +70,8 @@ qom_path='/machine/peripheral/vcpu7' topology: socket='7' core='0' thread='0' vcpus='1' [vcpu libvirt-id='8'] + online=yes + hotpluggable=yes thread-id='895063' qemu-id='9' type='Broadwell-x86_64-cpu' @@ -61,6 +79,8 @@ qom_path='/machine/peripheral/vcpu8' topology: socket='8' core='0' thread='0' vcpus='1' [vcpu libvirt-id='9'] + online=yes + hotpluggable=yes thread-id='895064' qemu-id='10' type='Broadwell-x86_64-cpu' @@ -68,6 +88,8 @@ qom_path='/machine/peripheral/vcpu9' topology: socket='9' core='0' thread='0' vcpus='1' [vcpu libvirt-id='10'] + online=yes + hotpluggable=yes thread-id='895065' qemu-id='11' type='Broadwell-x86_64-cpu' diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index d3ec3b1..9e195d7 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2357,6 +2357,9 @@ testQemuMonitorCPUInfoFormat(qemuMonitorCPUInfoPtr vcpus, virBufferAsprintf(&buf, "[vcpu libvirt-id='%zu']\n", i); virBufferAdjustIndent(&buf, 4);
+ virBufferAsprintf(&buf, "online=%s\n", vcpu->online ? "yes" : "no"); + virBufferAsprintf(&buf, "hotpluggable=%s\n", vcpu->hotpluggable ? "yes" : "no"); + if (vcpu->tid) virBufferAsprintf(&buf, "thread-id='%llu'\n", (unsigned long long) vcpu->tid);

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

At 09/14/2016 11:35 AM, Peter Krempa wrote:
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
Yes, I see. I confused it with the id feature in QEmu device_add command. Sorry for trouble you. By the way, as far as I know, the online/offline usually refer to the status of a CPU. Before we use the status, the CPU has already existed. Here, we use the online/offline to represent qom_path which indicates that the CPU is existed or not. So, it causes me do that judgment. :) Thanks, Dou
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

On Tue, Sep 13, 2016 at 06:27:45PM +0200, 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;
This could be merged together: vcpus[mastervcpu].hotpluggable = !!hotplugvcpus[i].alias || !vcpus[mastervcpu].online ACK Pavel

Use the state information (online, hotpluggable) provided by the monitor code rather than trying to infer it. This fixes a issue where on architectures that require hotplug of multiple threads at once the sub-cores would get updated as offline on daemon restart thus creating an invalid configuration. --- src/qemu/qemu_domain.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fb766d0..3f16dbe 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5941,15 +5941,11 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver, vcpupriv->enable_id = info[i].id; if (hotplug && state) { - vcpu->online = !!info[i].qom_path; - - /* mark cpus that don't have an alias as non-hotpluggable */ - if (vcpu->online) { - if (vcpupriv->alias) - vcpu->hotpluggable = VIR_TRISTATE_BOOL_YES; - else - vcpu->hotpluggable = VIR_TRISTATE_BOOL_NO; - } + vcpu->online = info[i].online; + if (info[i].hotpluggable) + vcpu->hotpluggable = VIR_TRISTATE_BOOL_YES; + else + vcpu->hotpluggable = VIR_TRISTATE_BOOL_NO; } } -- 2.10.0

On Tue, Sep 13, 2016 at 06:27:46PM +0200, Peter Krempa wrote:
Use the state information (online, hotpluggable) provided by the monitor code rather than trying to infer it. This fixes a issue where on
s/a issue/an issue/
architectures that require hotplug of multiple threads at once the sub-cores would get updated as offline on daemon restart thus creating an invalid configuration. --- src/qemu/qemu_domain.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
ACK Pavel
participants (3)
-
Dou Liyang
-
Pavel Hrdina
-
Peter Krempa