qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL 06/25] xen: create xenstore areas for XenDevice-s


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] [PULL 06/25] xen: create xenstore areas for XenDevice-s
Date: Thu, 2 Apr 2020 14:07:22 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

On 4/2/20 11:49 AM, Paul Durrant wrote:
-----Original Message-----
From: Philippe Mathieu-Daudé <address@hidden>
Sent: 01 April 2020 17:14
To: Anthony PERARD <address@hidden>; address@hidden
Cc: address@hidden; Peter Maydell <address@hidden>; Paul Durrant
<address@hidden>; Markus Armbruster <address@hidden>
Subject: Re: [Qemu-devel] [PULL 06/25] xen: create xenstore areas for 
XenDevice-s

Hi Anthony, Paul.

Cc'ing Markus too.

On 1/14/19 2:51 PM, Anthony PERARD wrote:
From: Paul Durrant <address@hidden>

This patch adds a new source module, xen-bus-helper.c, which builds on
basic libxenstore primitives to provide functions to create (setting
permissions appropriately) and destroy xenstore areas, and functions to
'printf' and 'scanf' nodes therein. The main xen-bus code then uses
these primitives [1] to initialize and destroy the frontend and backend
areas for a XenDevice during realize and unrealize respectively.

The 'xen-block' implementation is extended with a 'get_name' method that
returns the VBD number. This number is required to 'name' the xenstore
areas.

NOTE: An exit handler is also added to make sure the xenstore areas are
        cleaned up if QEMU terminates without devices being unrealized.

[1] The 'scanf' functions are actually not yet needed, but they will be
      needed by code delivered in subsequent patches.

Signed-off-by: Paul Durrant <address@hidden>
Reviewed-by: Anthony Perard <address@hidden>
Signed-off-by: Anthony PERARD <address@hidden>
---
   hw/block/xen-block.c            |   9 +
   hw/xen/Makefile.objs            |   2 +-
   hw/xen/trace-events             |  12 +-
   hw/xen/xen-bus-helper.c         | 150 +++++++++++++++
   hw/xen/xen-bus.c                | 321 +++++++++++++++++++++++++++++++-
   include/hw/xen/xen-bus-helper.h |  39 ++++
   include/hw/xen/xen-bus.h        |  12 ++
   7 files changed, 540 insertions(+), 5 deletions(-)
   create mode 100644 hw/xen/xen-bus-helper.c
   create mode 100644 include/hw/xen/xen-bus-helper.h

[...]
+static void xen_device_exit(Notifier *n, void *data)
+{
+    XenDevice *xendev = container_of(n, XenDevice, exit);
+
+    xen_device_unrealize(DEVICE(xendev), &error_abort);
   }

   static void xen_device_realize(DeviceState *dev, Error **errp)
   {
       XenDevice *xendev = XEN_DEVICE(dev);
       XenDeviceClass *xendev_class = XEN_DEVICE_GET_CLASS(xendev);
+    XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
       const char *type = object_get_typename(OBJECT(xendev));
       Error *local_err = NULL;

-    trace_xen_device_realize(type);
+    if (xendev->frontend_id == DOMID_INVALID) {
+        xendev->frontend_id = xen_domid;
+    }
+
+    if (xendev->frontend_id >= DOMID_FIRST_RESERVED) {
+        error_setg(errp, "invalid frontend-id");
+        goto unrealize;
+    }
+
+    if (!xendev_class->get_name) {
+        error_setg(errp, "get_name method not implemented");
+        goto unrealize;
+    }
+
+    xendev->name = xendev_class->get_name(xendev, &local_err);
+    if (local_err) {
+        error_propagate_prepend(errp, local_err,
+                                "failed to get device name: ");
+        goto unrealize;
+    }
+
+    trace_xen_device_realize(type, xendev->name);
+
+    xen_device_backend_create(xendev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto unrealize;
+    }
+
+    xen_device_frontend_create(xendev, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto unrealize;
+    }

       if (xendev_class->realize) {
           xendev_class->realize(xendev, &local_err);
@@ -72,18 +364,43 @@ static void xen_device_realize(DeviceState *dev, Error 
**errp)
           }
       }

+    xen_device_backend_printf(xendev, "frontend", "%s",
+                              xendev->frontend_path);
+    xen_device_backend_printf(xendev, "frontend-id", "%u",
+                              xendev->frontend_id);
+    xen_device_backend_printf(xendev, "online", "%u", 1);
+    xen_device_backend_printf(xendev, "hotplug-status", "connected");
+
+    xen_device_backend_set_state(xendev, XenbusStateInitWait);
+
+    xen_device_frontend_printf(xendev, "backend", "%s",
+                               xendev->backend_path);
+    xen_device_frontend_printf(xendev, "backend-id", "%u",
+                               xenbus->backend_id);
+
+    xen_device_frontend_set_state(xendev, XenbusStateInitialising);
+
+    xendev->exit.notify = xen_device_exit;
+    qemu_add_exit_notifier(&xendev->exit);
       return;

   unrealize:
       xen_device_unrealize(dev, &error_abort);

It seems if unrealize() fails, the error stored in &local_err is never
reported. Not sure if this can be improved although.

In this case that's essentially a "don't care". We want to know why the realize 
failed but if the unrealize fails something is probably pretty seriously screwed (hence 
the error_abort).

OK. I was surprised by the singular pattern (which confuses Coccinelle semantic transformations), but it works.

Thanks for the quick feedback Paul!

Regards,

Phil.


   Paul





reply via email to

[Prev in Thread] Current Thread [Next in Thread]