[Qemu-devel] [PATCH 3/3] pc: Don't make CPU properties mandatory unless necessary

[Eduardo Habkost <ehabkost@redhat.com>]

We have this issue reported when using libvirt to hotplug CPUs:
https://bugzilla.redhat.com/show_bug.cgi?id=1741451

Basically, libvirt is not copying die-id from
query-hotpluggable-cpus, but die-id is now mandatory.

We could blame libvirt and say it is not following the documented
interface, because we have this buried in the QAPI schema
documentation:

> Note: currently there are 5 properties that could be present
> but management should be prepared to pass through other
> properties with device_add command to allow for future
> interface extension. This also requires the filed names to be kept in
> sync with the properties passed to -device/device_add.

But I don't think this would be reasonable from us.  We can just
make QEMU more flexible and let CPU properties to be omitted when
there's no ambiguity.  This will allow us to keep compatibility
with existing libvirt versions.

Test case included to ensure we don't break this again.

Cc: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c                             | 17 +++++++
 tests/acceptance/pc_cpu_hotplug_props.py | 59 ++++++++++++++++++++++++
 2 files changed, 76 insertions(+)
 create mode 100644 tests/acceptance/pc_cpu_hotplug_props.py

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fb4ac5ca90..4d773c862d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2403,6 +2403,23 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         int max_socket = (ms->smp.max_cpus - 1) /
                                 smp_threads / smp_cores / pcms->smp_dies;
 
+        /*
+         * If there's only one possible value for a topology property,
+         * allow it to be omitted.
+         */
+        if (cpu->socket_id < 0 && max_socket == 0) {
+            cpu->socket_id = 0;
+        }
+        if (cpu->die_id < 0 && pcms->smp_dies == 1) {
+            cpu->die_id = 0;
+        }
+        if (cpu->core_id < 0 && smp_cores == 1) {
+            cpu->core_id = 0;
+        }
+        if (cpu->thread_id < 0 && smp_threads == 1) {
+            cpu->thread_id = 0;
+        }
+
         if (cpu->socket_id < 0) {
             error_setg(errp, "CPU socket-id is not set");
             return;
diff --git a/tests/acceptance/pc_cpu_hotplug_props.py b/tests/acceptance/pc_cpu_hotplug_props.py
new file mode 100644
index 0000000000..2c36926214
--- /dev/null
+++ b/tests/acceptance/pc_cpu_hotplug_props.py
@@ -0,0 +1,59 @@
+#
+# Ensure CPU topology parameters may be omitted on -device
+#
+#  Copyright (c) 2019 Red Hat Inc
+#
+# Author:
+#  Eduardo Habkost <ehabkost@redhat.com>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, see <http://www.gnu.org/licenses/>.
+#
+
+from avocado_qemu import Test
+
+class OmittedCPUProps(Test):
+    """
+    :avocado: tags=arch:x86_64
+    """
+    def test_only_socket(self):
+        self.vm.add_args('-nodefaults', '-S')
+        self.vm.add_args('-smp', '1,sockets=2,maxcpus=2')
+        self.vm.add_args('-cpu', 'qemu64')
+        self.vm.add_args('-device', 'qemu64-x86_64-cpu,socket-id=1')
+        self.vm.launch()
+        self.assertEquals(len(self.vm.command('query-cpus')), 2)
+
+    def test_only_die(self):
+        self.vm.add_args('-nodefaults', '-S')
+        self.vm.add_args('-smp', '1,dies=2,maxcpus=2')
+        self.vm.add_args('-cpu', 'qemu64')
+        self.vm.add_args('-device', 'qemu64-x86_64-cpu,die-id=1')
+        self.vm.launch()
+        self.assertEquals(len(self.vm.command('query-cpus')), 2)
+
+    def test_only_core(self):
+        self.vm.add_args('-nodefaults', '-S')
+        self.vm.add_args('-smp', '1,cores=2,maxcpus=2')
+        self.vm.add_args('-cpu', 'qemu64')
+        self.vm.add_args('-device', 'qemu64-x86_64-cpu,core-id=1')
+        self.vm.launch()
+        self.assertEquals(len(self.vm.command('query-cpus')), 2)
+
+    def test_only_thread(self):
+        self.vm.add_args('-nodefaults', '-S')
+        self.vm.add_args('-smp', '1,threads=2,maxcpus=2')
+        self.vm.add_args('-cpu', 'qemu64')
+        self.vm.add_args('-device', 'qemu64-x86_64-cpu,thread-id=1')
+        self.vm.launch()
+        self.assertEquals(len(self.vm.command('query-cpus')), 2)
-- 
2.21.0