[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



On 4/2/20 11:49 AM, Paul Durrant wrote:
-----Original Message-----
From: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
Sent: 01 April 2020 17:14
To: Anthony PERARD <anthony.perard@xxxxxxxxxx>; qemu-devel@xxxxxxxxxx
Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Peter Maydell <peter.maydell@xxxxxxxxxx>; 
Paul Durrant
<paul@xxxxxxx>; Markus Armbruster <armbru@xxxxxxxxxx>
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 <paul.durrant@xxxxxxxxxx>

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 <paul.durrant@xxxxxxxxxx>
Reviewed-by: Anthony Perard <anthony.perard@xxxxxxxxxx>
Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
---
   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





 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.