[v3,3/4] xen: create qdev for each backend device

Commit Message

Jürgen Groß Nov. 22, 2016, 6:10 a.m. UTC

Create a qdev plugged to the xen-sysbus for each new backend device.
This device can be used as a parent for all needed devices of that
backend. The id of the new device will be "xen-<type>-<dev>" with
<type> being the xen backend type (e.g. "qdisk") and <dev> the xen
backend number of the type under which it is to be found in xenstore.

Signed-off-by: Juergen Gross <[e-mail]>
Reviewed-by: Stefano Stabellini <[e-mail]>
---
 hw/xen/xen_backend.c         | 47 ++++++++++++++++++++++++++++++++++++++++++++
 hw/xen/xen_pvdev.c           |  4 +++-
 include/hw/xen/xen_backend.h |  4 ++++
 include/hw/xen/xen_pvdev.h   |  1 +
 4 files changed, 55 insertions(+), 1 deletion(-)

Comments

Eduardo Habkost March 23, 2017, 8:57 p.m. UTC | #1

Hi,

I am reviewing the existing code that sets has_dynamic_sysbus,
and this patch confused me. I have a question below:

On Tue, Nov 22, 2016 at 07:10:58AM +0100, Juergen Gross wrote:
> Create a qdev plugged to the xen-sysbus for each new backend device.
> This device can be used as a parent for all needed devices of that
> backend. The id of the new device will be "xen-<type>-<dev>" with
> <type> being the xen backend type (e.g. "qdisk") and <dev> the xen
> backend number of the type under which it is to be found in xenstore.
> 
> Signed-off-by: Juergen Gross <[e-mail]>
> Reviewed-by: Stefano Stabellini <[e-mail]>
> ---
>  hw/xen/xen_backend.c         | 47 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/xen/xen_pvdev.c           |  4 +++-
>  include/hw/xen/xen_backend.h |  4 ++++
>  include/hw/xen/xen_pvdev.h   |  1 +
>  4 files changed, 55 insertions(+), 1 deletion(-)
> 
[...]
> +static void xen_set_dynamic_sysbus(void)
> +{
> +    Object *machine = qdev_get_machine();
> +    ObjectClass *oc = object_get_class(machine);
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->has_dynamic_sysbus = true;
> +}
> +

Why exactly is this code necessary? I assume there's no current
use case for "-device xen-sysdev" and "-device xen-backend",
correct? The devices created by C code like xen_be_get_xendev()
aren't supposed to be returned by
foreach_dynamic_sysbus_device().


>  int xen_be_register(const char *type, struct XenDevOps *ops)
>  {
>      char path[50];
> @@ -562,6 +579,8 @@ int xen_be_register(const char *type, struct XenDevOps *ops)
>  
>  void xen_be_register_common(void)
>  {
> +    xen_set_dynamic_sysbus();
> +
>      xen_be_register("console", &xen_console_ops);
>      xen_be_register("vkbd", &xen_kbdmouse_ops);
>      xen_be_register("qdisk", &xen_blkdev_ops);
[...]

Patch

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 5ad3caa..d119004 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -27,11 +27,13 @@ 
 
 #include "hw/hw.h"
 #include "hw/sysbus.h"
+#include "hw/boards.h"
 #include "sysemu/char.h"
 #include "qemu/log.h"
 #include "qapi/error.h"
 #include "hw/xen/xen_backend.h"
 #include "hw/xen/xen_pvdev.h"
+#include "monitor/qdev.h"
 
 #include <xen/grant_table.h>
 
@@ -121,6 +123,12 @@  static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev,
 
     /* init new xendev */
     xendev = g_malloc0(ops->size);
+    object_initialize(&xendev->qdev, ops->size, TYPE_XENBACKEND);
+    qdev_set_parent_bus(&xendev->qdev, xen_sysbus);
+    qdev_set_id(&xendev->qdev, g_strdup_printf("xen-%s-%d", type, dev));
+    qdev_init_nofail(&xendev->qdev);
+    object_unref(OBJECT(&xendev->qdev));
+
     xendev->type  = type;
     xendev->dom   = dom;
     xendev->dev   = dev;
@@ -541,6 +549,15 @@  err:
     return -1;
 }
 
+static void xen_set_dynamic_sysbus(void)
+{
+    Object *machine = qdev_get_machine();
+    ObjectClass *oc = object_get_class(machine);
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->has_dynamic_sysbus = true;
+}
+
 int xen_be_register(const char *type, struct XenDevOps *ops)
 {
     char path[50];
@@ -562,6 +579,8 @@  int xen_be_register(const char *type, struct XenDevOps *ops)
 
 void xen_be_register_common(void)
 {
+    xen_set_dynamic_sysbus();
+
     xen_be_register("console", &xen_console_ops);
     xen_be_register("vkbd", &xen_kbdmouse_ops);
     xen_be_register("qdisk", &xen_blkdev_ops);
@@ -588,9 +607,36 @@  int xen_be_bind_evtchn(struct XenDevice *xendev)
 }
 
 
+static Property xendev_properties[] = {
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void xendev_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->props = xendev_properties;
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo xendev_type_info = {
+    .name          = TYPE_XENBACKEND,
+    .parent        = TYPE_XENSYSDEV,
+    .class_init    = xendev_class_init,
+    .instance_size = sizeof(struct XenDevice),
+};
+
+static void xen_sysbus_class_init(ObjectClass *klass, void *data)
+{
+    HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
+
+    hc->unplug = qdev_simple_device_unplug_cb;
+}
+
 static const TypeInfo xensysbus_info = {
     .name       = TYPE_XENSYSBUS,
     .parent     = TYPE_BUS,
+    .class_init = xen_sysbus_class_init,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_HOTPLUG_HANDLER },
         { }
@@ -627,6 +673,7 @@  static void xenbe_register_types(void)
 {
     type_register_static(&xensysbus_info);
     type_register_static(&xensysdev_info);
+    type_register_static(&xendev_type_info);
 }
 
 type_init(xenbe_register_types)
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index 5212bc6..aed783e 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -19,6 +19,7 @@ 
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
+#include "hw/qdev-core.h"
 #include "hw/xen/xen_backend.h"
 #include "hw/xen/xen_pvdev.h"
 
@@ -307,7 +308,8 @@  void xen_pv_del_xendev(struct XenDevice *xendev)
     }
 
     QTAILQ_REMOVE(&xendevs, xendev, next);
-    g_free(xendev);
+
+    qdev_unplug(&xendev->qdev, NULL);
 }
 
 void xen_pv_insert_xendev(struct XenDevice *xendev)
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index 38f730e..4f4799a 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -8,6 +8,10 @@ 
 
 #define TYPE_XENSYSDEV "xen-sysdev"
 #define TYPE_XENSYSBUS "xen-sysbus"
+#define TYPE_XENBACKEND "xen-backend"
+
+#define XENBACKEND_DEVICE(obj) \
+    OBJECT_CHECK(XenDevice, (obj), TYPE_XENBACKEND)
 
 /* variables */
 extern xc_interface *xen_xc;
diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h
index 083f0a9..d473e9b 100644
--- a/include/hw/xen/xen_pvdev.h
+++ b/include/hw/xen/xen_pvdev.h
@@ -29,6 +29,7 @@  struct XenDevOps {
 };
 
 struct XenDevice {
+    DeviceState        qdev;
     const char         *type;
     int                dom;
     int                dev;