[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 01/19] xen: Create a new file xen_pvdev.c
On Jul 17, 2016 10:41, "Quan Xu" <quan.xu2@xxxxxxxxxx> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
