xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Xen pvUSB correction
@ 2016-09-26 12:43 Juergen Gross
  2016-09-26 12:43 ` [PATCH 1/2] xen: add an own bus for xen backend devices Juergen Gross
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Juergen Gross @ 2016-09-26 12:43 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: anthony.perard, Juergen Gross, sstabellini, kraxel

Trying to use pvUSB in a Xen guest with a qemu emulated USB controller
will crash qemu as it tries to attach a pvUSB device to the emulated
controller.

This can be avoided by adding a unique id to each pvUSB controller which
can be used when attaching the pvUSB device. In order to make this
possible the pvUSB controller has to be a hotpluggable qemu device.

Juergen Gross (2):
  xen: add an own bus for xen backend devices
  xen: add qemu device for each pvusb backend

 hw/usb/xen-usb.c             | 81 +++++++++++++++++++++++++++++++++++++-------
 hw/xen/xen_backend.c         | 19 +++++++++--
 include/hw/xen/xen_backend.h |  4 +++
 3 files changed, 88 insertions(+), 16 deletions(-)

-- 
2.6.6


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] xen: add an own bus for xen backend devices
  2016-09-26 12:43 [PATCH 0/2] Xen pvUSB correction Juergen Gross
@ 2016-09-26 12:43 ` Juergen Gross
  2016-09-27  8:53   ` Gerd Hoffmann
  2016-09-26 12:43 ` [PATCH 2/2] xen: add qemu device for each pvusb backend Juergen Gross
  2016-09-27  8:51 ` [PATCH 0/2] Xen pvUSB correction Gerd Hoffmann
  2 siblings, 1 reply; 13+ messages in thread
From: Juergen Gross @ 2016-09-26 12:43 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: anthony.perard, Juergen Gross, sstabellini, kraxel

Add a bus for Xen backend devices in order to be able to establish a
dedicated device path for pluggable devices.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 hw/xen/xen_backend.c         | 19 ++++++++++++++++---
 include/hw/xen/xen_backend.h |  4 ++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 69a2388..687adf4 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -29,13 +29,13 @@
 #include "hw/sysbus.h"
 #include "sysemu/char.h"
 #include "qemu/log.h"
+#include "qapi/error.h"
 #include "hw/xen/xen_backend.h"
 
 #include <xen/grant_table.h>
 
-#define TYPE_XENSYSDEV "xensysdev"
-
 DeviceState *xen_sysdev;
+BusState *xen_sysbus;
 
 /* ------------------------------------------------------------- */
 
@@ -750,6 +750,8 @@ int xen_be_init(void)
 
     xen_sysdev = qdev_create(NULL, TYPE_XENSYSDEV);
     qdev_init_nofail(xen_sysdev);
+    xen_sysbus = qbus_create(TYPE_XENSYSBUS, DEVICE(xen_sysdev), "xen-sysbus");
+    qbus_set_bus_hotplug_handler(xen_sysbus, &error_abort);
 
     return 0;
 
@@ -862,6 +864,15 @@ void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, ...
     qemu_log_flush();
 }
 
+static const TypeInfo xensysbus_info = {
+    .name       = TYPE_XENSYSBUS,
+    .parent     = TYPE_BUS,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_HOTPLUG_HANDLER },
+        { }
+    }
+};
+
 static int xen_sysdev_init(SysBusDevice *dev)
 {
     return 0;
@@ -878,6 +889,7 @@ static void xen_sysdev_class_init(ObjectClass *klass, void *data)
 
     k->init = xen_sysdev_init;
     dc->props = xen_sysdev_properties;
+    dc->bus_type = TYPE_XENSYSBUS;
 }
 
 static const TypeInfo xensysdev_info = {
@@ -889,7 +901,8 @@ static const TypeInfo xensysdev_info = {
 
 static void xenbe_register_types(void)
 {
+    type_register_static(&xensysbus_info);
     type_register_static(&xensysdev_info);
 }
 
-type_init(xenbe_register_types);
+type_init(xenbe_register_types)
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index 0df282a..4087231 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -54,6 +54,9 @@ struct XenDevice {
     QTAILQ_ENTRY(XenDevice) next;
 };
 
+#define TYPE_XENSYSDEV "xensysdev"
+#define TYPE_XENSYSBUS "xen-sysbus"
+
 /* ------------------------------------------------------------- */
 
 /* variables */
@@ -62,6 +65,7 @@ extern xenforeignmemory_handle *xen_fmem;
 extern struct xs_handle *xenstore;
 extern const char *xen_protocol;
 extern DeviceState *xen_sysdev;
+extern BusState *xen_sysbus;
 
 /* xenstore helper functions */
 int xenstore_mkdir(char *path, int p);
-- 
2.6.6


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] xen: add qemu device for each pvusb backend
  2016-09-26 12:43 [PATCH 0/2] Xen pvUSB correction Juergen Gross
  2016-09-26 12:43 ` [PATCH 1/2] xen: add an own bus for xen backend devices Juergen Gross
@ 2016-09-26 12:43 ` Juergen Gross
  2016-09-27  9:00   ` Daniel P. Berrange
  2016-09-27  9:08   ` Gerd Hoffmann
  2016-09-27  8:51 ` [PATCH 0/2] Xen pvUSB correction Gerd Hoffmann
  2 siblings, 2 replies; 13+ messages in thread
From: Juergen Gross @ 2016-09-26 12:43 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: anthony.perard, Juergen Gross, sstabellini, kraxel

In order to be able to specify to which pvusb controller a new pvusb
device should be added we need a qemu device for each pvusb controller
with an associated id.

Add such a device when a new controller is requested and attach the
usb bus of that controller to the new device. Any device connected to
that controller can now specify the bus and port directly via its
properties.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 hw/usb/xen-usb.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 68 insertions(+), 13 deletions(-)

diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index 174d715..439d104 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -29,6 +29,7 @@
 #include "hw/usb.h"
 #include "hw/xen/xen_backend.h"
 #include "monitor/qdev.h"
+#include "qapi/error.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qstring.h"
@@ -47,12 +48,16 @@
         struct timeval tv;                                          \
                                                                     \
         gettimeofday(&tv, NULL);                                    \
-        xen_be_printf(xendev, lvl, "%8ld.%06ld xen-usb(%s):" fmt,   \
+        xen_be_printf(xendev, 0, "%8ld.%06ld xen-usb(%s):" fmt,   \
                       tv.tv_sec, tv.tv_usec, __func__, ##args);     \
     }
 #define TR_BUS(xendev, fmt, args...) TR(xendev, 2, fmt, ##args)
 #define TR_REQ(xendev, fmt, args...) TR(xendev, 3, fmt, ##args)
 
+#define TYPE_USBBACK            "xen-pvusb"
+#define USBBACK_DEVICE(obj) \
+     OBJECT_CHECK(USBBACKDevice, (obj), TYPE_USBBACK)
+
 #define USBBACK_MAXPORTS        USBIF_PIPE_PORT_MASK
 #define USB_DEV_ADDR_SIZE       (USBIF_PIPE_DEV_MASK + 1)
 
@@ -67,6 +72,7 @@ struct usbif_ctrlrequest {
 
 struct usbback_info;
 struct usbback_req;
+struct USBBACKDevice;
 
 struct usbback_stub {
     USBDevice     *dev;
@@ -101,6 +107,8 @@ struct usbback_hotplug {
 
 struct usbback_info {
     struct XenDevice         xendev;  /* must be first */
+    char                     id[24];
+    struct USBBACKDevice     *dev;
     USBBus                   bus;
     void                     *urb_sring;
     void                     *conn_sring;
@@ -116,6 +124,10 @@ struct usbback_info {
     QEMUBH                   *bh;
 };
 
+typedef struct USBBACKDevice {
+    DeviceState qdev;
+} USBBACKDevice;
+
 static struct usbback_req *usbback_get_req(struct usbback_info *usbif)
 {
     struct usbback_req *usbback_req;
@@ -712,15 +724,10 @@ static void usbback_portid_detach(struct usbback_info *usbif, unsigned port)
 
 static void usbback_portid_remove(struct usbback_info *usbif, unsigned port)
 {
-    USBPort *p;
-
     if (!usbif->ports[port - 1].dev) {
         return;
     }
 
-    p = &(usbif->ports[port - 1].port);
-    snprintf(p->path, sizeof(p->path), "%d", 99);
-
     object_unparent(OBJECT(usbif->ports[port - 1].dev));
     usbif->ports[port - 1].dev = NULL;
     usbback_portid_detach(usbif, port);
@@ -733,10 +740,10 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
 {
     unsigned speed;
     char *portname;
-    USBPort *p;
     Error *local_err = NULL;
     QDict *qdict;
     QemuOpts *opts;
+    char tmp[32];
 
     if (usbif->ports[port - 1].dev) {
         return;
@@ -749,11 +756,14 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
         return;
     }
     portname++;
-    p = &(usbif->ports[port - 1].port);
-    snprintf(p->path, sizeof(p->path), "%s", portname);
 
     qdict = qdict_new();
     qdict_put(qdict, "driver", qstring_from_str("usb-host"));
+    snprintf(tmp, sizeof(tmp), "%s.0", usbif->id);
+    qdict_put(qdict, "bus", qstring_from_str(tmp));
+    snprintf(tmp, sizeof(tmp), "%s-%u", usbif->id, port);
+    qdict_put(qdict, "id", qstring_from_str(tmp));
+    qdict_put(qdict, "port", qint_from_int(port));
     qdict_put(qdict, "hostbus", qint_from_int(atoi(busid)));
     qdict_put(qdict, "hostport", qstring_from_str(portname));
     opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, &local_err);
@@ -765,7 +775,6 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
         goto err;
     }
     QDECREF(qdict);
-    snprintf(p->path, sizeof(p->path), "%d", port);
     speed = usbif->ports[port - 1].dev->speed;
     switch (speed) {
     case USB_SPEED_LOW:
@@ -799,7 +808,6 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
 
 err:
     QDECREF(qdict);
-    snprintf(p->path, sizeof(p->path), "%d", 99);
     xen_be_printf(&usbif->xendev, 0, "device %s could not be opened\n", busid);
 }
 
@@ -1009,16 +1017,36 @@ static void usbback_alloc(struct XenDevice *xendev)
     struct usbback_info *usbif;
     USBPort *p;
     unsigned int i, max_grants;
+    Error *local_err = NULL;
+    QDict *qdict;
+    QemuOpts *opts;
 
     usbif = container_of(xendev, struct usbback_info, xendev);
 
-    usb_bus_new(&usbif->bus, sizeof(usbif->bus), &xen_usb_bus_ops, xen_sysdev);
+    snprintf(usbif->id, sizeof(usbif->id), TYPE_USBBACK "-%d", xendev->dev);
+    qdict = qdict_new();
+    qdict_put(qdict, "driver", qstring_from_str(TYPE_USBBACK));
+    qdict_put(qdict, "id", qstring_from_str(usbif->id));
+    opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, &local_err);
+    if (local_err) {
+        QDECREF(qdict);
+        xen_be_printf(xendev, 0, "qemu_opts_from_qdict failed\n");
+        return;
+    }
+    usbif->dev = USBBACK_DEVICE(qdev_device_add(opts, &local_err));
+    QDECREF(qdict);
+    if (!usbif->dev) {
+        xen_be_printf(xendev, 0, "qdev_device_add failed\n");
+        return;
+    }
+
+    usb_bus_new(&usbif->bus, sizeof(usbif->bus), &xen_usb_bus_ops,
+                DEVICE(usbif->dev));
     for (i = 0; i < USBBACK_MAXPORTS; i++) {
         p = &(usbif->ports[i].port);
         usb_register_port(&usbif->bus, p, usbif, i, &xen_usb_port_ops,
                           USB_SPEED_MASK_LOW | USB_SPEED_MASK_FULL |
                           USB_SPEED_MASK_HIGH);
-        snprintf(p->path, sizeof(p->path), "%d", 99);
     }
 
     QTAILQ_INIT(&usbif->req_free_q);
@@ -1067,6 +1095,7 @@ static int usbback_free(struct XenDevice *xendev)
 
     usb_bus_release(&usbif->bus);
     object_unparent(OBJECT(&usbif->bus));
+    object_unparent(OBJECT(usbif->dev));
 
     TR_BUS(xendev, "finished\n");
 
@@ -1093,6 +1122,32 @@ struct XenDevOps xen_usb_ops = {
     .event           = usbback_event,
 };
 
+static Property usbback_properties[] = {
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void usbback_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    TR_BUS(NULL, "\n");
+    dc->props   = usbback_properties;
+    set_bit(DEVICE_CATEGORY_USB, dc->categories);
+}
+
+static const TypeInfo usbback_type_info = {
+    .name          = TYPE_USBBACK,
+    .parent        = TYPE_XENSYSDEV,
+    .class_init    = usbback_class_init,
+    .instance_size = sizeof(USBBACKDevice),
+};
+
+static void usbback_register_types(void)
+{
+    type_register_static(&usbback_type_info);
+}
+
+type_init(usbback_register_types)
 #else /* USBIF_SHORT_NOT_OK */
 
 static int usbback_not_supported(void)
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Xen pvUSB correction
  2016-09-26 12:43 [PATCH 0/2] Xen pvUSB correction Juergen Gross
  2016-09-26 12:43 ` [PATCH 1/2] xen: add an own bus for xen backend devices Juergen Gross
  2016-09-26 12:43 ` [PATCH 2/2] xen: add qemu device for each pvusb backend Juergen Gross
@ 2016-09-27  8:51 ` Gerd Hoffmann
  2016-09-29 14:38   ` Juergen Gross
  2 siblings, 1 reply; 13+ messages in thread
From: Gerd Hoffmann @ 2016-09-27  8:51 UTC (permalink / raw)
  To: Juergen Gross; +Cc: anthony.perard, xen-devel, sstabellini, qemu-devel

On Mo, 2016-09-26 at 14:43 +0200, Juergen Gross wrote:
> Trying to use pvUSB in a Xen guest with a qemu emulated USB controller
> will crash qemu as it tries to attach a pvUSB device to the emulated
> controller.

Hmm.  --verbose please.

While this clearly doesn't do what you intended I think it should not
have crashed qemu.  pvUSB devices should work on emulated controller
(and emulated devices should work on the pvUSB controller).  If they
don't you probably taking shortcuts somewhere which work only for the
pvUSB device on pvUSB controller case.

Please check.

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] xen: add an own bus for xen backend devices
  2016-09-26 12:43 ` [PATCH 1/2] xen: add an own bus for xen backend devices Juergen Gross
@ 2016-09-27  8:53   ` Gerd Hoffmann
  2016-09-29 14:39     ` Juergen Gross
  0 siblings, 1 reply; 13+ messages in thread
From: Gerd Hoffmann @ 2016-09-27  8:53 UTC (permalink / raw)
  To: Juergen Gross; +Cc: anthony.perard, xen-devel, sstabellini, qemu-devel

On Mo, 2016-09-26 at 14:43 +0200, Juergen Gross wrote:
> Add a bus for Xen backend devices in order to be able to establish a
> dedicated device path for pluggable devices.

Looks sane to me.  Can take this through the usb queue if I get an ack
from xen.

> +#define TYPE_XENSYSDEV "xensysdev"
> +#define TYPE_XENSYSBUS "xen-sysbus"

I'd make this consistent and use the dash either for both or not at all.

cheers,
  Gerd



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] xen: add qemu device for each pvusb backend
  2016-09-26 12:43 ` [PATCH 2/2] xen: add qemu device for each pvusb backend Juergen Gross
@ 2016-09-27  9:00   ` Daniel P. Berrange
  2016-09-29 14:39     ` [Qemu-devel] " Juergen Gross
  2016-09-27  9:08   ` Gerd Hoffmann
  1 sibling, 1 reply; 13+ messages in thread
From: Daniel P. Berrange @ 2016-09-27  9:00 UTC (permalink / raw)
  To: Juergen Gross; +Cc: anthony.perard, xen-devel, sstabellini, qemu-devel, kraxel

On Mon, Sep 26, 2016 at 02:43:57PM +0200, Juergen Gross wrote:
> In order to be able to specify to which pvusb controller a new pvusb
> device should be added we need a qemu device for each pvusb controller
> with an associated id.
> 
> Add such a device when a new controller is requested and attach the
> usb bus of that controller to the new device. Any device connected to
> that controller can now specify the bus and port directly via its
> properties.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  hw/usb/xen-usb.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 68 insertions(+), 13 deletions(-)
> 
> @@ -733,10 +740,10 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
>  {
>      unsigned speed;
>      char *portname;
> -    USBPort *p;
>      Error *local_err = NULL;
>      QDict *qdict;
>      QemuOpts *opts;
> +    char tmp[32];
>  
>      if (usbif->ports[port - 1].dev) {
>          return;
> @@ -749,11 +756,14 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
>          return;
>      }
>      portname++;
> -    p = &(usbif->ports[port - 1].port);
> -    snprintf(p->path, sizeof(p->path), "%s", portname);
>  
>      qdict = qdict_new();
>      qdict_put(qdict, "driver", qstring_from_str("usb-host"));
> +    snprintf(tmp, sizeof(tmp), "%s.0", usbif->id);

Don't snprintf into fixed length buffers. g_strdup_printf() does the
right thing

> +    qdict_put(qdict, "bus", qstring_from_str(tmp));
> +    snprintf(tmp, sizeof(tmp), "%s-%u", usbif->id, port);
> +    qdict_put(qdict, "id", qstring_from_str(tmp));


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] xen: add qemu device for each pvusb backend
  2016-09-26 12:43 ` [PATCH 2/2] xen: add qemu device for each pvusb backend Juergen Gross
  2016-09-27  9:00   ` Daniel P. Berrange
@ 2016-09-27  9:08   ` Gerd Hoffmann
  2016-09-29 14:49     ` Juergen Gross
  1 sibling, 1 reply; 13+ messages in thread
From: Gerd Hoffmann @ 2016-09-27  9:08 UTC (permalink / raw)
  To: Juergen Gross; +Cc: anthony.perard, xen-devel, sstabellini, qemu-devel

  Hi,

>  struct usbback_info {
>      struct XenDevice         xendev;  /* must be first */
> +    char                     id[24];
> +    struct USBBACKDevice     *dev;
>      USBBus                   bus;
>      void                     *urb_sring;
>      void                     *conn_sring;
> @@ -116,6 +124,10 @@ struct usbback_info {
>      QEMUBH                   *bh;
>  };
>  
> +typedef struct USBBACKDevice {
> +    DeviceState qdev;
> +} USBBACKDevice;

Hmm, I think the xen core needs better QOM support ...

struct XenDevice should have a DeviceState element, so it can be used as
device object directly instead of attaching a device object like
this ...

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Xen pvUSB correction
  2016-09-27  8:51 ` [PATCH 0/2] Xen pvUSB correction Gerd Hoffmann
@ 2016-09-29 14:38   ` Juergen Gross
  0 siblings, 0 replies; 13+ messages in thread
From: Juergen Gross @ 2016-09-29 14:38 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: anthony.perard, xen-devel, sstabellini, qemu-devel

On 27/09/16 10:51, Gerd Hoffmann wrote:
> On Mo, 2016-09-26 at 14:43 +0200, Juergen Gross wrote:
>> Trying to use pvUSB in a Xen guest with a qemu emulated USB controller
>> will crash qemu as it tries to attach a pvUSB device to the emulated
>> controller.
> 
> Hmm.  --verbose please.
> 
> While this clearly doesn't do what you intended I think it should not
> have crashed qemu.  pvUSB devices should work on emulated controller
> (and emulated devices should work on the pvUSB controller).  If they
> don't you probably taking shortcuts somewhere which work only for the
> pvUSB device on pvUSB controller case.

Of course a pvUSB device connected by the pvUSB controller is
expecting to be on that controller when doing I/Os. I believe this
was the problem here: The device was attached to an emulated USB
controller and the pvUSB controller started an I/O which confused the
emulated one.

> Please check.

There is something wrong, sure. A pvUSB device ending on the wrong
controller should never receive I/Os from the pvUSB controller. I'll
check that. But this problem is independent from the one solved by
these patches: I have to make sure the device is connected to the
pvUSB controller or otherwise the guest won't be able to access it
the way it was meant to.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] xen: add an own bus for xen backend devices
  2016-09-27  8:53   ` Gerd Hoffmann
@ 2016-09-29 14:39     ` Juergen Gross
  0 siblings, 0 replies; 13+ messages in thread
From: Juergen Gross @ 2016-09-29 14:39 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: anthony.perard, xen-devel, sstabellini, qemu-devel

On 27/09/16 10:53, Gerd Hoffmann wrote:
> On Mo, 2016-09-26 at 14:43 +0200, Juergen Gross wrote:
>> Add a bus for Xen backend devices in order to be able to establish a
>> dedicated device path for pluggable devices.
> 
> Looks sane to me.  Can take this through the usb queue if I get an ack
> from xen.
> 
>> +#define TYPE_XENSYSDEV "xensysdev"
>> +#define TYPE_XENSYSBUS "xen-sysbus"
> 
> I'd make this consistent and use the dash either for both or not at all.

Okay, I'll change it to use the dash in both cases.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] xen: add qemu device for each pvusb backend
  2016-09-27  9:00   ` Daniel P. Berrange
@ 2016-09-29 14:39     ` Juergen Gross
  0 siblings, 0 replies; 13+ messages in thread
From: Juergen Gross @ 2016-09-29 14:39 UTC (permalink / raw)
  To: Daniel P. Berrange
  Cc: anthony.perard, xen-devel, sstabellini, qemu-devel, kraxel

On 27/09/16 11:00, Daniel P. Berrange wrote:
> On Mon, Sep 26, 2016 at 02:43:57PM +0200, Juergen Gross wrote:
>> In order to be able to specify to which pvusb controller a new pvusb
>> device should be added we need a qemu device for each pvusb controller
>> with an associated id.
>>
>> Add such a device when a new controller is requested and attach the
>> usb bus of that controller to the new device. Any device connected to
>> that controller can now specify the bus and port directly via its
>> properties.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  hw/usb/xen-usb.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++---------
>>  1 file changed, 68 insertions(+), 13 deletions(-)
>>
>> @@ -733,10 +740,10 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
>>  {
>>      unsigned speed;
>>      char *portname;
>> -    USBPort *p;
>>      Error *local_err = NULL;
>>      QDict *qdict;
>>      QemuOpts *opts;
>> +    char tmp[32];
>>  
>>      if (usbif->ports[port - 1].dev) {
>>          return;
>> @@ -749,11 +756,14 @@ static void usbback_portid_add(struct usbback_info *usbif, unsigned port,
>>          return;
>>      }
>>      portname++;
>> -    p = &(usbif->ports[port - 1].port);
>> -    snprintf(p->path, sizeof(p->path), "%s", portname);
>>  
>>      qdict = qdict_new();
>>      qdict_put(qdict, "driver", qstring_from_str("usb-host"));
>> +    snprintf(tmp, sizeof(tmp), "%s.0", usbif->id);
> 
> Don't snprintf into fixed length buffers. g_strdup_printf() does the
> right thing

Okay, will change it.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] xen: add qemu device for each pvusb backend
  2016-09-27  9:08   ` Gerd Hoffmann
@ 2016-09-29 14:49     ` Juergen Gross
  2016-09-29 15:16       ` Gerd Hoffmann
  0 siblings, 1 reply; 13+ messages in thread
From: Juergen Gross @ 2016-09-29 14:49 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: anthony.perard, xen-devel, sstabellini, qemu-devel

On 27/09/16 11:08, Gerd Hoffmann wrote:
>   Hi,
> 
>>  struct usbback_info {
>>      struct XenDevice         xendev;  /* must be first */
>> +    char                     id[24];
>> +    struct USBBACKDevice     *dev;
>>      USBBus                   bus;
>>      void                     *urb_sring;
>>      void                     *conn_sring;
>> @@ -116,6 +124,10 @@ struct usbback_info {
>>      QEMUBH                   *bh;
>>  };
>>  
>> +typedef struct USBBACKDevice {
>> +    DeviceState qdev;
>> +} USBBACKDevice;
> 
> Hmm, I think the xen core needs better QOM support ...
> 
> struct XenDevice should have a DeviceState element, so it can be used as
> device object directly instead of attaching a device object like
> this ...

Hmm, interesting idea. The device object could even be added in
Xen common code if the backend is indicating the need for it via a
special flag/field. I'll have a try.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] xen: add qemu device for each pvusb backend
  2016-09-29 14:49     ` Juergen Gross
@ 2016-09-29 15:16       ` Gerd Hoffmann
  2016-09-29 19:39         ` Markus Armbruster
  0 siblings, 1 reply; 13+ messages in thread
From: Gerd Hoffmann @ 2016-09-29 15:16 UTC (permalink / raw)
  To: Juergen Gross; +Cc: anthony.perard, xen-devel, sstabellini, qemu-devel

  Hi,

> > Hmm, I think the xen core needs better QOM support ...
> > 
> > struct XenDevice should have a DeviceState element, so it can be used as
> > device object directly instead of attaching a device object like
> > this ...
> 
> Hmm, interesting idea. The device object could even be added in
> Xen common code if the backend is indicating the need for it via a
> special flag/field. I'll have a try.

No, not optional.  Just turn *all* xen devices into QOM objects.

XenDevice should probably a subclass of the base device object
(DeviceState), and all Xen backends (block, net, fb, pvusb, ...)
should be subclasses of XenDevice.

The latter is probably how things are modeled already, just the QOM
object stuff is missing (register classes, macros to cast objects, ...)
because qdev (the QOM predecessor) didn't have that.

Once this is in place you can simply use DEVICE(xendevice) to get the
DeviceState pointer.

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] xen: add qemu device for each pvusb backend
  2016-09-29 15:16       ` Gerd Hoffmann
@ 2016-09-29 19:39         ` Markus Armbruster
  0 siblings, 0 replies; 13+ messages in thread
From: Markus Armbruster @ 2016-09-29 19:39 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Juergen Gross, anthony.perard, xen-devel, sstabellini, qemu-devel

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
>> > Hmm, I think the xen core needs better QOM support ...
>> > 
>> > struct XenDevice should have a DeviceState element, so it can be used as
>> > device object directly instead of attaching a device object like
>> > this ...
>> 
>> Hmm, interesting idea. The device object could even be added in
>> Xen common code if the backend is indicating the need for it via a
>> special flag/field. I'll have a try.
>
> No, not optional.  Just turn *all* xen devices into QOM objects.

Yes, please.

> XenDevice should probably a subclass of the base device object
> (DeviceState), and all Xen backends (block, net, fb, pvusb, ...)
> should be subclasses of XenDevice.
>
> The latter is probably how things are modeled already, just the QOM
> object stuff is missing (register classes, macros to cast objects, ...)
> because qdev (the QOM predecessor) didn't have that.
>
> Once this is in place you can simply use DEVICE(xendevice) to get the
> DeviceState pointer.

Related thread: qdevification of xen_disk

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-09-29 19:39 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-26 12:43 [PATCH 0/2] Xen pvUSB correction Juergen Gross
2016-09-26 12:43 ` [PATCH 1/2] xen: add an own bus for xen backend devices Juergen Gross
2016-09-27  8:53   ` Gerd Hoffmann
2016-09-29 14:39     ` Juergen Gross
2016-09-26 12:43 ` [PATCH 2/2] xen: add qemu device for each pvusb backend Juergen Gross
2016-09-27  9:00   ` Daniel P. Berrange
2016-09-29 14:39     ` [Qemu-devel] " Juergen Gross
2016-09-27  9:08   ` Gerd Hoffmann
2016-09-29 14:49     ` Juergen Gross
2016-09-29 15:16       ` Gerd Hoffmann
2016-09-29 19:39         ` Markus Armbruster
2016-09-27  8:51 ` [PATCH 0/2] Xen pvUSB correction Gerd Hoffmann
2016-09-29 14:38   ` Juergen Gross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).