All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduardo Habkost <ehabkost@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>,
	qemu-devel@nongnu.org,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Richard Henderson <rth@twiddle.net>,
	Peter Maydell <peter.maydell@linaro.org>
Subject: [Qemu-devel] [PULL v6 09/12] pc: Don't make die-id mandatory unless necessary
Date: Wed, 28 Aug 2019 15:40:23 -0300	[thread overview]
Message-ID: <20190828184026.5840-10-ehabkost@redhat.com> (raw)
In-Reply-To: <20190828184026.5840-1-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 die-id 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.

Fixes: commit 176d2cda0dee ("i386/cpu: Consolidate die-id validity in smp context")
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <20190816170750.23910-1-ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c                             |  8 ++++++
 tests/acceptance/pc_cpu_hotplug_props.py | 35 ++++++++++++++++++++++++
 2 files changed, 43 insertions(+)
 create mode 100644 tests/acceptance/pc_cpu_hotplug_props.py

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3494423d63..c7200b0b54 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2421,6 +2421,14 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         int max_socket = (ms->smp.max_cpus - 1) /
                                 smp_threads / smp_cores / pcms->smp_dies;
 
+        /*
+         * die-id was optional in QEMU 4.0 and older, so keep it optional
+         * if there's only one die per socket.
+         */
+        if (cpu->die_id < 0 && pcms->smp_dies == 1) {
+            cpu->die_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..08b7e632c6
--- /dev/null
+++ b/tests/acceptance/pc_cpu_hotplug_props.py
@@ -0,0 +1,35 @@
+#
+# Ensure CPU die-id can 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_no_die_id(self):
+        self.vm.add_args('-nodefaults', '-S')
+        self.vm.add_args('-smp', '1,sockets=2,cores=2,threads=2,maxcpus=8')
+        self.vm.add_args('-cpu', 'qemu64')
+        self.vm.add_args('-device', 'qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0')
+        self.vm.launch()
+        self.assertEquals(len(self.vm.command('query-cpus')), 2)
-- 
2.21.0



  parent reply	other threads:[~2019-08-28 18:45 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-28 18:40 [Qemu-devel] [PULL v6 00/12] Machine + x86 queue, 2019-08-28 Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 01/12] includes: remove stale [smp|max]_cpus externs Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 02/12] hw/arm: simplify arm_load_dtb Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 03/12] numa: move numa global variable nb_numa_nodes into MachineState Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 04/12] numa: move numa global variable have_numa_distance " Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 05/12] numa: move numa global variable numa_info " Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 06/12] numa: Introduce MachineClass::auto_enable_numa for implicit NUMA node Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 07/12] pc: Fix error message on die-id validation Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 08/12] pc: Improve error message when die-id is omitted Eduardo Habkost
2019-08-28 18:40 ` Eduardo Habkost [this message]
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 10/12] qapi: report the default CPU type for each machine Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 11/12] hostmem-file: fix pmem file size check Eduardo Habkost
2019-08-28 18:40 ` [Qemu-devel] [PULL v6 12/12] i386/vmmouse: Properly reset state Eduardo Habkost
2019-09-03 15:03 ` [Qemu-devel] [PULL v6 00/12] Machine + x86 queue, 2019-08-28 Peter Maydell
2019-09-03 19:27   ` Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190828184026.5840-10-ehabkost@redhat.com \
    --to=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.