On Jul 17, 2016 10:41, "Quan Xu" <quan....@aliyun.com> wrote: > > > [Quan:]: comment starts with [Quan:] > Thanks, Quan for your comments.
The first patches from this series just move some code from xen_backend to xen_pvdev file. I would not group the reorg from xen_backend with refactoring in the same patch. Eventually this can be done in another patch later. > > > The purpose of the new file is to store generic functions shared by frontend > and backends such as xenstore operations, xendevs. > > Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx> > Signed-off-by: Emil Condrea <emilcondrea@xxxxxxxxx> > --- > hw/xen/Makefile.objs | 2 +- > hw/xen/xen_backend.c | 125 +----------------------------------- > hw/xen/xen_pvdev.c | 149 +++++++++++++++++++++++++++++++++++++++++++ > include/hw/xen/xen_backend.h | 63 +----------------- > include/hw/xen/xen_pvdev.h | 71 +++++++++++++++++++++ > 5 files changed, 223 insertions(+), 187 deletions(-) > create mode 100644 hw/xen/xen_pvdev.c > create mode 100644 include/hw/xen/xen_pvdev.h > > diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs > index d367094..591cdc2 100644 > --- a/hw/xen/Makefile.objs > +++ b/hw/xen/Makefile.objs > @@ -1,5 +1,5 @@ > # xen backend driver support > -common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o > +common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o xen_pvdev.o > > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o > xen_pt_graphics.o xen_pt_msi.o > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c > index bab79b1..a251a4a 100644 > --- a/hw/xen/xen_backend.c > +++ b/hw/xen/xen_backend.c > @@ -30,6 +30,7 @@ > #include "sysemu/char.h" > #include "qemu/log.h" > #include "hw/xen/xen_backend.h" > +#include "hw/xen/xen_pvdev.h" > > #include <xen/grant_table.h> > > @@ -56,8 +57,6 @@ static QTAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup = > static QTAILQ_HEAD(XenDeviceHead, XenDevice) xendevs = > QTAILQ_HEAD_INITIALIZER(xendevs); > static int debug = 0; > > -/* ------------------------------------------------------------- */ > - > static void xenstore_cleanup_dir(char *dir) > { > struct xs_dirs *d; > @@ -76,34 +75,6 @@ void xen_config_cleanup(void) > } > } > > -int xenstore_write_str(const char *base, const char *node, const char *val) > -{ > - char abspath[XEN_BUFSIZE]; > - > - snprintf(abspath, sizeof(abspath), "%s/%s", base, node); > - if (!xs_write(xenstore, 0, abspath, val, strlen(val))) { > - return -1; > - } > - return 0; > -} > - > -char *xenstore_read_str(const char *base, const char *node) > -{ > - char abspath[XEN_BUFSIZE]; > - unsigned int len; > - char *str, *ret = NULL; > - > - snprintf(abspath, sizeof(abspath), "%s/%s", base, node); > - str = xs_read(xenstore, 0, abspath, &len); > - if (str != NULL) { > - /* move to qemu-allocated memory to make sure > - * callers can savely g_free() stuff. */ > - ret = g_strdup(str); > - free(str); > - } > - return ret; > -} > - > int xenstore_mkdir(char *path, int p) > { > struct xs_permissions perms[2] = { > @@ -128,48 +99,6 @@ int xenstore_mkdir(char *path, int p) > return 0; > } > > -int xenstore_write_int(const char *base, const char *node, int ival) > -{ > - char val[12]; > - > > [Quan:]: why 12 ? what about XEN_BUFSIZE? > > - snprintf(val, sizeof(val), "%d", ival); > - return xenstore_write_str(base, node, val); > -} > - > -int xenstore_write_int64(const char *base, const char *node, int64_t ival) > -{ > - char val[21]; > - > > [Quan:]: why 21 ? what about XEN_BUFSIZE? > > > - snprintf(val, sizeof(val), "%"PRId64, ival); > - return xenstore_write_str(base, node, val); > -} > - > -int xenstore_read_int(const char *base, const char *node, int *ival) > -{ > - char *val; > - int rc = -1; > - > - val = xenstore_read_str(base, node); > > [Quan:]: IMO, it is better to initialize val when declares. the same comment for the other 'val' > > - if (val && 1 == sscanf(val, "%d", ival)) { > - rc = 0; > - } > - g_free(val); > - return rc; > -} > - > -int xenstore_read_uint64(const char *base, const char *node, uint64_t *uval) > -{ > - char *val; > - int rc = -1; > - > - val = xenstore_read_str(base, node); > - if (val && 1 == sscanf(val, "%"SCNu64, uval)) { > - rc = 0; > - } > - g_free(val); > - return rc; > -} > - > int xenstore_write_be_str(struct XenDevice *xendev, const char *node, const > char *val) > { > return xenstore_write_str(xendev->be, node, val); > @@ -212,20 +141,6 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, > const char *node, uint64_t > > /* ------------------------------------------------------------- */ > > -const char *xenbus_strstate(enum xenbus_state state) > -{ > - static const char *const name[] = { > - [ XenbusStateUnknown ] = "Unknown", > - [ XenbusStateInitialising ] = "Initialising", > - [ XenbusStateInitWait ] = "InitWait", > - [ XenbusStateInitialised ] = "Initialised", > - [ XenbusStateConnected ] = "Connected", > - [ XenbusStateClosing ] = "Closing", > - [ XenbusStateClosed ] = "Closed", > - }; > - return (state < ARRAY_SIZE(name)) ? name[state] : "INVALID"; > -} > - > int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state) > { > int rc; > @@ -833,44 +748,6 @@ int xen_be_send_notify(struct XenDevice *xendev) > return xenevtchn_notify(xendev->evtchndev, xendev->local_port); > } > > -/* > - * msg_level: > - * 0 == errors (stderr + logfile). > - * 1 == informative debug messages (logfile only). > - * 2 == noisy debug messages (logfile only). > - * 3 == will flood your log (logfile only). > - */ > -void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, > ...) > -{ > - va_list args; > - > - if (xendev) { > - if (msg_level > xendev->debug) { > - return; > - } > - qemu_log("xen be: %s: ", xendev->name); > - if (msg_level == 0) { > - fprintf(stderr, "xen be: %s: ", xendev->name); > - } > - } else { > - if (msg_level > debug) { > - return; > - } > - qemu_log("xen be core: "); > - if (msg_level == 0) { > - fprintf(stderr, "xen be core: "); > - } > - } > - va_start(args, fmt); > - qemu_log_vprintf(fmt, args); > - va_end(args); > - if (msg_level == 0) { > - va_start(args, fmt); > - vfprintf(stderr, fmt, args); > - va_end(args); > - } > - qemu_log_flush(); > -} > > static int xen_sysdev_init(SysBusDevice *dev) > { > diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c > new file mode 100644 > index 0000000..a444855 > --- /dev/null > +++ b/hw/xen/xen_pvdev.c > @@ -0,0 +1,149 @@ > +/* > + * Xen para-virtualization device > + * > + * (c) 2008 Gerd Hoffmann <kraxel@xxxxxxxxxx> > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see < http://www.gnu.org/licenses/> > + */ > + > +#include "qemu/osdep.h" > + > +#include "hw/xen/xen_backend.h" > +#include "hw/xen/xen_pvdev.h" > + > +static int debug = 0; > +/* ------------------------------------------------------------- */ > + > +int xenstore_write_str(const char *base, const char *node, const char *val) > +{ > + char abspath[XEN_BUFSIZE]; > + > + snprintf(abspath, sizeof(abspath), "%s/%s", base, node); > + if (!xs_write(xenstore, 0, abspath, val, strlen(val))) { > + return -1; > + } > + return 0; > +} > + > +char *xenstore_read_str(const char *base, const char *node) > +{ > + char abspath[XEN_BUFSIZE]; > + unsigned int len; > + char *str, *ret = NULL; > + > + snprintf(abspath, sizeof(abspath), "%s/%s", base, node); > + str = xs_read(xenstore, 0, abspath, &len); > + if (str != NULL) { > + /* move to qemu-allocated memory to make sure > + * callers can savely g_free() stuff. */ > + ret = g_strdup(str); > + free(str); > + } > + return ret; > +} > + > +int xenstore_write_int(const char *base, const char *node, int ival) > +{ > + char val[12]; > + > + snprintf(val, sizeof(val), "%d", ival); > + return xenstore_write_str(base, node, val); > +} > + > +int xenstore_write_int64(const char *base, const char *node, int64_t ival) > +{ > + char val[21]; > + > + snprintf(val, sizeof(val), "%"PRId64, ival); > + return xenstore_write_str(base, node, val); > +} > + > +int xenstore_read_int(const char *base, const char *node, int *ival) > +{ > + char *val; > + int rc = -1; > + > + val = xenstore_read_str(base, node); > + if (val && 1 == sscanf(val, "%d", ival)) { > + rc = 0; > + } > + g_free(val); > + return rc; > +} > + > +int xenstore_read_uint64(const char *base, const char *node, uint64_t *uval) > +{ > + char *val; > + int rc = -1; > + > + val = xenstore_read_str(base, node); > + if (val && 1 == sscanf(val, "%"SCNu64, uval)) { > + rc = 0; > + } > + g_free(val); > + return rc; > +} > + > +const char *xenbus_strstate(enum xenbus_state state) > +{ > + static const char *const name[] = { > + [ XenbusStateUnknown ] = "Unknown", > + [ XenbusStateInitialising ] = "Initialising", > + [ XenbusStateInitWait ] = "InitWait", > + [ XenbusStateInitialised ] = "Initialised", > + [ XenbusStateConnected ] = "Connected", > + [ XenbusStateClosing ] = "Closing", > + [ XenbusStateClosed ] = "Closed", > + }; > + return (state < ARRAY_SIZE(name)) ? name[state] : "INVALID"; > +} > + > +/* > + * msg_level: > + * 0 == errors (stderr + logfile). > + * 1 == informative debug messages (logfile only). > + * 2 == noisy debug messages (logfile only). > + * 3 == will flood your log (logfile only). > + */ > +void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, > ...) > +{ > + va_list args; > + > + if (xendev) { > + if (msg_level > xendev->debug) { > + return; > + } > + qemu_log("xen be: %s: ", xendev->name); > + if (msg_level == 0) { > + fprintf(stderr, "xen be: %s: ", xendev->name); > + } > + } else { > + if (msg_level > debug) { > + return; > + } > + qemu_log("xen be core: "); > + if (msg_level == 0) { > + fprintf(stderr, "xen be core: "); > + } > + } > + va_start(args, fmt); > + qemu_log_vprintf(fmt, args); > + va_end(args); > + if (msg_level == 0) { > + va_start(args, fmt); > + vfprintf(stderr, fmt, args); > + va_end(args); > + } > + qemu_log_flush(); > +} > diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h > index 6e18a46..0f009e3 100644 > --- a/include/hw/xen/xen_backend.h > +++ b/include/hw/xen/xen_backend.h > @@ -2,60 +2,10 @@ > #define QEMU_HW_XEN_BACKEND_H 1 > > #include "hw/xen/xen_common.h" > +#include "hw/xen/xen_pvdev.h" > #include "sysemu/sysemu.h" > #include "net/net.h" > > -/* ------------------------------------------------------------- */ > - > -#define XEN_BUFSIZE 1024 > - > -struct XenDevice; > - > -/* driver uses grant tables -> open gntdev device (xendev->gnttabdev) */ > -#define DEVOPS_FLAG_NEED_GNTDEV 1 > -/* don't expect frontend doing correct state transitions (aka console quirk) */ > -#define DEVOPS_FLAG_IGNORE_STATE 2 > - > -struct XenDevOps { > - size_t size; > - uint32_t flags; > - void (*alloc)(struct XenDevice *xendev); > - int (*init)(struct XenDevice *xendev); > - int (*initialise)(struct XenDevice *xendev); > - void (*connected)(struct XenDevice *xendev); > - void (*event)(struct XenDevice *xendev); > - void (*disconnect)(struct XenDevice *xendev); > - int (*free)(struct XenDevice *xendev); > - void (*backend_changed)(struct XenDevice *xendev, const char *node); > - void (*frontend_changed)(struct XenDevice *xendev, const char *node); > - int (*backend_register)(void); > -}; > - > -struct XenDevice { > - const char *type; > - int dom; > - int dev; > - char name[64]; > - int debug; > - > - enum xenbus_state be_state; > - enum xenbus_state fe_state; > - int online; > - char be[XEN_BUFSIZE]; > - char *fe; > - char *protocol; > - int remote_port; > - int local_port; > - > - xenevtchn_handle *evtchndev; > - xengnttab_handle *gnttabdev; > - > - struct XenDevOps *ops; > - QTAILQ_ENTRY(XenDevice) next; > -}; > - > -/* ------------------------------------------------------------- */ > - > /* variables */ > extern xc_interface *xen_xc; > extern xenforeignmemory_handle *xen_fmem; > @@ -63,14 +13,7 @@ extern struct xs_handle *xenstore; > extern const char *xen_protocol; > extern DeviceState *xen_sysdev; > > -/* xenstore helper functions */ > int xenstore_mkdir(char *path, int p); > -int xenstore_write_str(const char *base, const char *node, const char *val); > -int xenstore_write_int(const char *base, const char *node, int ival); > -int xenstore_write_int64(const char *base, const char *node, int64_t ival); > -char *xenstore_read_str(const char *base, const char *node); > -int xenstore_read_int(const char *base, const char *node, int *ival); > - > int xenstore_write_be_str(struct XenDevice *xendev, const char *node, const > char *val); > int xenstore_write_be_int(struct XenDevice *xendev, const char *node, int > ival); > int xenstore_write_be_int64(struct XenDevice *xendev, const char *node, > int64_t ival); > @@ -78,10 +21,8 @@ char *xenstore_read_be_str(struct XenDevice *xendev, const > char *node); > int xenstore_read_be_int(struct XenDevice *xendev, const char *node, int > *ival); > char *xenstore_read_fe_str(struct XenDevice *xendev, const char *node); > int xenstore_read_fe_int(struct XenDevice *xendev, const char *node, int > *ival); > -int xenstore_read_uint64(const char *base, const char *node, uint64_t *uval); > int xenstore_read_fe_uint64(struct XenDevice *xendev, const char *node, > uint64_t *uval); > > -const char *xenbus_strstate(enum xenbus_state state); > struct XenDevice *xen_be_find_xendev(const char *type, int dom, int dev); > void xen_be_check_state(struct XenDevice *xendev); > > @@ -92,8 +33,6 @@ 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); > int xen_be_send_notify(struct XenDevice *xendev); > -void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, > ...) > - GCC_FMT_ATTR(3, 4); > > /* actual backend drivers */ > extern struct XenDevOps xen_console_ops; /* xen_console.c */ > diff --git a/include/hw/xen/xen_pvdev.h b/include/hw/xen/xen_pvdev.h > new file mode 100644 > index 0000000..f60bfae > --- /dev/null > +++ b/include/hw/xen/xen_pvdev.h > @@ -0,0 +1,71 @@ > +#ifndef QEMU_HW_XEN_PVDEV_H > +#define QEMU_HW_XEN_PVDEV_H 1 > + > +#include "hw/xen/xen_common.h" > +#include <inttypes.h> > + > +/* ------------------------------------------------------------- */ > + > +#define XEN_BUFSIZE 1024 > + > +struct XenDevice; > + > +/* driver uses grant tables -> open gntdev device (xendev->gnttabdev) */ > +#define DEVOPS_FLAG_NEED_GNTDEV 1 > +/* don't expect frontend doing correct state transitions (aka console quirk) */ > +#define DEVOPS_FLAG_IGNORE_STATE 2 > + > +struct XenDevOps { > + size_t size; > + uint32_t flags; > + void (*alloc)(struct XenDevice *xendev); > + int (*init)(struct XenDevice *xendev); > + int (*initialise)(struct XenDevice *xendev); > + void (*connected)(struct XenDevice *xendev); > + void (*event)(struct XenDevice *xendev); > + void (*disconnect)(struct XenDevice *xendev); > + int (*free)(struct XenDevice *xendev); > + void (*backend_changed)(struct XenDevice *xendev, const char *node); > + void (*frontend_changed)(struct XenDevice *xendev, const char *node); > + int (*backend_register)(void); > +}; > + > +struct XenDevice { > + const char *type; > + int dom; > + int dev; > + char name[64]; > + int debug; > + > + enum xenbus_state be_state; > + enum xenbus_state fe_state; > + int online; > + char be[XEN_BUFSIZE]; > + char *fe; > + char *protocol; > + int remote_port; > + int local_port; > + > + xenevtchn_handle *evtchndev; > + xengnttab_handle *gnttabdev; > + > + struct XenDevOps *ops; > + QTAILQ_ENTRY(XenDevice) next; > +}; > + > +/* ------------------------------------------------------------- */ > + > +/* xenstore helper functions */ > +int xenstore_write_str(const char *base, const char *node, const char *val); > +int xenstore_write_int(const char *base, const char *node, int ival); > +int xenstore_write_int64(const char *base, const char *node, int64_t ival); > +char *xenstore_read_str(const char *base, const char *node); > +int xenstore_read_int(const char *base, const char *node, int *ival); > +int xenstore_read_uint64(const char *base, const char *node, uint64_t *uval); > + > +const char *xenbus_strstate(enum xenbus_state state); > + > +void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, > ...) > + GCC_FMT_ATTR(3, 4); > + > +#endif /* QEMU_HW_XEN_PVDEV_H */ > -- > 1.9.1
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel