[Qemu-devel] [PULL 0/9] x86 and machine queue, 2016-07-27

[Qemu-devel] [PULL 0/9] x86 and machine queue, 2016-07-27

[Eduardo Habkost]

The following changes since commit f49ee630d73729ecaeecf4b38a8df11bc613914d:

  Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.7-20160726' into staging (2016-07-26 11:53:47 +0100)

are available in the git repository at:

  git://github.com/ehabkost/qemu.git tags/x86-pull-request

for you to fetch changes up to 03f28efbbb0ee521611e0eb28b45096b3598fb34:

  vl: exit if a bad property value is passed to -global (2016-07-27 11:25:06 -0300)

----------------------------------------------------------------
x86 and machine queue, 2016-07-27

Highlights:
* Fixes to allow CPU hotplug/unplug in any order;
* Exit QEMU on invalid global properties.

----------------------------------------------------------------

Greg Kurz (3):
  machine: Add comment to abort path in machine_set_kernel_irqchip
  qdev: ignore GlobalProperty.errp for hotplugged devices
  vl: exit if a bad property value is passed to -global

Igor Mammedov (6):
  exec: Reduce CONFIG_USER_ONLY ifdeffenery
  exec: Don't use cpu_index to detect if cpu_exec_init()'s been called
  exec: Set cpu_index only if it's not been explictly set
  qdev: Fix object reference leak in case device.realize() fails
  pc: Init CPUState->cpu_index with index in possible_cpus[]
  Revert "pc: Enforce adding CPUs contiguously and removing them in
    opposite order"

 bsd-user/qemu.h           |  2 --
 exec.c                    | 66 +++++++++--------------------------------------
 hw/core/machine.c         |  3 +++
 hw/core/qdev-properties.c |  4 +--
 hw/core/qdev.c            |  8 +++++-
 hw/i386/pc.c              | 38 +++------------------------
 include/exec/exec-all.h   | 12 +++++++++
 include/hw/qdev-core.h    |  4 ++-
 include/qom/cpu.h         |  2 ++
 linux-user/qemu.h         |  2 --
 qom/cpu.c                 |  2 +-
 vl.c                      |  1 +
 12 files changed, 47 insertions(+), 97 deletions(-)

-- 
2.5.5

[Qemu-devel] [PULL 1/9] exec: Reduce CONFIG_USER_ONLY ifdeffenery

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 bsd-user/qemu.h         |  2 --
 exec.c                  | 17 +++--------------
 include/exec/exec-all.h | 12 ++++++++++++
 linux-user/qemu.h       |  2 --
 4 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 6ccc544..2b2b918 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -209,8 +209,6 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
                        abi_ulong new_addr);
 int target_msync(abi_ulong start, abi_ulong len, int flags);
 extern unsigned long last_brk;
-void cpu_list_lock(void);
-void cpu_list_unlock(void);
 #if defined(CONFIG_USE_NPTL)
 void mmap_fork_start(void);
 void mmap_fork_end(int child);
diff --git a/exec.c b/exec.c
index 60cf46a..2f57c62 100644
--- a/exec.c
+++ b/exec.c
@@ -642,23 +642,17 @@ void cpu_exec_exit(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
-#if defined(CONFIG_USER_ONLY)
     cpu_list_lock();
-#endif
     if (cpu->cpu_index == -1) {
         /* cpu_index was never allocated by this @cpu or was already freed. */
-#if defined(CONFIG_USER_ONLY)
         cpu_list_unlock();
-#endif
         return;
     }
 
     QTAILQ_REMOVE(&cpus, cpu, node);
     cpu_release_index(cpu);
     cpu->cpu_index = -1;
-#if defined(CONFIG_USER_ONLY)
     cpu_list_unlock();
-#endif
 
     if (cc->vmsd != NULL) {
         vmstate_unregister(NULL, cc->vmsd, cpu);
@@ -670,7 +664,7 @@ void cpu_exec_exit(CPUState *cpu)
 
 void cpu_exec_init(CPUState *cpu, Error **errp)
 {
-    CPUClass *cc = CPU_GET_CLASS(cpu);
+    CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
     Error *local_err = NULL;
 
     cpu->as = NULL;
@@ -694,22 +688,17 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
     object_ref(OBJECT(cpu->memory));
 #endif
 
-#if defined(CONFIG_USER_ONLY)
     cpu_list_lock();
-#endif
     cpu->cpu_index = cpu_get_free_index(&local_err);
     if (local_err) {
         error_propagate(errp, local_err);
-#if defined(CONFIG_USER_ONLY)
         cpu_list_unlock();
-#endif
         return;
     }
     QTAILQ_INSERT_TAIL(&cpus, cpu, node);
-#if defined(CONFIG_USER_ONLY)
-    (void) cc;
     cpu_list_unlock();
-#else
+
+#ifndef CONFIG_USER_ONLY
     if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
         vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu);
     }
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index acda7b6..d008296 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -56,6 +56,18 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
                               target_ulong pc, target_ulong cs_base,
                               uint32_t flags,
                               int cflags);
+#if defined(CONFIG_USER_ONLY)
+void cpu_list_lock(void);
+void cpu_list_unlock(void);
+#else
+static inline void cpu_list_unlock(void)
+{
+}
+static inline void cpu_list_lock(void)
+{
+}
+#endif
+
 void cpu_exec_init(CPUState *cpu, Error **errp);
 void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
 void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index cdf23a7..bef465d 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -419,8 +419,6 @@ int target_msync(abi_ulong start, abi_ulong len, int flags);
 extern unsigned long last_brk;
 extern abi_ulong mmap_next_start;
 abi_ulong mmap_find_vma(abi_ulong, abi_ulong);
-void cpu_list_lock(void);
-void cpu_list_unlock(void);
 void mmap_fork_start(void);
 void mmap_fork_end(int child);
 
-- 
2.5.5

[Qemu-devel] [PULL 2/9] exec: Don't use cpu_index to detect if cpu_exec_init()'s been called

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

Instead use QTAIL's tqe_prev field to detect if cpu's been
placed in list by cpu_exec_init() which is always set if
QTAIL element is in list.

Fixes SIGSEGV on failure path in case cpu_index is assigned
by board and cpu.relalize() fails before cpu_exec_init() is called.

In follow up patches, cpu_index will be assigned by boards that
support cpu hot(un)plug and need stable cpu_index that doesn't
depend on order cpus are created/removed.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reported-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 exec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 2f57c62..ae45a70 100644
--- a/exec.c
+++ b/exec.c
@@ -643,13 +643,14 @@ void cpu_exec_exit(CPUState *cpu)
     CPUClass *cc = CPU_GET_CLASS(cpu);
 
     cpu_list_lock();
-    if (cpu->cpu_index == -1) {
-        /* cpu_index was never allocated by this @cpu or was already freed. */
+    if (cpu->node.tqe_prev == NULL) {
+        /* there is nothing to undo since cpu_exec_init() hasn't been called */
         cpu_list_unlock();
         return;
     }
 
     QTAILQ_REMOVE(&cpus, cpu, node);
+    cpu->node.tqe_prev = NULL;
     cpu_release_index(cpu);
     cpu->cpu_index = -1;
     cpu_list_unlock();
-- 
2.5.5

[Qemu-devel] [PULL 3/9] exec: Set cpu_index only if it's not been explictly set

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

It keeps the legacy behavior for all users that doesn't care
about stable cpu_index value, but would allow boards that
would support device_add/device_del to set stable cpu_index
that won't depend on order in which cpus are created/destroyed.

While at that simplify cpu_get_free_index() as cpu_index
generated by USER_ONLY and softmmu variants is the same
since none of the users support cpu-remove so far, except
of not yet released spapr/x86 device_add/delr, which
will be altered by follow up patches to set stable
cpu_index manually.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 exec.c            | 44 ++++++--------------------------------------
 include/qom/cpu.h |  2 ++
 qom/cpu.c         |  2 +-
 3 files changed, 9 insertions(+), 39 deletions(-)

diff --git a/exec.c b/exec.c
index ae45a70..50e3ee2 100644
--- a/exec.c
+++ b/exec.c
@@ -598,30 +598,7 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
 }
 #endif
 
-#ifndef CONFIG_USER_ONLY
-static DECLARE_BITMAP(cpu_index_map, MAX_CPUMASK_BITS);
-
-static int cpu_get_free_index(Error **errp)
-{
-    int cpu = find_first_zero_bit(cpu_index_map, MAX_CPUMASK_BITS);
-
-    if (cpu >= MAX_CPUMASK_BITS) {
-        error_setg(errp, "Trying to use more CPUs than max of %d",
-                   MAX_CPUMASK_BITS);
-        return -1;
-    }
-
-    bitmap_set(cpu_index_map, cpu, 1);
-    return cpu;
-}
-
-static void cpu_release_index(CPUState *cpu)
-{
-    bitmap_clear(cpu_index_map, cpu->cpu_index, 1);
-}
-#else
-
-static int cpu_get_free_index(Error **errp)
+static int cpu_get_free_index(void)
 {
     CPUState *some_cpu;
     int cpu_index = 0;
@@ -632,12 +609,6 @@ static int cpu_get_free_index(Error **errp)
     return cpu_index;
 }
 
-static void cpu_release_index(CPUState *cpu)
-{
-    return;
-}
-#endif
-
 void cpu_exec_exit(CPUState *cpu)
 {
     CPUClass *cc = CPU_GET_CLASS(cpu);
@@ -651,8 +622,7 @@ void cpu_exec_exit(CPUState *cpu)
 
     QTAILQ_REMOVE(&cpus, cpu, node);
     cpu->node.tqe_prev = NULL;
-    cpu_release_index(cpu);
-    cpu->cpu_index = -1;
+    cpu->cpu_index = UNASSIGNED_CPU_INDEX;
     cpu_list_unlock();
 
     if (cc->vmsd != NULL) {
@@ -666,7 +636,7 @@ void cpu_exec_exit(CPUState *cpu)
 void cpu_exec_init(CPUState *cpu, Error **errp)
 {
     CPUClass *cc ATTRIBUTE_UNUSED = CPU_GET_CLASS(cpu);
-    Error *local_err = NULL;
+    Error *local_err ATTRIBUTE_UNUSED = NULL;
 
     cpu->as = NULL;
     cpu->num_ases = 0;
@@ -690,11 +660,9 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
 #endif
 
     cpu_list_lock();
-    cpu->cpu_index = cpu_get_free_index(&local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        cpu_list_unlock();
-        return;
+    if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
+        cpu->cpu_index = cpu_get_free_index();
+        assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
     }
     QTAILQ_INSERT_TAIL(&cpus, cpu, node);
     cpu_list_unlock();
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index cbcd64c..ce0c406 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -883,4 +883,6 @@ extern const struct VMStateDescription vmstate_cpu_common;
     .offset = 0,                                                            \
 }
 
+#define UNASSIGNED_CPU_INDEX -1
+
 #endif
diff --git a/qom/cpu.c b/qom/cpu.c
index 42b5631..2553247 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -340,7 +340,7 @@ static void cpu_common_initfn(Object *obj)
     CPUState *cpu = CPU(obj);
     CPUClass *cc = CPU_GET_CLASS(obj);
 
-    cpu->cpu_index = -1;
+    cpu->cpu_index = UNASSIGNED_CPU_INDEX;
     cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
     qemu_mutex_init(&cpu->work_mutex);
     QTAILQ_INIT(&cpu->breakpoints);
-- 
2.5.5

[Qemu-devel] [PULL 4/9] qdev: Fix object reference leak in case device.realize() fails

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

If device doesn't have parent assined before its realize
is called, device_set_realized() will implicitly set parent
to '/machine/unattached'.

However device_set_realized() may fail after that point at
several other points leaving not realized object dangling
in '/machine/unattached' and as result caller of

  obj = object_new()
    obj->ref == 1
  object_property_set_bool(obj,..., true, "realized",...)
    obj->ref == 2
  if (fail)
      object_unref(obj);
      obj->ref == 1

will get object leak instead of expected object destruction.

Fix it by making device_set_realized() to cleanup after itself
in case of failure.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/qdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6680089..ee4a083 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -885,6 +885,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
     HotplugHandler *hotplug_ctrl;
     BusState *bus;
     Error *local_err = NULL;
+    bool unattached_parent = false;
+    static int unattached_count;
 
     if (dev->hotplugged && !dc->hotpluggable) {
         error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
@@ -893,12 +895,12 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
 
     if (value && !dev->realized) {
         if (!obj->parent) {
-            static int unattached_count;
             gchar *name = g_strdup_printf("device[%d]", unattached_count++);
 
             object_property_add_child(container_get(qdev_get_machine(),
                                                     "/unattached"),
                                       name, obj, &error_abort);
+            unattached_parent = true;
             g_free(name);
         }
 
@@ -987,6 +989,10 @@ post_realize_fail:
 
 fail:
     error_propagate(errp, local_err);
+    if (unattached_parent) {
+        object_unparent(OBJECT(dev));
+        unattached_count--;
+    }
 }
 
 static bool device_get_hotpluggable(Object *obj, Error **errp)
-- 
2.5.5

[Qemu-devel] [PULL 5/9] pc: Init CPUState->cpu_index with index in possible_cpus[]

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

It will enshure that cpu_index for a given cpu stays the same
regardless of the order cpus has been created/deleted.

No compat code is needed as for initial cpus index in
possible_cpus[] matches cpu_index that's been auto-allocated
in cpu_exec_init().

Tha same applies for hotplug with cpu-add command if cpus are
added sequentially in increasing order as 'id' matches cpu_index.

If cpu-add had been used for creating out-of-order cpus,
that created unmigratable instance since it were not possible
to start target with the same cpu_index using old way
of migrating instance with hotplugged cpus:

* source QEMU with CLI (-smp 1,maxcpus=3 and cpu-add id=2)
  following set of cpu_index is allocated [0, 1] with
  apics set [0, 2] respectivelly
* target QEMU is started with CLI -smp 2,maxcpus=3
  resulting in set of cpu_index [0, 1] but with
  set of apics [0, 1] wich doesn't match source.

So we don't need compat code in this case as it's never worked
and newelly added device_add support would use stable cpu_index
set by machine to begin with, so it won't have above limitation
and source QEMU could be migrated to destination regardless
of the order cpus were created.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9e3c70f..d6f0347 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1875,6 +1875,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
                             DeviceState *dev, Error **errp)
 {
     int idx;
+    CPUState *cs;
     CPUArchId *cpu_slot;
     X86CPUTopoInfo topo;
     X86CPU *cpu = X86_CPU(dev);
@@ -1975,6 +1976,9 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         return;
     }
     cpu->thread_id = topo.smt_id;
+
+    cs = CPU(cpu);
+    cs->cpu_index = idx;
 }
 
 static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
-- 
2.5.5

[Qemu-devel] [PULL 6/9] Revert "pc: Enforce adding CPUs contiguously and removing them in opposite order"

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

This reverts commit 4da7faaeb0c7dd3f7f233165d336c878f78fd1eb.

Since commit:
  pc: init CPUState->cpu_index with index in possible_cpus[]
cpu_index is stable regardless of the order cpus were created
and QEMU instance stays migratable always so limitation added
by 4da7faaeb could be safely removed.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/pc.c | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d6f0347..47593b7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1818,23 +1818,6 @@ static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
         goto out;
     }
 
-    if (idx < pcms->possible_cpus->len - 1 &&
-        pcms->possible_cpus->cpus[idx + 1].cpu != NULL) {
-        X86CPU *cpu;
-
-        for (idx = pcms->possible_cpus->len - 1;
-             pcms->possible_cpus->cpus[idx].cpu == NULL; idx--) {
-            ;;
-        }
-
-        cpu = X86_CPU(pcms->possible_cpus->cpus[idx].cpu);
-        error_setg(&local_err, "CPU [socket-id: %u, core-id: %u,"
-                   " thread-id: %u] should be removed first",
-                   cpu->socket_id, cpu->core_id, cpu->thread_id);
-        goto out;
-
-    }
-
     hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev);
     hhc->unplug_request(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
 
@@ -1932,23 +1915,6 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
         return;
     }
 
-    if (idx != 0 && pcms->possible_cpus->cpus[idx - 1].cpu == NULL) {
-        PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
-
-        for (idx = 1; pcms->possible_cpus->cpus[idx].cpu != NULL; idx++) {
-            ;;
-        }
-
-        x86_topo_ids_from_apicid(pcms->possible_cpus->cpus[idx].arch_id,
-                                 smp_cores, smp_threads, &topo);
-
-        if (!pcmc->legacy_cpu_hotplug) {
-            error_setg(errp, "CPU [socket: %u, core: %u, thread: %u] should be"
-                       " added first", topo.pkg_id, topo.core_id, topo.smt_id);
-            return;
-        }
-    }
-
     /* if 'address' properties socket-id/core-id/thread-id are not set, set them
      * so that query_hotpluggable_cpus would show correct values
      */
-- 
2.5.5

[Qemu-devel] [PULL 7/9] machine: Add comment to abort path in machine_set_kernel_irqchip

[Eduardo Habkost]

From: Greg Kurz <groug@kaod.org>

We're not supposed to abort when the user passes a bogus value.
Since the checking is done in visit_type_OnOffSplit(), the call
to abort() is legitimate. Let's add a comment to make it
explicit.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/machine.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2fe6ff6..e5a456f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -65,6 +65,9 @@ static void machine_set_kernel_irqchip(Object *obj, Visitor *v,
             ms->kernel_irqchip_split = true;
             break;
         default:
+            /* The value was checked in visit_type_OnOffSplit() above. If
+             * we get here, then something is wrong in QEMU.
+             */
             abort();
         }
     }
-- 
2.5.5

[Qemu-devel] [PULL 8/9] qdev: ignore GlobalProperty.errp for hotplugged devices

[Eduardo Habkost]

From: Greg Kurz <groug@kaod.org>

This patch ensures QEMU won't terminate while hotplugging a device if the
global property cannot be set and errp points to error_fatal or error_abort.

While here, it also fixes indentation of the typename argument.

Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/qdev-properties.c | 4 ++--
 include/hw/qdev-core.h    | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 14e544a..311af6d 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1084,7 +1084,7 @@ int qdev_prop_check_globals(void)
 }
 
 static void qdev_prop_set_globals_for_type(DeviceState *dev,
-                                const char *typename)
+                                           const char *typename)
 {
     GList *l;
 
@@ -1100,7 +1100,7 @@ static void qdev_prop_set_globals_for_type(DeviceState *dev,
         if (err != NULL) {
             error_prepend(&err, "can't apply global %s.%s=%s: ",
                           prop->driver, prop->property, prop->value);
-            if (prop->errp) {
+            if (!dev->hotplugged && prop->errp) {
                 error_propagate(prop->errp, err);
             } else {
                 assert(prop->user_provided);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 1d1f861..4b4b33b 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -261,7 +261,9 @@ struct PropertyInfo {
  * @used: Set to true if property was used when initializing a device.
  * @errp: Error destination, used like first argument of error_setg()
  *        in case property setting fails later. If @errp is NULL, we
- *        print warnings instead of ignoring errors silently.
+ *        print warnings instead of ignoring errors silently. For
+ *        hotplugged devices, errp is always ignored and warnings are
+ *        printed instead.
  */
 typedef struct GlobalProperty {
     const char *driver;
-- 
2.5.5

[Qemu-devel] [PULL 9/9] vl: exit if a bad property value is passed to -global

[Eduardo Habkost]

From: Greg Kurz <groug@kaod.org>

When passing '-global driver=host-powerpc64-cpu,property=compat,value=foo'
on the command line, without this patch, we get the following warning per
device (which means many lines if the guests has many cpus):

qemu-system-ppc64: Warning: can't apply global host-powerpc64-cpu.compat=foo:
    Invalid compatibility mode "foo"

... and QEMU continues execution, ignoring the property.

With this patch, we get a single line:

qemu-system-ppc64: can't apply global host-powerpc64-cpu.compat=foo:
    Invalid compatibility mode "foo"

... and QEMU exits.

The previous behavior is kept for hotplugged devices since we don't want
QEMU to exit when doing device_add.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 vl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/vl.c b/vl.c
index a455947..e7c2c62 100644
--- a/vl.c
+++ b/vl.c
@@ -2922,6 +2922,7 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
     g->property = qemu_opt_get(opts, "property");
     g->value    = qemu_opt_get(opts, "value");
     g->user_provided = true;
+    g->errp = &error_fatal;
     qdev_prop_register_global(g);
     return 0;
 }
-- 
2.5.5

Re: [Qemu-devel] [PULL 0/9] x86 and machine queue, 2016-07-27

[Peter Maydell]

On 27 July 2016 at 15:29, Eduardo Habkost <ehabkost@redhat.com> wrote:
> The following changes since commit f49ee630d73729ecaeecf4b38a8df11bc613914d:
>
>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.7-20160726' into staging (2016-07-26 11:53:47 +0100)
>
> are available in the git repository at:
>
>   git://github.com/ehabkost/qemu.git tags/x86-pull-request
>
> for you to fetch changes up to 03f28efbbb0ee521611e0eb28b45096b3598fb34:
>
>   vl: exit if a bad property value is passed to -global (2016-07-27 11:25:06 -0300)
>
> ----------------------------------------------------------------
> x86 and machine queue, 2016-07-27
>
> Highlights:
> * Fixes to allow CPU hotplug/unplug in any order;
> * Exit QEMU on invalid global properties.
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM