Bug 1452053

Summary:	libvirt should pass node-id property if it's reported by qemu when hotplugging cpu
Product:	Red Hat Enterprise Linux 7	Reporter:	Igor Mammedov <imammedo>
Component:	libvirt	Assignee:	Peter Krempa <pkrempa>
Status:	CLOSED ERRATA	QA Contact:	Luyao Huang <lhuang>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	7.5	CC:	dyuan, rbalakri, xuzhang, yalzhang
Target Milestone:	rc
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	libvirt-3.7.0-1.el7	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2018-04-10 10:44:33 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Igor Mammedov 2017-05-18 09:00:34 UTC

Description of problem:

QMP command query-hotpluggable-cpus returns a list of possible to hotplug CPUs
and corresponding properties that should go in device_add command to hotplug
a cpu.

Libvirt were supposed to passthrough ALL properties from
query-hotpluggable-cpus to device_add command, however it only passes
socket-id/core-id/thread-id properties and drops other.

query-hotpluggable-cpus since 2.10 adds node-id to list of properties needed to hotplug cpu if NUMA mode is enabled on CLI, but due to above bug libvirt doesn't pass it to device_add command.
Which forced QEMU to include workaround and fixup missed node-id from somewhere else at pre_plug time.

Libvirt should fix logic of qemuBuildHotpluggableCPUProps() and pass all properties that query-hotpluggable-cpus reported even if it doesn't know/care about them (as it's been intended originally) so that in future qemu won't have to include similar worarounds for broken interface contract when a new property is added to list.

Version-Release number of selected component (if applicable):
upstream v3.2.0

How reproducible:
100%

Steps to Reproduce:
x86/qemu-2.10 -numa node -numa node -smp 1,maxcpus=2 ...

Actual results:
QMP command query-hotpluggable-cpus reports node-id properties for each CPU
but 'device_add' for second cpu doesn't include it. 

Expected results:
libvirt issues device_add should include all properties that were reported by
query-hotpluggable-cpus

Comment 2 Peter Krempa 2017-06-27 14:28:07 UTC

Proposed patch upstream:

https://www.redhat.com/archives/libvir-list/2017-June/msg01189.html

Comment 3 Peter Krempa 2017-07-10 11:25:33 UTC

Fixed upstream:

commit ccac446545c18a4c50fb6cba77a1a1dae94fd339
Author: Peter Krempa <pkrempa>
Date:   Tue Jun 27 16:04:38 2017 +0200

    qemu: domain: Use vcpu 'node-id' property and pass it back to qemu
    
    vcpu properties gathered from query-hotpluggable cpus need to be passed
    back to qemu. As qemu did not use the node-id property until now and
    libvirt forgot to pass it back properly (it was parsed but not passed
    around) we did not honor this.
    
    This patch adds node-id to the structures where it was missing and
    passes it around as necessary.
    
    The test data was generated with a VM with following config:
        <numa>
          <cell id='0' cpus='0,2,4,6' memory='512000' unit='KiB'/>
          <cell id='1' cpus='1,3,5,7' memory='512000' unit='KiB'/>
        </numa>

Comment 5 Luyao Huang 2017-12-13 08:33:10 UTC

Verify this bug on libvirt-3.9.0-6.el7.x86_64:

1. prepare a guest which have numa node:

  <vcpu placement='static' current='5'>10</vcpu>
...
  <cpu>
    <numa>
      <cell id='0' cpus='0,2,4,6,8' memory='524288' unit='KiB'/>
      <cell id='1' cpus='1,3,5,7,9' memory='524288' unit='KiB'/>
    </numa>
  </cpu>


2. start guest

# virsh start r7-mig
Domain r7-mig started

3. open another terminal to run the stap:

# stap /usr/share/doc/libvirt-docs-3.9.0/systemtap/qemu-monitor.stp

4. hotplug vcpu:

# virsh setvcpus r7-mig 10


5. check qmp command:
1798.319 > 0x7f19a8013ec0 {"execute":"device_add","arguments":{"driver":"qemu64-x86_64-cpu","id":"vcpu5","socket-id":5,"core-id":0,"thread-id":0,"node-id":1},"id":"libvirt-21"}
1798.329 < 0x7f19a8013ec0 {"return": {}, "id": "libvirt-21"}
...
1798.342 > 0x7f19a8013ec0 {"execute":"device_add","arguments":{"driver":"qemu64-x86_64-cpu","id":"vcpu6","socket-id":6,"core-id":0,"thread-id":0,"node-id":0},"id":"libvirt-24"}
1798.350 < 0x7f19a8013ec0 {"return": {}, "id": "libvirt-24"}
...
1798.362 > 0x7f19a8013ec0 {"execute":"device_add","arguments":{"driver":"qemu64-x86_64-cpu","id":"vcpu7","socket-id":7,"core-id":0,"thread-id":0,"node-id":1},"id":"libvirt-27"}
1798.370 < 0x7f19a8013ec0 {"return": {}, "id": "libvirt-27"}
...
1798.381 > 0x7f19a8013ec0 {"execute":"device_add","arguments":{"driver":"qemu64-x86_64-cpu","id":"vcpu8","socket-id":8,"core-id":0,"thread-id":0,"node-id":0},"id":"libvirt-30"}
1798.390 < 0x7f19a8013ec0 {"return": {}, "id": "libvirt-30"}
...
1798.401 > 0x7f19a8013ec0 {"execute":"device_add","arguments":{"driver":"qemu64-x86_64-cpu","id":"vcpu9","socket-id":9,"core-id":0,"thread-id":0,"node-id":1},"id":"libvirt-33"}

Comment 9 errata-xmlrpc 2018-04-10 10:44:33 UTC

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2018:0704