[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |