[libvirt] [PULL v2 02/43] vl.c deprecate incorrect CPUs topology

Eduardo Habkost ehabkost at redhat.com
Thu Oct 25 13:32:20 UTC 2018


From: Igor Mammedov <imammedo at redhat.com>

-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
so that total number of logical CPUs [sockets * cores * threads]
would be equal to [maxcpus], however historically we didn't have
such check in QEMU and it is possible to start VM with an invalid
topology.
Deprecate invalid options combination so we can make sure that
the topology VM started with is always correct in the future.
Users with an invalid sockets/cores/threads/maxcpus values should
fix their CLI to make sure that
   [sockets * cores * threads] == [maxcpus]

Signed-off-by: Igor Mammedov <imammedo at redhat.com>
Reviewed-by: Andrew Jones <drjones at redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost at redhat.com>
Message-Id: <1536836762-273036-2-git-send-email-imammedo at redhat.com>
Reviewed-by: Eric Blake <eblake at redhat.com>
[ehabkost: squashed unit test fix]
Message-Id: <20181019215345.521d58d7 at igors-macbook-pro.local>
Signed-off-by: Eduardo Habkost <ehabkost at redhat.com>
---
 tests/cpu-plug-test.c | 18 +++++++++---------
 vl.c                  |  7 +++++++
 qemu-deprecated.texi  | 12 ++++++++++++
 3 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/tests/cpu-plug-test.c b/tests/cpu-plug-test.c
index 3e93c8e096..f4a677d238 100644
--- a/tests/cpu-plug-test.c
+++ b/tests/cpu-plug-test.c
@@ -32,12 +32,12 @@ static void test_plug_with_cpu_add(gconstpointer data)
     unsigned int i;
 
     args = g_strdup_printf("-machine %s -cpu %s "
-                           "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u",
+                           "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u",
                            s->machine, s->cpu_model,
                            s->sockets, s->cores, s->threads, s->maxcpus);
     qtest_start(args);
 
-    for (i = s->sockets * s->cores * s->threads; i < s->maxcpus; i++) {
+    for (i = 1; i < s->maxcpus; i++) {
         response = qmp("{ 'execute': 'cpu-add',"
                        "  'arguments': { 'id': %d } }", i);
         g_assert(response);
@@ -56,7 +56,7 @@ static void test_plug_without_cpu_add(gconstpointer data)
     QDict *response;
 
     args = g_strdup_printf("-machine %s -cpu %s "
-                           "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u",
+                           "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u",
                            s->machine, s->cpu_model,
                            s->sockets, s->cores, s->threads, s->maxcpus);
     qtest_start(args);
@@ -79,12 +79,12 @@ static void test_plug_with_device_add_x86(gconstpointer data)
     unsigned int s, c, t;
 
     args = g_strdup_printf("-machine %s -cpu %s "
-                           "-smp sockets=%u,cores=%u,threads=%u,maxcpus=%u",
+                           "-smp 1,sockets=%u,cores=%u,threads=%u,maxcpus=%u",
                            td->machine, td->cpu_model,
                            td->sockets, td->cores, td->threads, td->maxcpus);
     qtest_start(args);
 
-    for (s = td->sockets; s < td->maxcpus / td->cores / td->threads; s++) {
+    for (s = 1; s < td->sockets; s++) {
         for (c = 0; c < td->cores; c++) {
             for (t = 0; t < td->threads; t++) {
                 char *id = g_strdup_printf("id-%i-%i-%i", s, c, t);
@@ -113,7 +113,7 @@ static void test_plug_with_device_add_coreid(gconstpointer data)
                            td->sockets, td->cores, td->threads, td->maxcpus);
     qtest_start(args);
 
-    for (c = td->cores; c < td->maxcpus / td->sockets / td->threads; c++) {
+    for (c = 1; c < td->cores; c++) {
         char *id = g_strdup_printf("id-%i", c);
         qtest_qmp_device_add(td->device_model, id, "{'core-id':%u}", c);
         g_free(id);
@@ -148,7 +148,7 @@ static void add_pc_test_case(const char *mname)
     data->sockets = 1;
     data->cores = 3;
     data->threads = 2;
-    data->maxcpus = data->sockets * data->cores * data->threads * 2;
+    data->maxcpus = data->sockets * data->cores * data->threads;
     if (g_str_has_suffix(mname, "-1.4") ||
         (strcmp(mname, "pc-1.3") == 0) ||
         (strcmp(mname, "pc-1.2") == 0) ||
@@ -203,7 +203,7 @@ static void add_pseries_test_case(const char *mname)
     data->sockets = 2;
     data->cores = 3;
     data->threads = 1;
-    data->maxcpus = data->sockets * data->cores * data->threads * 2;
+    data->maxcpus = data->sockets * data->cores * data->threads;
 
     path = g_strdup_printf("cpu-plug/%s/device-add/%ux%ux%u&maxcpus=%u",
                            mname, data->sockets, data->cores,
@@ -229,7 +229,7 @@ static void add_s390x_test_case(const char *mname)
     data->sockets = 1;
     data->cores = 3;
     data->threads = 1;
-    data->maxcpus = data->sockets * data->cores * data->threads * 2;
+    data->maxcpus = data->sockets * data->cores * data->threads;
 
     data2 = g_memdup(data, sizeof(PlugTestData));
     data2->machine = g_strdup(data->machine);
diff --git a/vl.c b/vl.c
index b2a405f80f..31febe965c 100644
--- a/vl.c
+++ b/vl.c
@@ -1266,6 +1266,13 @@ static void smp_parse(QemuOpts *opts)
             exit(1);
         }
 
+        if (sockets * cores * threads != max_cpus) {
+            warn_report("Invalid CPU topology deprecated: "
+                        "sockets (%u) * cores (%u) * threads (%u) "
+                        "!= maxcpus (%u)",
+                        sockets, cores, threads, max_cpus);
+        }
+
         smp_cpus = cpus;
         smp_cores = cores;
         smp_threads = threads;
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 11b870c5c1..5d2d7a3588 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -86,6 +86,18 @@ for these file types is 'host_cdrom' or 'host_device' as appropriate.
 The @option{name} parameter of the @option{-net} option is a synonym
 for the @option{id} parameter, which should now be used instead.
 
+ at subsection -smp (invalid topologies) (since 3.1)
+
+CPU topology properties should describe whole machine topology including
+possible CPUs.
+
+However, historically it was possible to start QEMU with an incorrect topology
+where @math{@var{n} <= @var{sockets} * @var{cores} * @var{threads} < @var{maxcpus}},
+which could lead to an incorrect topology enumeration by the guest.
+Support for invalid topologies will be removed, the user must ensure
+topologies described with -smp include all possible cpus, i.e.
+  @math{@var{sockets} * @var{cores} * @var{threads} = @var{maxcpus}}.
+
 @section QEMU Machine Protocol (QMP) commands
 
 @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
-- 
2.18.0.rc1.1.g3f1ff2140




More information about the libvir-list mailing list