[Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Eduardo Habkost]

This series moves some qdev code outside qdev.o, so it can be
compiled only in CONFIG_SOFTMMU.

The code being moved includes two qdev_get_machine() calls, so
this will make it easier to move qdev_get_machine() to
CONFIG_SOFTMMU later.

After this series, there's one remaining qdev_get_machine() call
that seems more difficult to remove:

    static void device_set_realized(Object *obj, bool value, Error **errp)
    {
        /* [...] */
        if (!obj->parent) {
            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);
        }
        /* [...] */
    }

This one is tricky because on system emulation mode it needs
"/machine" to already exist, but in user-only mode it needs to
implicitly create a "/machine" container.

Eduardo Habkost (4):
  machine: Move gpio code to hw/core/gpio.c
  move qdev hotplug code to qdev-hotplug.c
  qdev: Don't compile hotplug code in user-mode emulation
  qdev-hotplug: Don't check type of qdev_get_machine()

 hw/core/bus.c                |  11 --
 hw/core/gpio.c               | 206 ++++++++++++++++++++++++++++++++
 hw/core/qdev-hotplug-stubs.c |  44 +++++++
 hw/core/qdev-hotplug.c       |  64 ++++++++++
 hw/core/qdev.c               | 219 -----------------------------------
 hw/core/Makefile.objs        |   5 +-
 tests/Makefile.include       |   3 +-
 7 files changed, 320 insertions(+), 232 deletions(-)
 create mode 100644 hw/core/gpio.c
 create mode 100644 hw/core/qdev-hotplug-stubs.c
 create mode 100644 hw/core/qdev-hotplug.c

-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Eduardo Habkost]

This series moves some qdev code outside qdev.o, so it can be
compiled only in CONFIG_SOFTMMU.

The code being moved includes two qdev_get_machine() calls, so
this will make it easier to move qdev_get_machine() to
CONFIG_SOFTMMU later.

After this series, there's one remaining qdev_get_machine() call
that seems more difficult to remove:

    static void device_set_realized(Object *obj, bool value, Error **errp)
    {
        /* [...] */
        if (!obj->parent) {
            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);
        }
        /* [...] */
    }

This one is tricky because on system emulation mode it needs
"/machine" to already exist, but in user-only mode it needs to
implicitly create a "/machine" container.

Eduardo Habkost (4):
  machine: Move gpio code to hw/core/gpio.c
  move qdev hotplug code to qdev-hotplug.c
  qdev: Don't compile hotplug code in user-mode emulation
  qdev-hotplug: Don't check type of qdev_get_machine()

 hw/core/bus.c                |  11 --
 hw/core/gpio.c               | 206 ++++++++++++++++++++++++++++++++
 hw/core/qdev-hotplug-stubs.c |  44 +++++++
 hw/core/qdev-hotplug.c       |  64 ++++++++++
 hw/core/qdev.c               | 219 -----------------------------------
 hw/core/Makefile.objs        |   5 +-
 tests/Makefile.include       |   3 +-
 7 files changed, 320 insertions(+), 232 deletions(-)
 create mode 100644 hw/core/gpio.c
 create mode 100644 hw/core/qdev-hotplug-stubs.c
 create mode 100644 hw/core/qdev-hotplug.c

-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PATCH 1/4] machine: Move gpio code to hw/core/gpio.c

[Eduardo Habkost]

Only softmmu code uses gpio, so move gpio code from qdev.c to
gpio.c and compile it only on softmmu mode.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/gpio.c        | 206 ++++++++++++++++++++++++++++++++++++++++++
 hw/core/qdev.c        | 184 -------------------------------------
 hw/core/Makefile.objs |   1 +
 3 files changed, 207 insertions(+), 184 deletions(-)
 create mode 100644 hw/core/gpio.c

diff --git a/hw/core/gpio.c b/hw/core/gpio.c
new file mode 100644
index 0000000000..2724b4d7e7
--- /dev/null
+++ b/hw/core/gpio.c
@@ -0,0 +1,206 @@
+/*
+ * qdev GPIO helpers
+ *
+ *  Copyright (c) 2009 CodeSourcery
+ *
+ * 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/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+#include "qapi/error.h"
+
+static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
+                                               const char *name)
+{
+    NamedGPIOList *ngl;
+
+    QLIST_FOREACH(ngl, &dev->gpios, node) {
+        /* NULL is a valid and matchable name, otherwise do a normal
+         * strcmp match.
+         */
+        if ((!ngl->name && !name) ||
+                (name && ngl->name && strcmp(name, ngl->name) == 0)) {
+            return ngl;
+        }
+    }
+
+    ngl = g_malloc0(sizeof(*ngl));
+    ngl->name = g_strdup(name);
+    QLIST_INSERT_HEAD(&dev->gpios, ngl, node);
+    return ngl;
+}
+
+void qdev_init_gpio_in_named_with_opaque(DeviceState *dev,
+                                         qemu_irq_handler handler,
+                                         void *opaque,
+                                         const char *name, int n)
+{
+    int i;
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(gpio_list->num_out == 0 || !name);
+    gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
+                                     opaque, n);
+
+    if (!name) {
+        name = "unnamed-gpio-in";
+    }
+    for (i = gpio_list->num_in; i < gpio_list->num_in + n; i++) {
+        gchar *propname = g_strdup_printf("%s[%u]", name, i);
+
+        object_property_add_child(OBJECT(dev), propname,
+                                  OBJECT(gpio_list->in[i]), &error_abort);
+        g_free(propname);
+    }
+
+    gpio_list->num_in += n;
+}
+
+void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
+{
+    qdev_init_gpio_in_named(dev, handler, NULL, n);
+}
+
+void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
+                              const char *name, int n)
+{
+    int i;
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(gpio_list->num_in == 0 || !name);
+
+    if (!name) {
+        name = "unnamed-gpio-out";
+    }
+    memset(pins, 0, sizeof(*pins) * n);
+    for (i = 0; i < n; ++i) {
+        gchar *propname = g_strdup_printf("%s[%u]", name,
+                                          gpio_list->num_out + i);
+
+        object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,
+                                 (Object **)&pins[i],
+                                 object_property_allow_set_link,
+                                 OBJ_PROP_LINK_STRONG,
+                                 &error_abort);
+        g_free(propname);
+    }
+    gpio_list->num_out += n;
+}
+
+void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
+{
+    qdev_init_gpio_out_named(dev, pins, NULL, n);
+}
+
+qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(n >= 0 && n < gpio_list->num_in);
+    return gpio_list->in[n];
+}
+
+qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
+{
+    return qdev_get_gpio_in_named(dev, NULL, n);
+}
+
+void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
+                                 qemu_irq pin)
+{
+    char *propname = g_strdup_printf("%s[%d]",
+                                     name ? name : "unnamed-gpio-out", n);
+    if (pin) {
+        /* We need a name for object_property_set_link to work.  If the
+         * object has a parent, object_property_add_child will come back
+         * with an error without doing anything.  If it has none, it will
+         * never fail.  So we can just call it with a NULL Error pointer.
+         */
+        object_property_add_child(container_get(qdev_get_machine(),
+                                                "/unattached"),
+                                  "non-qdev-gpio[*]", OBJECT(pin), NULL);
+    }
+    object_property_set_link(OBJECT(dev), OBJECT(pin), propname, &error_abort);
+    g_free(propname);
+}
+
+qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
+{
+    char *propname = g_strdup_printf("%s[%d]",
+                                     name ? name : "unnamed-gpio-out", n);
+
+    qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
+                                                      NULL);
+
+    return ret;
+}
+
+/* disconnect a GPIO output, returning the disconnected input (if any) */
+
+static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
+                                               const char *name, int n)
+{
+    char *propname = g_strdup_printf("%s[%d]",
+                                     name ? name : "unnamed-gpio-out", n);
+
+    qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
+                                                      NULL);
+    if (ret) {
+        object_property_set_link(OBJECT(dev), NULL, propname, NULL);
+    }
+    g_free(propname);
+    return ret;
+}
+
+qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
+                                 const char *name, int n)
+{
+    qemu_irq disconnected = qdev_disconnect_gpio_out_named(dev, name, n);
+    qdev_connect_gpio_out_named(dev, name, n, icpt);
+    return disconnected;
+}
+
+void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
+{
+    qdev_connect_gpio_out_named(dev, NULL, n, pin);
+}
+
+void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
+                     const char *name)
+{
+    int i;
+    NamedGPIOList *ngl = qdev_get_named_gpio_list(dev, name);
+
+    for (i = 0; i < ngl->num_in; i++) {
+        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-in";
+        char *propname = g_strdup_printf("%s[%d]", nm, i);
+
+        object_property_add_alias(OBJECT(container), propname,
+                                  OBJECT(dev), propname,
+                                  &error_abort);
+        g_free(propname);
+    }
+    for (i = 0; i < ngl->num_out; i++) {
+        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-out";
+        char *propname = g_strdup_printf("%s[%d]", nm, i);
+
+        object_property_add_alias(OBJECT(container), propname,
+                                  OBJECT(dev), propname,
+                                  &error_abort);
+        g_free(propname);
+    }
+    QLIST_REMOVE(ngl, node);
+    QLIST_INSERT_HEAD(&container->gpios, ngl, node);
+}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f9b6efe509..f73e7ded1a 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -346,190 +346,6 @@ BusState *qdev_get_parent_bus(DeviceState *dev)
     return dev->parent_bus;
 }
 
-static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
-                                               const char *name)
-{
-    NamedGPIOList *ngl;
-
-    QLIST_FOREACH(ngl, &dev->gpios, node) {
-        /* NULL is a valid and matchable name, otherwise do a normal
-         * strcmp match.
-         */
-        if ((!ngl->name && !name) ||
-                (name && ngl->name && strcmp(name, ngl->name) == 0)) {
-            return ngl;
-        }
-    }
-
-    ngl = g_malloc0(sizeof(*ngl));
-    ngl->name = g_strdup(name);
-    QLIST_INSERT_HEAD(&dev->gpios, ngl, node);
-    return ngl;
-}
-
-void qdev_init_gpio_in_named_with_opaque(DeviceState *dev,
-                                         qemu_irq_handler handler,
-                                         void *opaque,
-                                         const char *name, int n)
-{
-    int i;
-    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
-
-    assert(gpio_list->num_out == 0 || !name);
-    gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
-                                     opaque, n);
-
-    if (!name) {
-        name = "unnamed-gpio-in";
-    }
-    for (i = gpio_list->num_in; i < gpio_list->num_in + n; i++) {
-        gchar *propname = g_strdup_printf("%s[%u]", name, i);
-
-        object_property_add_child(OBJECT(dev), propname,
-                                  OBJECT(gpio_list->in[i]), &error_abort);
-        g_free(propname);
-    }
-
-    gpio_list->num_in += n;
-}
-
-void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
-{
-    qdev_init_gpio_in_named(dev, handler, NULL, n);
-}
-
-void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
-                              const char *name, int n)
-{
-    int i;
-    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
-
-    assert(gpio_list->num_in == 0 || !name);
-
-    if (!name) {
-        name = "unnamed-gpio-out";
-    }
-    memset(pins, 0, sizeof(*pins) * n);
-    for (i = 0; i < n; ++i) {
-        gchar *propname = g_strdup_printf("%s[%u]", name,
-                                          gpio_list->num_out + i);
-
-        object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,
-                                 (Object **)&pins[i],
-                                 object_property_allow_set_link,
-                                 OBJ_PROP_LINK_STRONG,
-                                 &error_abort);
-        g_free(propname);
-    }
-    gpio_list->num_out += n;
-}
-
-void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
-{
-    qdev_init_gpio_out_named(dev, pins, NULL, n);
-}
-
-qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n)
-{
-    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
-
-    assert(n >= 0 && n < gpio_list->num_in);
-    return gpio_list->in[n];
-}
-
-qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
-{
-    return qdev_get_gpio_in_named(dev, NULL, n);
-}
-
-void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
-                                 qemu_irq pin)
-{
-    char *propname = g_strdup_printf("%s[%d]",
-                                     name ? name : "unnamed-gpio-out", n);
-    if (pin) {
-        /* We need a name for object_property_set_link to work.  If the
-         * object has a parent, object_property_add_child will come back
-         * with an error without doing anything.  If it has none, it will
-         * never fail.  So we can just call it with a NULL Error pointer.
-         */
-        object_property_add_child(container_get(qdev_get_machine(),
-                                                "/unattached"),
-                                  "non-qdev-gpio[*]", OBJECT(pin), NULL);
-    }
-    object_property_set_link(OBJECT(dev), OBJECT(pin), propname, &error_abort);
-    g_free(propname);
-}
-
-qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
-{
-    char *propname = g_strdup_printf("%s[%d]",
-                                     name ? name : "unnamed-gpio-out", n);
-
-    qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
-                                                      NULL);
-
-    return ret;
-}
-
-/* disconnect a GPIO output, returning the disconnected input (if any) */
-
-static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
-                                               const char *name, int n)
-{
-    char *propname = g_strdup_printf("%s[%d]",
-                                     name ? name : "unnamed-gpio-out", n);
-
-    qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
-                                                      NULL);
-    if (ret) {
-        object_property_set_link(OBJECT(dev), NULL, propname, NULL);
-    }
-    g_free(propname);
-    return ret;
-}
-
-qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
-                                 const char *name, int n)
-{
-    qemu_irq disconnected = qdev_disconnect_gpio_out_named(dev, name, n);
-    qdev_connect_gpio_out_named(dev, name, n, icpt);
-    return disconnected;
-}
-
-void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
-{
-    qdev_connect_gpio_out_named(dev, NULL, n, pin);
-}
-
-void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
-                     const char *name)
-{
-    int i;
-    NamedGPIOList *ngl = qdev_get_named_gpio_list(dev, name);
-
-    for (i = 0; i < ngl->num_in; i++) {
-        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-in";
-        char *propname = g_strdup_printf("%s[%d]", nm, i);
-
-        object_property_add_alias(OBJECT(container), propname,
-                                  OBJECT(dev), propname,
-                                  &error_abort);
-        g_free(propname);
-    }
-    for (i = 0; i < ngl->num_out; i++) {
-        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-out";
-        char *propname = g_strdup_printf("%s[%d]", nm, i);
-
-        object_property_add_alias(OBJECT(container), propname,
-                                  OBJECT(dev), propname,
-                                  &error_abort);
-        g_free(propname);
-    }
-    QLIST_REMOVE(ngl, node);
-    QLIST_INSERT_HEAD(&container->gpios, ngl, node);
-}
-
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index a799c83815..6789154807 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -13,6 +13,7 @@ common-obj-$(CONFIG_XILINX_AXI) += stream.o
 common-obj-$(CONFIG_PTIMER) += ptimer.o
 common-obj-$(CONFIG_SOFTMMU) += sysbus.o
 common-obj-$(CONFIG_SOFTMMU) += machine.o
+common-obj-$(CONFIG_SOFTMMU) += gpio.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_FITLOADER) += loader-fit.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PATCH 1/4] machine: Move gpio code to hw/core/gpio.c

[Eduardo Habkost]

Only softmmu code uses gpio, so move gpio code from qdev.c to
gpio.c and compile it only on softmmu mode.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/gpio.c        | 206 ++++++++++++++++++++++++++++++++++++++++++
 hw/core/qdev.c        | 184 -------------------------------------
 hw/core/Makefile.objs |   1 +
 3 files changed, 207 insertions(+), 184 deletions(-)
 create mode 100644 hw/core/gpio.c

diff --git a/hw/core/gpio.c b/hw/core/gpio.c
new file mode 100644
index 0000000000..2724b4d7e7
--- /dev/null
+++ b/hw/core/gpio.c
@@ -0,0 +1,206 @@
+/*
+ * qdev GPIO helpers
+ *
+ *  Copyright (c) 2009 CodeSourcery
+ *
+ * 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/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+#include "qapi/error.h"
+
+static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
+                                               const char *name)
+{
+    NamedGPIOList *ngl;
+
+    QLIST_FOREACH(ngl, &dev->gpios, node) {
+        /* NULL is a valid and matchable name, otherwise do a normal
+         * strcmp match.
+         */
+        if ((!ngl->name && !name) ||
+                (name && ngl->name && strcmp(name, ngl->name) == 0)) {
+            return ngl;
+        }
+    }
+
+    ngl = g_malloc0(sizeof(*ngl));
+    ngl->name = g_strdup(name);
+    QLIST_INSERT_HEAD(&dev->gpios, ngl, node);
+    return ngl;
+}
+
+void qdev_init_gpio_in_named_with_opaque(DeviceState *dev,
+                                         qemu_irq_handler handler,
+                                         void *opaque,
+                                         const char *name, int n)
+{
+    int i;
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(gpio_list->num_out == 0 || !name);
+    gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
+                                     opaque, n);
+
+    if (!name) {
+        name = "unnamed-gpio-in";
+    }
+    for (i = gpio_list->num_in; i < gpio_list->num_in + n; i++) {
+        gchar *propname = g_strdup_printf("%s[%u]", name, i);
+
+        object_property_add_child(OBJECT(dev), propname,
+                                  OBJECT(gpio_list->in[i]), &error_abort);
+        g_free(propname);
+    }
+
+    gpio_list->num_in += n;
+}
+
+void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
+{
+    qdev_init_gpio_in_named(dev, handler, NULL, n);
+}
+
+void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
+                              const char *name, int n)
+{
+    int i;
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(gpio_list->num_in == 0 || !name);
+
+    if (!name) {
+        name = "unnamed-gpio-out";
+    }
+    memset(pins, 0, sizeof(*pins) * n);
+    for (i = 0; i < n; ++i) {
+        gchar *propname = g_strdup_printf("%s[%u]", name,
+                                          gpio_list->num_out + i);
+
+        object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,
+                                 (Object **)&pins[i],
+                                 object_property_allow_set_link,
+                                 OBJ_PROP_LINK_STRONG,
+                                 &error_abort);
+        g_free(propname);
+    }
+    gpio_list->num_out += n;
+}
+
+void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
+{
+    qdev_init_gpio_out_named(dev, pins, NULL, n);
+}
+
+qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(n >= 0 && n < gpio_list->num_in);
+    return gpio_list->in[n];
+}
+
+qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
+{
+    return qdev_get_gpio_in_named(dev, NULL, n);
+}
+
+void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
+                                 qemu_irq pin)
+{
+    char *propname = g_strdup_printf("%s[%d]",
+                                     name ? name : "unnamed-gpio-out", n);
+    if (pin) {
+        /* We need a name for object_property_set_link to work.  If the
+         * object has a parent, object_property_add_child will come back
+         * with an error without doing anything.  If it has none, it will
+         * never fail.  So we can just call it with a NULL Error pointer.
+         */
+        object_property_add_child(container_get(qdev_get_machine(),
+                                                "/unattached"),
+                                  "non-qdev-gpio[*]", OBJECT(pin), NULL);
+    }
+    object_property_set_link(OBJECT(dev), OBJECT(pin), propname, &error_abort);
+    g_free(propname);
+}
+
+qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
+{
+    char *propname = g_strdup_printf("%s[%d]",
+                                     name ? name : "unnamed-gpio-out", n);
+
+    qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
+                                                      NULL);
+
+    return ret;
+}
+
+/* disconnect a GPIO output, returning the disconnected input (if any) */
+
+static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
+                                               const char *name, int n)
+{
+    char *propname = g_strdup_printf("%s[%d]",
+                                     name ? name : "unnamed-gpio-out", n);
+
+    qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
+                                                      NULL);
+    if (ret) {
+        object_property_set_link(OBJECT(dev), NULL, propname, NULL);
+    }
+    g_free(propname);
+    return ret;
+}
+
+qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
+                                 const char *name, int n)
+{
+    qemu_irq disconnected = qdev_disconnect_gpio_out_named(dev, name, n);
+    qdev_connect_gpio_out_named(dev, name, n, icpt);
+    return disconnected;
+}
+
+void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
+{
+    qdev_connect_gpio_out_named(dev, NULL, n, pin);
+}
+
+void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
+                     const char *name)
+{
+    int i;
+    NamedGPIOList *ngl = qdev_get_named_gpio_list(dev, name);
+
+    for (i = 0; i < ngl->num_in; i++) {
+        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-in";
+        char *propname = g_strdup_printf("%s[%d]", nm, i);
+
+        object_property_add_alias(OBJECT(container), propname,
+                                  OBJECT(dev), propname,
+                                  &error_abort);
+        g_free(propname);
+    }
+    for (i = 0; i < ngl->num_out; i++) {
+        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-out";
+        char *propname = g_strdup_printf("%s[%d]", nm, i);
+
+        object_property_add_alias(OBJECT(container), propname,
+                                  OBJECT(dev), propname,
+                                  &error_abort);
+        g_free(propname);
+    }
+    QLIST_REMOVE(ngl, node);
+    QLIST_INSERT_HEAD(&container->gpios, ngl, node);
+}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f9b6efe509..f73e7ded1a 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -346,190 +346,6 @@ BusState *qdev_get_parent_bus(DeviceState *dev)
     return dev->parent_bus;
 }
 
-static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
-                                               const char *name)
-{
-    NamedGPIOList *ngl;
-
-    QLIST_FOREACH(ngl, &dev->gpios, node) {
-        /* NULL is a valid and matchable name, otherwise do a normal
-         * strcmp match.
-         */
-        if ((!ngl->name && !name) ||
-                (name && ngl->name && strcmp(name, ngl->name) == 0)) {
-            return ngl;
-        }
-    }
-
-    ngl = g_malloc0(sizeof(*ngl));
-    ngl->name = g_strdup(name);
-    QLIST_INSERT_HEAD(&dev->gpios, ngl, node);
-    return ngl;
-}
-
-void qdev_init_gpio_in_named_with_opaque(DeviceState *dev,
-                                         qemu_irq_handler handler,
-                                         void *opaque,
-                                         const char *name, int n)
-{
-    int i;
-    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
-
-    assert(gpio_list->num_out == 0 || !name);
-    gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
-                                     opaque, n);
-
-    if (!name) {
-        name = "unnamed-gpio-in";
-    }
-    for (i = gpio_list->num_in; i < gpio_list->num_in + n; i++) {
-        gchar *propname = g_strdup_printf("%s[%u]", name, i);
-
-        object_property_add_child(OBJECT(dev), propname,
-                                  OBJECT(gpio_list->in[i]), &error_abort);
-        g_free(propname);
-    }
-
-    gpio_list->num_in += n;
-}
-
-void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
-{
-    qdev_init_gpio_in_named(dev, handler, NULL, n);
-}
-
-void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
-                              const char *name, int n)
-{
-    int i;
-    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
-
-    assert(gpio_list->num_in == 0 || !name);
-
-    if (!name) {
-        name = "unnamed-gpio-out";
-    }
-    memset(pins, 0, sizeof(*pins) * n);
-    for (i = 0; i < n; ++i) {
-        gchar *propname = g_strdup_printf("%s[%u]", name,
-                                          gpio_list->num_out + i);
-
-        object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,
-                                 (Object **)&pins[i],
-                                 object_property_allow_set_link,
-                                 OBJ_PROP_LINK_STRONG,
-                                 &error_abort);
-        g_free(propname);
-    }
-    gpio_list->num_out += n;
-}
-
-void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
-{
-    qdev_init_gpio_out_named(dev, pins, NULL, n);
-}
-
-qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n)
-{
-    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
-
-    assert(n >= 0 && n < gpio_list->num_in);
-    return gpio_list->in[n];
-}
-
-qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
-{
-    return qdev_get_gpio_in_named(dev, NULL, n);
-}
-
-void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
-                                 qemu_irq pin)
-{
-    char *propname = g_strdup_printf("%s[%d]",
-                                     name ? name : "unnamed-gpio-out", n);
-    if (pin) {
-        /* We need a name for object_property_set_link to work.  If the
-         * object has a parent, object_property_add_child will come back
-         * with an error without doing anything.  If it has none, it will
-         * never fail.  So we can just call it with a NULL Error pointer.
-         */
-        object_property_add_child(container_get(qdev_get_machine(),
-                                                "/unattached"),
-                                  "non-qdev-gpio[*]", OBJECT(pin), NULL);
-    }
-    object_property_set_link(OBJECT(dev), OBJECT(pin), propname, &error_abort);
-    g_free(propname);
-}
-
-qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n)
-{
-    char *propname = g_strdup_printf("%s[%d]",
-                                     name ? name : "unnamed-gpio-out", n);
-
-    qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
-                                                      NULL);
-
-    return ret;
-}
-
-/* disconnect a GPIO output, returning the disconnected input (if any) */
-
-static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
-                                               const char *name, int n)
-{
-    char *propname = g_strdup_printf("%s[%d]",
-                                     name ? name : "unnamed-gpio-out", n);
-
-    qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
-                                                      NULL);
-    if (ret) {
-        object_property_set_link(OBJECT(dev), NULL, propname, NULL);
-    }
-    g_free(propname);
-    return ret;
-}
-
-qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
-                                 const char *name, int n)
-{
-    qemu_irq disconnected = qdev_disconnect_gpio_out_named(dev, name, n);
-    qdev_connect_gpio_out_named(dev, name, n, icpt);
-    return disconnected;
-}
-
-void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
-{
-    qdev_connect_gpio_out_named(dev, NULL, n, pin);
-}
-
-void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
-                     const char *name)
-{
-    int i;
-    NamedGPIOList *ngl = qdev_get_named_gpio_list(dev, name);
-
-    for (i = 0; i < ngl->num_in; i++) {
-        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-in";
-        char *propname = g_strdup_printf("%s[%d]", nm, i);
-
-        object_property_add_alias(OBJECT(container), propname,
-                                  OBJECT(dev), propname,
-                                  &error_abort);
-        g_free(propname);
-    }
-    for (i = 0; i < ngl->num_out; i++) {
-        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-out";
-        char *propname = g_strdup_printf("%s[%d]", nm, i);
-
-        object_property_add_alias(OBJECT(container), propname,
-                                  OBJECT(dev), propname,
-                                  &error_abort);
-        g_free(propname);
-    }
-    QLIST_REMOVE(ngl, node);
-    QLIST_INSERT_HEAD(&container->gpios, ngl, node);
-}
-
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index a799c83815..6789154807 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -13,6 +13,7 @@ common-obj-$(CONFIG_XILINX_AXI) += stream.o
 common-obj-$(CONFIG_PTIMER) += ptimer.o
 common-obj-$(CONFIG_SOFTMMU) += sysbus.o
 common-obj-$(CONFIG_SOFTMMU) += machine.o
+common-obj-$(CONFIG_SOFTMMU) += gpio.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_FITLOADER) += loader-fit.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-properties-system.o
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [PATCH 1/4] machine: Move gpio code to hw/core/gpio.c

[Philippe Mathieu-Daudé]

On 4/25/19 22:00, Eduardo Habkost wrote:
> Only softmmu code uses gpio, so move gpio code from qdev.c to
> gpio.c and compile it only on softmmu mode.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/core/gpio.c        | 206 ++++++++++++++++++++++++++++++++++++++++++
>  hw/core/qdev.c        | 184 -------------------------------------
>  hw/core/Makefile.objs |   1 +
>  3 files changed, 207 insertions(+), 184 deletions(-)
>  create mode 100644 hw/core/gpio.c

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[Qemu-devel] [PATCH 2/4] move qdev hotplug code to qdev-hotplug.c

[Eduardo Habkost]

The qdev hotplug code is used only in softmmu mode, so move it to
a separate file so we can eventually avoid compiling it in
user-only mode.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/bus.c          | 11 -------
 hw/core/qdev-hotplug.c | 69 ++++++++++++++++++++++++++++++++++++++++++
 hw/core/qdev.c         | 35 ---------------------
 hw/core/Makefile.objs  |  2 +-
 4 files changed, 70 insertions(+), 47 deletions(-)
 create mode 100644 hw/core/qdev-hotplug.c

diff --git a/hw/core/bus.c b/hw/core/bus.c
index e09843f6ab..35e042416c 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -22,17 +22,6 @@
 #include "hw/qdev.h"
 #include "qapi/error.h"
 
-void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
-{
-    object_property_set_link(OBJECT(bus), OBJECT(handler),
-                             QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
-}
-
-void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
-{
-    qbus_set_hotplug_handler(bus, OBJECT(bus), errp);
-}
-
 int qbus_walk_children(BusState *bus,
                        qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
                        qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c
new file mode 100644
index 0000000000..8ab31043a7
--- /dev/null
+++ b/hw/core/qdev-hotplug.c
@@ -0,0 +1,69 @@
+/*
+ * qdev and qbus hotplug helpers
+ *
+ *  Copyright (c) 2009 CodeSourcery
+ *
+ * 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/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+#include "sysemu/sysemu.h"
+#include "hw/boards.h"
+
+void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
+{
+    object_property_set_link(OBJECT(bus), OBJECT(handler),
+                             QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
+}
+
+void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
+{
+    qbus_set_hotplug_handler(bus, OBJECT(bus), errp);
+}
+
+HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)
+{
+    MachineState *machine;
+    MachineClass *mc;
+    Object *m_obj = qdev_get_machine();
+
+    if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
+        machine = MACHINE(m_obj);
+        mc = MACHINE_GET_CLASS(machine);
+        if (mc->get_hotplug_handler) {
+            return mc->get_hotplug_handler(machine, dev);
+        }
+    }
+
+    return NULL;
+}
+
+HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev)
+{
+    if (dev->parent_bus) {
+        return dev->parent_bus->hotplug_handler;
+    }
+    return NULL;
+}
+
+HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
+{
+    HotplugHandler *hotplug_ctrl = qdev_get_machine_hotplug_handler(dev);
+
+    if (hotplug_ctrl == NULL && dev->parent_bus) {
+        hotplug_ctrl = qdev_get_bus_hotplug_handler(dev);
+    }
+    return hotplug_ctrl;
+}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f73e7ded1a..3015da0ac9 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -219,41 +219,6 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
     dev->alias_required_for_version = required_for_version;
 }
 
-HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)
-{
-    MachineState *machine;
-    MachineClass *mc;
-    Object *m_obj = qdev_get_machine();
-
-    if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
-        machine = MACHINE(m_obj);
-        mc = MACHINE_GET_CLASS(machine);
-        if (mc->get_hotplug_handler) {
-            return mc->get_hotplug_handler(machine, dev);
-        }
-    }
-
-    return NULL;
-}
-
-HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev)
-{
-    if (dev->parent_bus) {
-        return dev->parent_bus->hotplug_handler;
-    }
-    return NULL;
-}
-
-HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
-{
-    HotplugHandler *hotplug_ctrl = qdev_get_machine_hotplug_handler(dev);
-
-    if (hotplug_ctrl == NULL && dev->parent_bus) {
-        hotplug_ctrl = qdev_get_bus_hotplug_handler(dev);
-    }
-    return hotplug_ctrl;
-}
-
 static int qdev_reset_one(DeviceState *dev, void *opaque)
 {
     device_reset(dev);
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 6789154807..9c4f953716 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -1,6 +1,6 @@
 # core qdev-related obj files, also used by *-user:
 common-obj-y += qdev.o qdev-properties.o
-common-obj-y += bus.o reset.o
+common-obj-y += bus.o reset.o qdev-hotplug.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
 common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
 # irq.o needed for qdev GPIO handling:
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PATCH 2/4] move qdev hotplug code to qdev-hotplug.c

[Eduardo Habkost]

The qdev hotplug code is used only in softmmu mode, so move it to
a separate file so we can eventually avoid compiling it in
user-only mode.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/bus.c          | 11 -------
 hw/core/qdev-hotplug.c | 69 ++++++++++++++++++++++++++++++++++++++++++
 hw/core/qdev.c         | 35 ---------------------
 hw/core/Makefile.objs  |  2 +-
 4 files changed, 70 insertions(+), 47 deletions(-)
 create mode 100644 hw/core/qdev-hotplug.c

diff --git a/hw/core/bus.c b/hw/core/bus.c
index e09843f6ab..35e042416c 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -22,17 +22,6 @@
 #include "hw/qdev.h"
 #include "qapi/error.h"
 
-void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
-{
-    object_property_set_link(OBJECT(bus), OBJECT(handler),
-                             QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
-}
-
-void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
-{
-    qbus_set_hotplug_handler(bus, OBJECT(bus), errp);
-}
-
 int qbus_walk_children(BusState *bus,
                        qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
                        qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c
new file mode 100644
index 0000000000..8ab31043a7
--- /dev/null
+++ b/hw/core/qdev-hotplug.c
@@ -0,0 +1,69 @@
+/*
+ * qdev and qbus hotplug helpers
+ *
+ *  Copyright (c) 2009 CodeSourcery
+ *
+ * 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/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+#include "sysemu/sysemu.h"
+#include "hw/boards.h"
+
+void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
+{
+    object_property_set_link(OBJECT(bus), OBJECT(handler),
+                             QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
+}
+
+void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
+{
+    qbus_set_hotplug_handler(bus, OBJECT(bus), errp);
+}
+
+HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)
+{
+    MachineState *machine;
+    MachineClass *mc;
+    Object *m_obj = qdev_get_machine();
+
+    if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
+        machine = MACHINE(m_obj);
+        mc = MACHINE_GET_CLASS(machine);
+        if (mc->get_hotplug_handler) {
+            return mc->get_hotplug_handler(machine, dev);
+        }
+    }
+
+    return NULL;
+}
+
+HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev)
+{
+    if (dev->parent_bus) {
+        return dev->parent_bus->hotplug_handler;
+    }
+    return NULL;
+}
+
+HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
+{
+    HotplugHandler *hotplug_ctrl = qdev_get_machine_hotplug_handler(dev);
+
+    if (hotplug_ctrl == NULL && dev->parent_bus) {
+        hotplug_ctrl = qdev_get_bus_hotplug_handler(dev);
+    }
+    return hotplug_ctrl;
+}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f73e7ded1a..3015da0ac9 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -219,41 +219,6 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
     dev->alias_required_for_version = required_for_version;
 }
 
-HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)
-{
-    MachineState *machine;
-    MachineClass *mc;
-    Object *m_obj = qdev_get_machine();
-
-    if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
-        machine = MACHINE(m_obj);
-        mc = MACHINE_GET_CLASS(machine);
-        if (mc->get_hotplug_handler) {
-            return mc->get_hotplug_handler(machine, dev);
-        }
-    }
-
-    return NULL;
-}
-
-HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev)
-{
-    if (dev->parent_bus) {
-        return dev->parent_bus->hotplug_handler;
-    }
-    return NULL;
-}
-
-HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
-{
-    HotplugHandler *hotplug_ctrl = qdev_get_machine_hotplug_handler(dev);
-
-    if (hotplug_ctrl == NULL && dev->parent_bus) {
-        hotplug_ctrl = qdev_get_bus_hotplug_handler(dev);
-    }
-    return hotplug_ctrl;
-}
-
 static int qdev_reset_one(DeviceState *dev, void *opaque)
 {
     device_reset(dev);
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 6789154807..9c4f953716 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -1,6 +1,6 @@
 # core qdev-related obj files, also used by *-user:
 common-obj-y += qdev.o qdev-properties.o
-common-obj-y += bus.o reset.o
+common-obj-y += bus.o reset.o qdev-hotplug.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
 common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
 # irq.o needed for qdev GPIO handling:
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PATCH 3/4] qdev: Don't compile hotplug code in user-mode emulation

[Eduardo Habkost]

Move qdev-hotplug.o and hotplug.o to CONFIG_SOFTMMU because the
code is not necessary for *-user.

Provide a few stubs in qdev-hotplug-stubs.c to replace
initialization functions that are called from qdev.c.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/qdev-hotplug-stubs.c | 44 ++++++++++++++++++++++++++++++++++++
 hw/core/Makefile.objs        |  6 +++--
 tests/Makefile.include       |  3 ++-
 3 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 hw/core/qdev-hotplug-stubs.c

diff --git a/hw/core/qdev-hotplug-stubs.c b/hw/core/qdev-hotplug-stubs.c
new file mode 100644
index 0000000000..c710f23388
--- /dev/null
+++ b/hw/core/qdev-hotplug-stubs.c
@@ -0,0 +1,44 @@
+/*
+ * qdev hotplug handler stubs (for user-mode emulation and unit tests)
+ *
+ *  Copyright (c) 2019 Red Hat Inc
+ *
+ * Authors:
+ *  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/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-core.h"
+#include "hw/hotplug.h"
+
+HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
+{
+    return NULL;
+}
+
+void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
+                              DeviceState *plugged_dev,
+                              Error **errp)
+{
+    assert(plug_handler == NULL);
+}
+
+void hotplug_handler_plug(HotplugHandler *plug_handler,
+                          DeviceState *plugged_dev,
+                          Error **errp)
+{
+    assert(plug_handler == NULL);
+}
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 9c4f953716..284fc306c5 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -1,11 +1,10 @@
 # core qdev-related obj files, also used by *-user:
 common-obj-y += qdev.o qdev-properties.o
-common-obj-y += bus.o reset.o qdev-hotplug.o
+common-obj-y += bus.o reset.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
 common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
 # irq.o needed for qdev GPIO handling:
 common-obj-y += irq.o
-common-obj-y += hotplug.o
 common-obj-$(CONFIG_SOFTMMU) += nmi.o
 
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
@@ -13,6 +12,7 @@ common-obj-$(CONFIG_XILINX_AXI) += stream.o
 common-obj-$(CONFIG_PTIMER) += ptimer.o
 common-obj-$(CONFIG_SOFTMMU) += sysbus.o
 common-obj-$(CONFIG_SOFTMMU) += machine.o
+common-obj-$(CONFIG_SOFTMMU) += hotplug.o qdev-hotplug.o
 common-obj-$(CONFIG_SOFTMMU) += gpio.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_FITLOADER) += loader-fit.o
@@ -23,3 +23,5 @@ common-obj-$(CONFIG_SOFTMMU) += split-irq.o
 common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
 common-obj-$(CONFIG_SOFTMMU) += generic-loader.o
 common-obj-$(CONFIG_SOFTMMU) += null-machine.o
+
+common-obj-$(CONFIG_USER_ONLY) += qdev-hotplug-stubs.o
diff --git a/tests/Makefile.include b/tests/Makefile.include
index e2432d5e77..fd5e7e0c77 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -568,7 +568,8 @@ tests/fp/%:
 	$(MAKE) -C $(dir $@) $(notdir $@)
 
 tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
-	hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
+	hw/core/qdev.o hw/core/qdev-properties.o \
+	hw/core/qdev-hotplug-stubs.o \
 	hw/core/bus.o \
 	hw/core/irq.o \
 	hw/core/fw-path-provider.o \
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PATCH 3/4] qdev: Don't compile hotplug code in user-mode emulation

[Eduardo Habkost]

Move qdev-hotplug.o and hotplug.o to CONFIG_SOFTMMU because the
code is not necessary for *-user.

Provide a few stubs in qdev-hotplug-stubs.c to replace
initialization functions that are called from qdev.c.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/qdev-hotplug-stubs.c | 44 ++++++++++++++++++++++++++++++++++++
 hw/core/Makefile.objs        |  6 +++--
 tests/Makefile.include       |  3 ++-
 3 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 hw/core/qdev-hotplug-stubs.c

diff --git a/hw/core/qdev-hotplug-stubs.c b/hw/core/qdev-hotplug-stubs.c
new file mode 100644
index 0000000000..c710f23388
--- /dev/null
+++ b/hw/core/qdev-hotplug-stubs.c
@@ -0,0 +1,44 @@
+/*
+ * qdev hotplug handler stubs (for user-mode emulation and unit tests)
+ *
+ *  Copyright (c) 2019 Red Hat Inc
+ *
+ * Authors:
+ *  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/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-core.h"
+#include "hw/hotplug.h"
+
+HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
+{
+    return NULL;
+}
+
+void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
+                              DeviceState *plugged_dev,
+                              Error **errp)
+{
+    assert(plug_handler == NULL);
+}
+
+void hotplug_handler_plug(HotplugHandler *plug_handler,
+                          DeviceState *plugged_dev,
+                          Error **errp)
+{
+    assert(plug_handler == NULL);
+}
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 9c4f953716..284fc306c5 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -1,11 +1,10 @@
 # core qdev-related obj files, also used by *-user:
 common-obj-y += qdev.o qdev-properties.o
-common-obj-y += bus.o reset.o qdev-hotplug.o
+common-obj-y += bus.o reset.o
 common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
 common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
 # irq.o needed for qdev GPIO handling:
 common-obj-y += irq.o
-common-obj-y += hotplug.o
 common-obj-$(CONFIG_SOFTMMU) += nmi.o
 
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
@@ -13,6 +12,7 @@ common-obj-$(CONFIG_XILINX_AXI) += stream.o
 common-obj-$(CONFIG_PTIMER) += ptimer.o
 common-obj-$(CONFIG_SOFTMMU) += sysbus.o
 common-obj-$(CONFIG_SOFTMMU) += machine.o
+common-obj-$(CONFIG_SOFTMMU) += hotplug.o qdev-hotplug.o
 common-obj-$(CONFIG_SOFTMMU) += gpio.o
 common-obj-$(CONFIG_SOFTMMU) += loader.o
 common-obj-$(CONFIG_FITLOADER) += loader-fit.o
@@ -23,3 +23,5 @@ common-obj-$(CONFIG_SOFTMMU) += split-irq.o
 common-obj-$(CONFIG_PLATFORM_BUS) += platform-bus.o
 common-obj-$(CONFIG_SOFTMMU) += generic-loader.o
 common-obj-$(CONFIG_SOFTMMU) += null-machine.o
+
+common-obj-$(CONFIG_USER_ONLY) += qdev-hotplug-stubs.o
diff --git a/tests/Makefile.include b/tests/Makefile.include
index e2432d5e77..fd5e7e0c77 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -568,7 +568,8 @@ tests/fp/%:
 	$(MAKE) -C $(dir $@) $(notdir $@)
 
 tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
-	hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
+	hw/core/qdev.o hw/core/qdev-properties.o \
+	hw/core/qdev-hotplug-stubs.o \
 	hw/core/bus.o \
 	hw/core/irq.o \
 	hw/core/fw-path-provider.o \
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [PATCH 3/4] qdev: Don't compile hotplug code in user-mode emulation

[Paolo Bonzini]

On 25/04/19 22:00, Eduardo Habkost wrote:
> diff --git a/hw/core/qdev-hotplug-stubs.c b/hw/core/qdev-hotplug-stubs.c
> new file mode 100644
> index 0000000000..c710f23388
> --- /dev/null
> +++ b/hw/core/qdev-hotplug-stubs.c
> @@ -0,0 +1,44 @@
> +/*
> + * qdev hotplug handler stubs (for user-mode emulation and unit tests)

Can you explain the issue with unit tests in the commit message?

> + *  Copyright (c) 2019 Red Hat Inc
> + *
> + * Authors:
> + *  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/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/qdev-core.h"
> +#include "hw/hotplug.h"
> +
> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> +{
> +    return NULL;
> +}
> +
> +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> +                              DeviceState *plugged_dev,
> +                              Error **errp)
> +{
> +    assert(plug_handler == NULL);
> +}
> +
> +void hotplug_handler_plug(HotplugHandler *plug_handler,
> +                          DeviceState *plugged_dev,
> +                          Error **errp)
> +{
> +    assert(plug_handler == NULL);
> +}

Would it work if you instead make these functions (and the others in
hw/core/hotplug.c) inlines in include/hw/hotplug.h?

Then all that remains is qdev_get_hotplug_handler.

Paolo

Re: [Qemu-devel] [PATCH 3/4] qdev: Don't compile hotplug code in user-mode emulation

[Paolo Bonzini]

On 25/04/19 22:00, Eduardo Habkost wrote:
> diff --git a/hw/core/qdev-hotplug-stubs.c b/hw/core/qdev-hotplug-stubs.c
> new file mode 100644
> index 0000000000..c710f23388
> --- /dev/null
> +++ b/hw/core/qdev-hotplug-stubs.c
> @@ -0,0 +1,44 @@
> +/*
> + * qdev hotplug handler stubs (for user-mode emulation and unit tests)

Can you explain the issue with unit tests in the commit message?

> + *  Copyright (c) 2019 Red Hat Inc
> + *
> + * Authors:
> + *  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/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/qdev-core.h"
> +#include "hw/hotplug.h"
> +
> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> +{
> +    return NULL;
> +}
> +
> +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> +                              DeviceState *plugged_dev,
> +                              Error **errp)
> +{
> +    assert(plug_handler == NULL);
> +}
> +
> +void hotplug_handler_plug(HotplugHandler *plug_handler,
> +                          DeviceState *plugged_dev,
> +                          Error **errp)
> +{
> +    assert(plug_handler == NULL);
> +}

Would it work if you instead make these functions (and the others in
hw/core/hotplug.c) inlines in include/hw/hotplug.h?

Then all that remains is qdev_get_hotplug_handler.

Paolo

Re: [Qemu-devel] [PATCH 3/4] qdev: Don't compile hotplug code in user-mode emulation

[Eduardo Habkost]

On Fri, Apr 26, 2019 at 10:27:58AM +0200, Paolo Bonzini wrote:
> On 25/04/19 22:00, Eduardo Habkost wrote:
> > diff --git a/hw/core/qdev-hotplug-stubs.c b/hw/core/qdev-hotplug-stubs.c
> > new file mode 100644
> > index 0000000000..c710f23388
> > --- /dev/null
> > +++ b/hw/core/qdev-hotplug-stubs.c
> > @@ -0,0 +1,44 @@
> > +/*
> > + * qdev hotplug handler stubs (for user-mode emulation and unit tests)
> 
> Can you explain the issue with unit tests in the commit message?

I don't think there are issues with unit tests.  I just thought
it was pointless to link qdev-hotplug.o into unit tests binaries
if no hotplug handler object is created by them.

What do you think?  Should I keep qdev-hotplug.o and hotplug.o in
the unit tests binaries?

> 
> > + *  Copyright (c) 2019 Red Hat Inc
> > + *
> > + * Authors:
> > + *  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/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/qdev-core.h"
> > +#include "hw/hotplug.h"
> > +
> > +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> > +{
> > +    return NULL;
> > +}
> > +
> > +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> > +                              DeviceState *plugged_dev,
> > +                              Error **errp)
> > +{
> > +    assert(plug_handler == NULL);
> > +}
> > +
> > +void hotplug_handler_plug(HotplugHandler *plug_handler,
> > +                          DeviceState *plugged_dev,
> > +                          Error **errp)
> > +{
> > +    assert(plug_handler == NULL);
> > +}
> 
> Would it work if you instead make these functions (and the others in
> hw/core/hotplug.c) inlines in include/hw/hotplug.h?
> 
> Then all that remains is qdev_get_hotplug_handler.

I think that would work.  I'll do that in v2, thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/4] qdev: Don't compile hotplug code in user-mode emulation

[Eduardo Habkost]

On Fri, Apr 26, 2019 at 10:27:58AM +0200, Paolo Bonzini wrote:
> On 25/04/19 22:00, Eduardo Habkost wrote:
> > diff --git a/hw/core/qdev-hotplug-stubs.c b/hw/core/qdev-hotplug-stubs.c
> > new file mode 100644
> > index 0000000000..c710f23388
> > --- /dev/null
> > +++ b/hw/core/qdev-hotplug-stubs.c
> > @@ -0,0 +1,44 @@
> > +/*
> > + * qdev hotplug handler stubs (for user-mode emulation and unit tests)
> 
> Can you explain the issue with unit tests in the commit message?

I don't think there are issues with unit tests.  I just thought
it was pointless to link qdev-hotplug.o into unit tests binaries
if no hotplug handler object is created by them.

What do you think?  Should I keep qdev-hotplug.o and hotplug.o in
the unit tests binaries?

> 
> > + *  Copyright (c) 2019 Red Hat Inc
> > + *
> > + * Authors:
> > + *  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/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/qdev-core.h"
> > +#include "hw/hotplug.h"
> > +
> > +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> > +{
> > +    return NULL;
> > +}
> > +
> > +void hotplug_handler_pre_plug(HotplugHandler *plug_handler,
> > +                              DeviceState *plugged_dev,
> > +                              Error **errp)
> > +{
> > +    assert(plug_handler == NULL);
> > +}
> > +
> > +void hotplug_handler_plug(HotplugHandler *plug_handler,
> > +                          DeviceState *plugged_dev,
> > +                          Error **errp)
> > +{
> > +    assert(plug_handler == NULL);
> > +}
> 
> Would it work if you instead make these functions (and the others in
> hw/core/hotplug.c) inlines in include/hw/hotplug.h?
> 
> Then all that remains is qdev_get_hotplug_handler.

I think that would work.  I'll do that in v2, thanks!

-- 
Eduardo

[Qemu-devel] [PATCH 4/4] qdev-hotplug: Don't check type of qdev_get_machine()

[Eduardo Habkost]

Now that qdev_get_machine_hotplug_handler() is only compiled in
softmmu mode, we don't need to check if qdev_gt_machine() is
really of type TYPE_MACHINE.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/qdev-hotplug.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c
index 8ab31043a7..017500c1e1 100644
--- a/hw/core/qdev-hotplug.c
+++ b/hw/core/qdev-hotplug.c
@@ -35,16 +35,11 @@ void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
 
 HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)
 {
-    MachineState *machine;
-    MachineClass *mc;
-    Object *m_obj = qdev_get_machine();
+    MachineState *machine = MACHINE(qdev_get_machine());
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
 
-    if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
-        machine = MACHINE(m_obj);
-        mc = MACHINE_GET_CLASS(machine);
-        if (mc->get_hotplug_handler) {
-            return mc->get_hotplug_handler(machine, dev);
-        }
+    if (mc->get_hotplug_handler) {
+        return mc->get_hotplug_handler(machine, dev);
     }
 
     return NULL;
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PATCH 4/4] qdev-hotplug: Don't check type of qdev_get_machine()

[Eduardo Habkost]

Now that qdev_get_machine_hotplug_handler() is only compiled in
softmmu mode, we don't need to check if qdev_gt_machine() is
really of type TYPE_MACHINE.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/qdev-hotplug.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c
index 8ab31043a7..017500c1e1 100644
--- a/hw/core/qdev-hotplug.c
+++ b/hw/core/qdev-hotplug.c
@@ -35,16 +35,11 @@ void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
 
 HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)
 {
-    MachineState *machine;
-    MachineClass *mc;
-    Object *m_obj = qdev_get_machine();
+    MachineState *machine = MACHINE(qdev_get_machine());
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
 
-    if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
-        machine = MACHINE(m_obj);
-        mc = MACHINE_GET_CLASS(machine);
-        if (mc->get_hotplug_handler) {
-            return mc->get_hotplug_handler(machine, dev);
-        }
+    if (mc->get_hotplug_handler) {
+        return mc->get_hotplug_handler(machine, dev);
     }
 
     return NULL;
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [PATCH 4/4] qdev-hotplug: Don't check type of qdev_get_machine()

[Philippe Mathieu-Daudé]

On 4/25/19 22:00, Eduardo Habkost wrote:
> Now that qdev_get_machine_hotplug_handler() is only compiled in
> softmmu mode, we don't need to check if qdev_gt_machine() is
> really of type TYPE_MACHINE.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/core/qdev-hotplug.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c
> index 8ab31043a7..017500c1e1 100644
> --- a/hw/core/qdev-hotplug.c
> +++ b/hw/core/qdev-hotplug.c
> @@ -35,16 +35,11 @@ void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
>  
>  HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev)
>  {
> -    MachineState *machine;
> -    MachineClass *mc;
> -    Object *m_obj = qdev_get_machine();
> +    MachineState *machine = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
>  
> -    if (object_dynamic_cast(m_obj, TYPE_MACHINE)) {
> -        machine = MACHINE(m_obj);
> -        mc = MACHINE_GET_CLASS(machine);
> -        if (mc->get_hotplug_handler) {
> -            return mc->get_hotplug_handler(machine, dev);
> -        }
> +    if (mc->get_hotplug_handler) {
> +        return mc->get_hotplug_handler(machine, dev);
>      }
>  
>      return NULL;
> 

I rebased this patch and tried it, but it fails 'check-unit':

Running test test-qdev-global-props
**
ERROR:../tests/unit/test-qdev-global-props.c:89:test_static_prop: child
process (/qdev/properties/static/default/subprocess [12678]) failed
unexpectedly
ERROR test-qdev-global-props - Bail out!
ERROR:../tests/unit/test-qdev-global-props.c:89:test_static_prop: child
process (/qdev/properties/static/default/subprocess [12678]) failed
unexpectedly
make: *** [Makefile.mtest:640: run-test-78] Error 1

Here the object is of type "container":

  Object *object_get_root(void)
  {
      static Object *root;

      if (!root) {
          root = object_new("container");
      }

      return root;
  }

Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190425200051.19906-1-ehabkost@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20190425200051.19906-1-ehabkost@redhat.com
Subject: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20190424004700.12766-1-richardw.yang@linux.intel.com -> patchew/20190424004700.12766-1-richardw.yang@linux.intel.com
 * [new tag]               patchew/20190425200051.19906-1-ehabkost@redhat.com -> patchew/20190425200051.19906-1-ehabkost@redhat.com
Switched to a new branch 'test'
c5c77ea2cc qdev-hotplug: Don't check type of qdev_get_machine()
854ded711d qdev: Don't compile hotplug code in user-mode emulation
1bd33307be move qdev hotplug code to qdev-hotplug.c
02b1f5db6f machine: Move gpio code to hw/core/gpio.c

=== OUTPUT BEGIN ===
1/4 Checking commit 02b1f5db6fcd (machine: Move gpio code to hw/core/gpio.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#60: FILE: hw/core/gpio.c:30:
+        /* NULL is a valid and matchable name, otherwise do a normal

WARNING: Block comments use a leading /* on a separate line
#156: FILE: hw/core/gpio.c:126:
+        /* We need a name for object_property_set_link to work.  If the

ERROR: "foo * bar" should be "foo *bar"
#205: FILE: hw/core/gpio.c:175:
+void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)

total: 1 errors, 3 warnings, 403 lines checked

Patch 1/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/4 Checking commit 1bd33307be11 (move qdev hotplug code to qdev-hotplug.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#49: 
new file mode 100644

total: 0 errors, 1 warnings, 134 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/4 Checking commit 854ded711daa (qdev: Don't compile hotplug code in user-mode emulation)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#48: 
new file mode 100644

total: 0 errors, 1 warnings, 77 lines checked

Patch 3/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/4 Checking commit c5c77ea2ccde (qdev-hotplug: Don't check type of qdev_get_machine())
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190425200051.19906-1-ehabkost@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190425200051.19906-1-ehabkost@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20190425200051.19906-1-ehabkost@redhat.com
Subject: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20190424004700.12766-1-richardw.yang@linux.intel.com -> patchew/20190424004700.12766-1-richardw.yang@linux.intel.com
 * [new tag]               patchew/20190425200051.19906-1-ehabkost@redhat.com -> patchew/20190425200051.19906-1-ehabkost@redhat.com
Switched to a new branch 'test'
c5c77ea2cc qdev-hotplug: Don't check type of qdev_get_machine()
854ded711d qdev: Don't compile hotplug code in user-mode emulation
1bd33307be move qdev hotplug code to qdev-hotplug.c
02b1f5db6f machine: Move gpio code to hw/core/gpio.c

=== OUTPUT BEGIN ===
1/4 Checking commit 02b1f5db6fcd (machine: Move gpio code to hw/core/gpio.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#60: FILE: hw/core/gpio.c:30:
+        /* NULL is a valid and matchable name, otherwise do a normal

WARNING: Block comments use a leading /* on a separate line
#156: FILE: hw/core/gpio.c:126:
+        /* We need a name for object_property_set_link to work.  If the

ERROR: "foo * bar" should be "foo *bar"
#205: FILE: hw/core/gpio.c:175:
+void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)

total: 1 errors, 3 warnings, 403 lines checked

Patch 1/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/4 Checking commit 1bd33307be11 (move qdev hotplug code to qdev-hotplug.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#49: 
new file mode 100644

total: 0 errors, 1 warnings, 134 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/4 Checking commit 854ded711daa (qdev: Don't compile hotplug code in user-mode emulation)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#48: 
new file mode 100644

total: 0 errors, 1 warnings, 77 lines checked

Patch 3/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/4 Checking commit c5c77ea2ccde (qdev-hotplug: Don't check type of qdev_get_machine())
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190425200051.19906-1-ehabkost@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Like Xu]

On 2019/4/26 4:00, Eduardo Habkost wrote:
> This series moves some qdev code outside qdev.o, so it can be
> compiled only in CONFIG_SOFTMMU.
> 
> The code being moved includes two qdev_get_machine() calls, so
> this will make it easier to move qdev_get_machine() to
> CONFIG_SOFTMMU later.
> 
> After this series, there's one remaining qdev_get_machine() call
> that seems more difficult to remove:
> 
>      static void device_set_realized(Object *obj, bool value, Error **errp)
>      {
>          /* [...] */
>          if (!obj->parent) {
>              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);
>          }
>          /* [...] */
>      }
> 

I may have an experimental patch to fix device_set_realized issue:

1. in qdev_get_machine():
replace
	dev = container_get(object_get_root(), "/machine");
with
	dev = object_resolve_path("/machine", NULL);

2. in device_set_realized():

Using
	Object *container = qdev_get_machine() ?
		qdev_get_machine() : object_get_root();
and pass it to
	object_property_add_child(
		container_get(container, "/unattached"),
		name, obj, &error_abort);

With this fix, we could say the qdev_get_machine() does
return the "/machine" object (or null) not a confused "/container".

We could continue to use qdev_get_machine() in system emulation mode,
getting rid of its surprising side effect as Markus said.

The return value of qdev_get_machine() in user-only mode
is the same object returned by object_get_root(),
so no semantic changes.


> This one is tricky because on system emulation mode it needs
> "/machine" to already exist, but in user-only mode it needs to
> implicitly create a "/machine" container.
> 
> Eduardo Habkost (4):
>    machine: Move gpio code to hw/core/gpio.c
>    move qdev hotplug code to qdev-hotplug.c
>    qdev: Don't compile hotplug code in user-mode emulation
>    qdev-hotplug: Don't check type of qdev_get_machine()
> 
>   hw/core/bus.c                |  11 --
>   hw/core/gpio.c               | 206 ++++++++++++++++++++++++++++++++
>   hw/core/qdev-hotplug-stubs.c |  44 +++++++
>   hw/core/qdev-hotplug.c       |  64 ++++++++++
>   hw/core/qdev.c               | 219 -----------------------------------
>   hw/core/Makefile.objs        |   5 +-
>   tests/Makefile.include       |   3 +-
>   7 files changed, 320 insertions(+), 232 deletions(-)
>   create mode 100644 hw/core/gpio.c
>   create mode 100644 hw/core/qdev-hotplug-stubs.c
>   create mode 100644 hw/core/qdev-hotplug.c
> 

Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Like Xu]

On 2019/4/26 4:00, Eduardo Habkost wrote:
> This series moves some qdev code outside qdev.o, so it can be
> compiled only in CONFIG_SOFTMMU.
> 
> The code being moved includes two qdev_get_machine() calls, so
> this will make it easier to move qdev_get_machine() to
> CONFIG_SOFTMMU later.
> 
> After this series, there's one remaining qdev_get_machine() call
> that seems more difficult to remove:
> 
>      static void device_set_realized(Object *obj, bool value, Error **errp)
>      {
>          /* [...] */
>          if (!obj->parent) {
>              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);
>          }
>          /* [...] */
>      }
> 

I may have an experimental patch to fix device_set_realized issue:

1. in qdev_get_machine():
replace
	dev = container_get(object_get_root(), "/machine");
with
	dev = object_resolve_path("/machine", NULL);

2. in device_set_realized():

Using
	Object *container = qdev_get_machine() ?
		qdev_get_machine() : object_get_root();
and pass it to
	object_property_add_child(
		container_get(container, "/unattached"),
		name, obj, &error_abort);

With this fix, we could say the qdev_get_machine() does
return the "/machine" object (or null) not a confused "/container".

We could continue to use qdev_get_machine() in system emulation mode,
getting rid of its surprising side effect as Markus said.

The return value of qdev_get_machine() in user-only mode
is the same object returned by object_get_root(),
so no semantic changes.


> This one is tricky because on system emulation mode it needs
> "/machine" to already exist, but in user-only mode it needs to
> implicitly create a "/machine" container.
> 
> Eduardo Habkost (4):
>    machine: Move gpio code to hw/core/gpio.c
>    move qdev hotplug code to qdev-hotplug.c
>    qdev: Don't compile hotplug code in user-mode emulation
>    qdev-hotplug: Don't check type of qdev_get_machine()
> 
>   hw/core/bus.c                |  11 --
>   hw/core/gpio.c               | 206 ++++++++++++++++++++++++++++++++
>   hw/core/qdev-hotplug-stubs.c |  44 +++++++
>   hw/core/qdev-hotplug.c       |  64 ++++++++++
>   hw/core/qdev.c               | 219 -----------------------------------
>   hw/core/Makefile.objs        |   5 +-
>   tests/Makefile.include       |   3 +-
>   7 files changed, 320 insertions(+), 232 deletions(-)
>   create mode 100644 hw/core/gpio.c
>   create mode 100644 hw/core/qdev-hotplug-stubs.c
>   create mode 100644 hw/core/qdev-hotplug.c
> 

Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Eduardo Habkost]

On Fri, Apr 26, 2019 at 04:55:17PM +0800, Like Xu wrote:
> On 2019/4/26 4:00, Eduardo Habkost wrote:
> > This series moves some qdev code outside qdev.o, so it can be
> > compiled only in CONFIG_SOFTMMU.
> > 
> > The code being moved includes two qdev_get_machine() calls, so
> > this will make it easier to move qdev_get_machine() to
> > CONFIG_SOFTMMU later.
> > 
> > After this series, there's one remaining qdev_get_machine() call
> > that seems more difficult to remove:
> > 
> >      static void device_set_realized(Object *obj, bool value, Error **errp)
> >      {
> >          /* [...] */
> >          if (!obj->parent) {
> >              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);
> >          }
> >          /* [...] */
> >      }
> > 
> 
> I may have an experimental patch to fix device_set_realized issue:
> 
> 1. in qdev_get_machine():
> replace
> 	dev = container_get(object_get_root(), "/machine");
> with
> 	dev = object_resolve_path("/machine", NULL);
> 
> 2. in device_set_realized():
> 
> Using
> 	Object *container = qdev_get_machine() ?
> 		qdev_get_machine() : object_get_root();
> and pass it to
> 	object_property_add_child(
> 		container_get(container, "/unattached"),
> 		name, obj, &error_abort);

I wouldn't like to call qdev_get_machine() here (see below[1]),
but trying "/machine" first and falling back to object_get_root()
sounds good, for two reasons:

* It won't require "/machine" to exist at all, on *-user;
* It will allow machines to create unattached devices on
  instance_init without crashing (which is not possible today).

> 
> With this fix, we could say the qdev_get_machine() does
> return the "/machine" object (or null) not a confused "/container".
> 
> We could continue to use qdev_get_machine() in system emulation mode,
> getting rid of its surprising side effect as Markus said.
> 
> The return value of qdev_get_machine() in user-only mode
> is the same object returned by object_get_root(),
> so no semantic changes.

[1] I wouldn't like to have a different qdev_get_machine()
function in user-only mode.  I would like to simply not call
qdev_get_machine() at all unless we're in system emulation mode.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Eduardo Habkost]

On Fri, Apr 26, 2019 at 04:55:17PM +0800, Like Xu wrote:
> On 2019/4/26 4:00, Eduardo Habkost wrote:
> > This series moves some qdev code outside qdev.o, so it can be
> > compiled only in CONFIG_SOFTMMU.
> > 
> > The code being moved includes two qdev_get_machine() calls, so
> > this will make it easier to move qdev_get_machine() to
> > CONFIG_SOFTMMU later.
> > 
> > After this series, there's one remaining qdev_get_machine() call
> > that seems more difficult to remove:
> > 
> >      static void device_set_realized(Object *obj, bool value, Error **errp)
> >      {
> >          /* [...] */
> >          if (!obj->parent) {
> >              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);
> >          }
> >          /* [...] */
> >      }
> > 
> 
> I may have an experimental patch to fix device_set_realized issue:
> 
> 1. in qdev_get_machine():
> replace
> 	dev = container_get(object_get_root(), "/machine");
> with
> 	dev = object_resolve_path("/machine", NULL);
> 
> 2. in device_set_realized():
> 
> Using
> 	Object *container = qdev_get_machine() ?
> 		qdev_get_machine() : object_get_root();
> and pass it to
> 	object_property_add_child(
> 		container_get(container, "/unattached"),
> 		name, obj, &error_abort);

I wouldn't like to call qdev_get_machine() here (see below[1]),
but trying "/machine" first and falling back to object_get_root()
sounds good, for two reasons:

* It won't require "/machine" to exist at all, on *-user;
* It will allow machines to create unattached devices on
  instance_init without crashing (which is not possible today).

> 
> With this fix, we could say the qdev_get_machine() does
> return the "/machine" object (or null) not a confused "/container".
> 
> We could continue to use qdev_get_machine() in system emulation mode,
> getting rid of its surprising side effect as Markus said.
> 
> The return value of qdev_get_machine() in user-only mode
> is the same object returned by object_get_root(),
> so no semantic changes.

[1] I wouldn't like to have a different qdev_get_machine()
function in user-only mode.  I would like to simply not call
qdev_get_machine() at all unless we're in system emulation mode.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> This series moves some qdev code outside qdev.o, so it can be
> compiled only in CONFIG_SOFTMMU.
>
> The code being moved includes two qdev_get_machine() calls, so
> this will make it easier to move qdev_get_machine() to
> CONFIG_SOFTMMU later.
>
> After this series, there's one remaining qdev_get_machine() call
> that seems more difficult to remove:
>
>     static void device_set_realized(Object *obj, bool value, Error **errp)
>     {
>         /* [...] */
>         if (!obj->parent) {
>             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);
>         }
>         /* [...] */
>     }
>
> This one is tricky because on system emulation mode it needs
> "/machine" to already exist, but in user-only mode it needs to
> implicitly create a "/machine" container.

The patches look good to me on a glance.  Looking forward to v2.

Re: [Qemu-devel] [PATCH 0/4] Remove some qdev_get_machine() calls from CONFIG_USER_ONLY

[Philippe Mathieu-Daudé]

On 4/25/19 22:00, Eduardo Habkost wrote:

> Eduardo Habkost (4):
>   machine: Move gpio code to hw/core/gpio.c
>   move qdev hotplug code to qdev-hotplug.c
>   qdev: Don't compile hotplug code in user-mode emulation
>   qdev-hotplug: Don't check type of qdev_get_machine()

Patch 1 queued to machine-next.