[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Xen-devel] [PATCH 01/19] xen: Create a new file xen_pv
From: |
Anthony PERARD |
Subject: |
Re: [Qemu-devel] [Xen-devel] [PATCH 01/19] xen: Create a new file xen_pvdev.c |
Date: |
Mon, 18 Jul 2016 15:50:27 +0100 |
User-agent: |
Mutt/1.6.2 (2016-07-01) |
On Sun, Jul 17, 2016 at 03:41:26PM +0800, Quan Xu wrote:
>
> [Quan:]: comment starts with [Quan:]
>
>
> The purpose of the new file is to store generic functions shared by
> frontendand backends such as xenstore operations, xendevs.
>
> Signed-off-by: Quan Xu <address@hidden>
> Signed-off-by: Emil Condrea <address@hidden>
> ---
> 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?
That is the number of digit in INT_MAX (10) + 1 for the sign + 1 for '\0'.
> - 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?
Same with INT64_MAX (19 digits).
>
> - 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.
I think I prefer it this way.
> - if (val && 1 == sscanf(val, "%d", ival)) {
> - rc = 0;
> - }
> - g_free(val);
> - return rc;
> -}
--
Anthony PERARD