[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/40] qdev-ify: xen backends
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 06/40] qdev-ify: xen backends |
Date: |
Tue, 02 Nov 2010 11:08:55 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Alexander Graf <address@hidden> writes:
> From: Gerd Hoffmann <address@hidden>
>
> This patch converts the xen backend code to qdev.
qdev conversions are always welcome. This one's not complete (search
for #if 0). The commit message should state that.
Two questions inline.
> Signed-off-by: Gerd Hoffmann <address@hidden>
> Signed-off-by: Alexander Graf <address@hidden>
> ---
> hw/xen_backend.c | 176
> ++++++++++++++++++++++++++++++++-------------------
> hw/xen_backend.h | 9 ++-
> hw/xen_console.c | 10 +++-
> hw/xen_disk.c | 10 +++-
> hw/xen_machine_pv.c | 6 +--
> hw/xen_nic.c | 10 +++-
> hw/xenfb.c | 14 ++++-
> 7 files changed, 158 insertions(+), 77 deletions(-)
>
> diff --git a/hw/xen_backend.c b/hw/xen_backend.c
> index a2e408f..0d6a96b 100644
> --- a/hw/xen_backend.c
> +++ b/hw/xen_backend.c
> @@ -42,13 +42,21 @@
>
> /* ------------------------------------------------------------- */
>
> +typedef struct XenBus {
> + BusState qbus;
> +} XenBus;
> +
> /* public */
> int xen_xc;
> struct xs_handle *xenstore = NULL;
> const char *xen_protocol;
>
> /* private */
> -static QTAILQ_HEAD(XenDeviceHead, XenDevice) xendevs =
> QTAILQ_HEAD_INITIALIZER(xendevs);
> +static struct BusInfo xen_bus_info = {
> + .name = "Xen",
> + .size = sizeof(XenBus),
> +};
> +static XenBus *xenbus;
> static int debug = 0;
>
> /* ------------------------------------------------------------- */
> @@ -163,14 +171,16 @@ int xen_be_set_state(struct XenDevice *xendev, enum
> xenbus_state state)
>
> struct XenDevice *xen_be_find_xendev(const char *type, int dom, int dev)
> {
> + struct DeviceState *qdev;
> struct XenDevice *xendev;
>
> - QTAILQ_FOREACH(xendev, &xendevs, next) {
> + QLIST_FOREACH(qdev, &xenbus->qbus.children, sibling) {
> + xendev = container_of(qdev, struct XenDevice, qdev);
> if (xendev->dom != dom)
> continue;
> if (xendev->dev != dev)
> continue;
> - if (strcmp(xendev->type, type) != 0)
> + if (strcmp(xendev->ops->type, type) != 0)
> continue;
> return xendev;
> }
> @@ -180,28 +190,34 @@ struct XenDevice *xen_be_find_xendev(const char *type,
> int dom, int dev)
> /*
> * get xen backend device, allocate a new one if it doesn't exist.
> */
> -static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int
> dev,
> +static struct XenDevice *xen_be_get_xendev(int dom, int dev,
> struct XenDevOps *ops)
> {
> + struct DeviceState *qdev;
> struct XenDevice *xendev;
> + char name[64];
> char *dom0;
>
> - xendev = xen_be_find_xendev(type, dom, dev);
> + xendev = xen_be_find_xendev(ops->type, dom, dev);
> if (xendev)
> return xendev;
>
> + /* create new xendev */
> + snprintf(name, sizeof(name), "xen-%s", ops->type);
> + qdev = qdev_create(&xenbus->qbus, name);
> + qdev_init_nofail(qdev);
> + xendev = container_of(qdev, struct XenDevice, qdev);
> +
> /* init new xendev */
> - xendev = qemu_mallocz(ops->size);
> - xendev->type = type;
> xendev->dom = dom;
> xendev->dev = dev;
> xendev->ops = ops;
>
> dom0 = xs_get_domain_path(xenstore, 0);
> snprintf(xendev->be, sizeof(xendev->be), "%s/backend/%s/%d/%d",
> - dom0, xendev->type, xendev->dom, xendev->dev);
> + dom0, xendev->ops->type, xendev->dom, xendev->dev);
> snprintf(xendev->name, sizeof(xendev->name), "%s-%d",
> - xendev->type, xendev->dev);
> + xendev->ops->type, xendev->dev);
> free(dom0);
>
> xendev->debug = debug;
> @@ -210,7 +226,7 @@ static struct XenDevice *xen_be_get_xendev(const char
> *type, int dom, int dev,
> xendev->evtchndev = xc_evtchn_open();
> if (xendev->evtchndev < 0) {
> xen_be_printf(NULL, 0, "can't open evtchn device\n");
> - qemu_free(xendev);
> + qdev_free(&xendev->qdev);
> return NULL;
> }
> fcntl(xc_evtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
> @@ -220,15 +236,13 @@ static struct XenDevice *xen_be_get_xendev(const char
> *type, int dom, int dev,
> if (xendev->gnttabdev < 0) {
> xen_be_printf(NULL, 0, "can't open gnttab device\n");
> xc_evtchn_close(xendev->evtchndev);
> - qemu_free(xendev);
> + qdev_free(&xendev->qdev);
> return NULL;
> }
> } else {
> xendev->gnttabdev = -1;
> }
>
> - QTAILQ_INSERT_TAIL(&xendevs, xendev, next);
> -
> if (xendev->ops->alloc)
> xendev->ops->alloc(xendev);
>
> @@ -238,43 +252,44 @@ static struct XenDevice *xen_be_get_xendev(const char
> *type, int dom, int dev,
> /*
> * release xen backend device.
> */
> -static struct XenDevice *xen_be_del_xendev(int dom, int dev)
> +static void xen_be_del_xendev(int dom, int dev, struct XenDevOps *ops)
> {
> - struct XenDevice *xendev, *xnext;
> -
> - /*
> - * This is pretty much like QTAILQ_FOREACH(xendev, &xendevs, next) but
> - * we save the next pointer in xnext because we might free xendev.
> - */
> - xnext = xendevs.tqh_first;
> - while (xnext) {
> - xendev = xnext;
> - xnext = xendev->next.tqe_next;
> -
> - if (xendev->dom != dom)
> - continue;
> - if (xendev->dev != dev && dev != -1)
> - continue;
> -
> - if (xendev->ops->free)
> - xendev->ops->free(xendev);
> -
> - if (xendev->fe) {
> - char token[XEN_BUFSIZE];
> - snprintf(token, sizeof(token), "fe:%p", xendev);
> - xs_unwatch(xenstore, xendev->fe, token);
> - qemu_free(xendev->fe);
> - }
> -
> - if (xendev->evtchndev >= 0)
> - xc_evtchn_close(xendev->evtchndev);
> - if (xendev->gnttabdev >= 0)
> - xc_gnttab_close(xendev->gnttabdev);
> -
> - QTAILQ_REMOVE(&xendevs, xendev, next);
> - qemu_free(xendev);
> - }
> - return NULL;
> + struct DeviceState *qdev;
> + struct XenDevice *xendev;
> + int done;
> +
> + do {
> + done = 1;
> + QLIST_FOREACH(qdev, &xenbus->qbus.children, sibling) {
> + xendev = container_of(qdev, struct XenDevice, qdev);
> + if (xendev->dom != dom)
> + continue;
> + if (xendev->dev != dev && dev != -1)
> + continue;
> + if (xendev->ops != ops)
> + continue;
> +
> + if (xendev->ops->free)
> + xendev->ops->free(xendev);
> +
> + if (xendev->fe) {
> + char token[XEN_BUFSIZE];
> + snprintf(token, sizeof(token), "fe:%p", xendev);
> + xs_unwatch(xenstore, xendev->fe, token);
> + qemu_free(xendev->fe);
> + }
> +
> + if (xendev->evtchndev >= 0)
> + xc_evtchn_close(xendev->evtchndev);
> + if (xendev->gnttabdev >= 0)
> + xc_gnttab_close(xendev->gnttabdev);
> +
> + qdev_free(&xendev->qdev);
> +
> + done = 0;
> + break;
> + }
> + } while (!done);
This loop nest confuses me. Why can't we just QLIST_FOREACH_SAFE()?
> }
>
> /*
> @@ -498,7 +513,7 @@ void xen_be_check_state(struct XenDevice *xendev)
>
> /* ------------------------------------------------------------- */
>
> -static int xenstore_scan(const char *type, int dom, struct XenDevOps *ops)
> +static int xenstore_scan(int dom, struct XenDevOps *ops)
> {
> struct XenDevice *xendev;
> char path[XEN_BUFSIZE], token[XEN_BUFSIZE];
> @@ -507,8 +522,8 @@ static int xenstore_scan(const char *type, int dom,
> struct XenDevOps *ops)
>
> /* setup watch */
> dom0 = xs_get_domain_path(xenstore, 0);
> - snprintf(token, sizeof(token), "be:%p:%d:%p", type, dom, ops);
> - snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom);
> + snprintf(token, sizeof(token), "be:%d:%p", dom, ops);
Why drop %p, type from token?
> + snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, ops->type, dom);
> free(dom0);
> if (!xs_watch(xenstore, path, token)) {
> xen_be_printf(NULL, 0, "xen be: watching backend path (%s) failed\n",
> path);
> @@ -520,7 +535,7 @@ static int xenstore_scan(const char *type, int dom,
> struct XenDevOps *ops)
> if (!dev)
> return 0;
> for (j = 0; j < cdev; j++) {
> - xendev = xen_be_get_xendev(type, dom, atoi(dev[j]), ops);
> + xendev = xen_be_get_xendev(dom, atoi(dev[j]), ops);
> if (xendev == NULL)
> continue;
> xen_be_check_state(xendev);
> @@ -529,15 +544,14 @@ static int xenstore_scan(const char *type, int dom,
> struct XenDevOps *ops)
> return 0;
> }
>
> -static void xenstore_update_be(char *watch, char *type, int dom,
> - struct XenDevOps *ops)
> +static void xenstore_update_be(char *watch, int dom, struct XenDevOps *ops)
> {
> struct XenDevice *xendev;
> char path[XEN_BUFSIZE], *dom0;
> unsigned int len, dev;
>
> dom0 = xs_get_domain_path(xenstore, 0);
> - len = snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, type, dom);
> + len = snprintf(path, sizeof(path), "%s/backend/%s/%d", dom0, ops->type,
> dom);
> free(dom0);
> if (strncmp(path, watch, len) != 0)
> return;
> @@ -551,10 +565,10 @@ static void xenstore_update_be(char *watch, char *type,
> int dom,
>
> if (0) {
> /* FIXME: detect devices being deleted from xenstore ... */
> - xen_be_del_xendev(dom, dev);
> + xen_be_del_xendev(dom, dev, ops);
> }
>
> - xendev = xen_be_get_xendev(type, dom, dev, ops);
> + xendev = xen_be_get_xendev(dom, dev, ops);
> if (xendev != NULL) {
> xen_be_backend_changed(xendev, path);
> xen_be_check_state(xendev);
> @@ -580,16 +594,16 @@ static void xenstore_update_fe(char *watch, struct
> XenDevice *xendev)
> static void xenstore_update(void *unused)
> {
> char **vec = NULL;
> - intptr_t type, ops, ptr;
> + intptr_t ops, ptr;
> unsigned int dom, count;
>
> vec = xs_read_watch(xenstore, &count);
> if (vec == NULL)
> goto cleanup;
>
> - if (sscanf(vec[XS_WATCH_TOKEN], "be:%" PRIxPTR ":%d:%" PRIxPTR,
> - &type, &dom, &ops) == 3)
> - xenstore_update_be(vec[XS_WATCH_PATH], (void*)type, dom, (void*)ops);
> + if (sscanf(vec[XS_WATCH_TOKEN], "be:%d:%" PRIxPTR,
> + &dom, &ops) == 2)
> + xenstore_update_be(vec[XS_WATCH_PATH], dom, (void*)ops);
> if (sscanf(vec[XS_WATCH_TOKEN], "fe:%" PRIxPTR, &ptr) == 1)
> xenstore_update_fe(vec[XS_WATCH_PATH], (void*)ptr);
>
> @@ -642,9 +656,43 @@ err:
> return -1;
> }
>
> -int xen_be_register(const char *type, struct XenDevOps *ops)
> +void xen_create_bus(DeviceState *parent)
> {
> - return xenstore_scan(type, xen_domid, ops);
> + DeviceInfo *info;
> + BusState *qbus;
> +
> + qbus = qbus_create(&xen_bus_info, parent, NULL);
> + xenbus = DO_UPCAST(XenBus, qbus, qbus);
> + for (info = device_info_list; info != NULL; info = info->next) {
> + if (info->bus_info != &xen_bus_info)
> + continue;
> + xenstore_scan(xen_domid, DO_UPCAST(struct XenDevOps, qinfo, info));
> + }
> +
> + qbus->allow_hotplug = 1;
> +}
> +
> +static int xen_be_initfn(DeviceState *dev, DeviceInfo *info)
> +{
> +#if 0
> + struct XenDevOps *ops = container_of(info, struct XenDevOps, qinfo);
> + struct XenDevice *xendev = container_of(dev, struct XenDevice, qdev);
> +
> + /* nothing to do as create + init isn't really splitted. */
> +#endif
> + return 0;
> +}
> +
> +void xen_qdev_register(struct XenDevOps *ops)
> +{
> + char name[64];
> +
> + snprintf(name, sizeof(name), "xen-%s", ops->type);
> + ops->qinfo.name = qemu_strdup(name);
> + ops->qinfo.init = xen_be_initfn;
> + ops->qinfo.bus_info = &xen_bus_info;
> + ops->qinfo.no_user = 1,
> + qdev_register(&ops->qinfo);
> }
>
> int xen_be_bind_evtchn(struct XenDevice *xendev)
> diff --git a/hw/xen_backend.h b/hw/xen_backend.h
> index 1b428e3..f53a742 100644
> --- a/hw/xen_backend.h
> +++ b/hw/xen_backend.h
> @@ -4,6 +4,7 @@
> #include "xen_common.h"
> #include "sysemu.h"
> #include "net.h"
> +#include "qdev.h"
>
> /* ------------------------------------------------------------- */
>
> @@ -17,7 +18,8 @@ struct XenDevice;
> #define DEVOPS_FLAG_IGNORE_STATE 2
>
> struct XenDevOps {
> - size_t size;
> + DeviceInfo qinfo;
> + char type[64];
> uint32_t flags;
> void (*alloc)(struct XenDevice *xendev);
> int (*init)(struct XenDevice *xendev);
> @@ -30,7 +32,7 @@ struct XenDevOps {
> };
>
> struct XenDevice {
> - const char *type;
> + DeviceState qdev;
> int dom;
> int dev;
> char name[64];
> @@ -78,7 +80,8 @@ void xen_be_check_state(struct XenDevice *xendev);
>
> /* xen backend driver bits */
> int xen_be_init(void);
> -int xen_be_register(const char *type, struct XenDevOps *ops);
> +void xen_create_bus(DeviceState *parent);
> +void xen_qdev_register(struct XenDevOps *ops);
> int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state);
> int xen_be_bind_evtchn(struct XenDevice *xendev);
> void xen_be_unbind_evtchn(struct XenDevice *xendev);
> diff --git a/hw/xen_console.c b/hw/xen_console.c
> index d2261f4..a980dc8 100644
> --- a/hw/xen_console.c
> +++ b/hw/xen_console.c
> @@ -260,10 +260,18 @@ static void con_event(struct XenDevice *xendev)
> /* -------------------------------------------------------------------- */
>
> struct XenDevOps xen_console_ops = {
> - .size = sizeof(struct XenConsole),
> + .qinfo.size = sizeof(struct XenConsole),
> + .type = "console",
> .flags = DEVOPS_FLAG_IGNORE_STATE,
> .init = con_init,
> .connect = con_connect,
> .event = con_event,
> .disconnect = con_disconnect,
> };
> +
> +static void xen_console_register_devices(void)
> +{
> + xen_qdev_register(&xen_console_ops);
> +}
> +
> +device_init(xen_console_register_devices)
> diff --git a/hw/xen_disk.c b/hw/xen_disk.c
> index 06752de..5392f58 100644
> --- a/hw/xen_disk.c
> +++ b/hw/xen_disk.c
> @@ -774,7 +774,8 @@ static void blk_event(struct XenDevice *xendev)
> }
>
> struct XenDevOps xen_blkdev_ops = {
> - .size = sizeof(struct XenBlkDev),
> + .qinfo.size = sizeof(struct XenBlkDev),
> + .type = "qdisk",
> .flags = DEVOPS_FLAG_NEED_GNTDEV,
> .alloc = blk_alloc,
> .init = blk_init,
> @@ -783,3 +784,10 @@ struct XenDevOps xen_blkdev_ops = {
> .event = blk_event,
> .free = blk_free,
> };
> +
> +static void xen_blkdev_register_devices(void)
> +{
> + xen_qdev_register(&xen_blkdev_ops);
> +}
> +
> +device_init(xen_blkdev_register_devices)
> diff --git a/hw/xen_machine_pv.c b/hw/xen_machine_pv.c
> index 77a34bf..b94d6e9 100644
> --- a/hw/xen_machine_pv.c
> +++ b/hw/xen_machine_pv.c
> @@ -75,11 +75,7 @@ static void xen_init_pv(ram_addr_t ram_size,
> break;
> }
>
> - xen_be_register("console", &xen_console_ops);
> - xen_be_register("vkbd", &xen_kbdmouse_ops);
> - xen_be_register("vfb", &xen_framebuffer_ops);
> - xen_be_register("qdisk", &xen_blkdev_ops);
> - xen_be_register("qnic", &xen_netdev_ops);
> + xen_create_bus(NULL);
>
> /* configure framebuffer */
> if (xenfb_enabled) {
> diff --git a/hw/xen_nic.c b/hw/xen_nic.c
> index 08055b8..02d3c4e 100644
> --- a/hw/xen_nic.c
> +++ b/hw/xen_nic.c
> @@ -411,7 +411,8 @@ static int net_free(struct XenDevice *xendev)
> /* ------------------------------------------------------------- */
>
> struct XenDevOps xen_netdev_ops = {
> - .size = sizeof(struct XenNetDev),
> + .qinfo.size = sizeof(struct XenNetDev),
> + .type = "qnic",
> .flags = DEVOPS_FLAG_NEED_GNTDEV,
> .init = net_init,
> .connect = net_connect,
> @@ -419,3 +420,10 @@ struct XenDevOps xen_netdev_ops = {
> .disconnect = net_disconnect,
> .free = net_free,
> };
> +
> +static void xen_netdev_register_devices(void)
> +{
> + xen_qdev_register(&xen_netdev_ops);
> +}
> +
> +device_init(xen_netdev_register_devices)
> diff --git a/hw/xenfb.c b/hw/xenfb.c
> index da5297b..293210f 100644
> --- a/hw/xenfb.c
> +++ b/hw/xenfb.c
> @@ -953,7 +953,8 @@ static void fb_event(struct XenDevice *xendev)
> /* -------------------------------------------------------------------- */
>
> struct XenDevOps xen_kbdmouse_ops = {
> - .size = sizeof(struct XenInput),
> + .qinfo.size = sizeof(struct XenInput),
> + .type = "vkbd",
> .init = input_init,
> .connect = input_connect,
> .disconnect = input_disconnect,
> @@ -961,7 +962,8 @@ struct XenDevOps xen_kbdmouse_ops = {
> };
>
> struct XenDevOps xen_framebuffer_ops = {
> - .size = sizeof(struct XenFB),
> + .qinfo.size = sizeof(struct XenFB),
> + .type = "vfb",
> .init = fb_init,
> .connect = fb_connect,
> .disconnect = fb_disconnect,
> @@ -1011,3 +1013,11 @@ wait_more:
> xen_be_check_state(xin);
> xen_be_check_state(xfb);
> }
> +
> +static void xenfb_register_devices(void)
> +{
> + xen_qdev_register(&xen_kbdmouse_ops);
> + xen_qdev_register(&xen_framebuffer_ops);
> +}
> +
> +device_init(xenfb_register_devices)
- [Qemu-devel] Re: [PATCH 35/40] xenner: Domain Builder, (continued)
[Qemu-devel] [PATCH 19/40] xenner: kernel: Makefile, Alexander Graf, 2010/11/01
[Qemu-devel] [PATCH 14/40] xenner: kernel: Instruction emulator, Alexander Graf, 2010/11/01
[Qemu-devel] [PATCH 30/40] xenner: libxc emu: memory mapping, Alexander Graf, 2010/11/01
[Qemu-devel] [PATCH 06/40] qdev-ify: xen backends, Alexander Graf, 2010/11/01
- Re: [Qemu-devel] [PATCH 06/40] qdev-ify: xen backends,
Markus Armbruster <=
[Qemu-devel] [PATCH 24/40] xenner: kernel: printk, Alexander Graf, 2010/11/01
[Qemu-devel] [PATCH 25/40] xenner: kernel: KVM PV code, Alexander Graf, 2010/11/01
[Qemu-devel] [PATCH 02/40] elf: Add notes implementation, Alexander Graf, 2010/11/01