* [PATCH 0/2] libxl: add PV display device driver interface
@ 2017-03-23 10:10 Oleksandr Grytsov
2017-03-23 10:10 ` [PATCH 1/2] " Oleksandr Grytsov
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Oleksandr Grytsov @ 2017-03-23 10:10 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Hi all,
We are working on series of PV drivers (display, sound, input etc.) and
would like to add their support to libxl and xl. These patches add PV display
device. PV display is based on [1] protocol.
During implementation I see a lot of code duplication. This problem was
mentioned in [2]. One of the solutions was to implement common parts in IDL
and make them autogenerated. On my side, to minimize the copy/pasting
I've moved common parts into macro functions: LIBXL_DEFINE_DEVICE_COMMIT,
LIBXL_DEFINE_DEVICE_LIST_GET, LIBXL_DEFINE_DEVICE_GETINFO etc.
Existing PV devices implementations can be reworked to use these macros as
well. Any other proposals to avoid the duplications are welcome.
Thanks.
[1] http://marc.info/?l=xen-devel&m=149000029128972&w=2
[2] http://marc.info/?l=xen-devel&m=145372933919792&w=2
Oleksandr Grytsov (2):
libxl: add PV display device driver interface
xl: add PV display device commands
tools/libxl/Makefile | 2 +-
tools/libxl/libxl.h | 21 ++++
tools/libxl/libxl_create.c | 1 +
tools/libxl/libxl_internal.h | 228 +++++++++++++++++++++++++++++++++++
tools/libxl/libxl_types.idl | 22 +++-
tools/libxl/libxl_types_internal.idl | 1 +
tools/libxl/libxl_utils.h | 4 +
tools/libxl/libxl_vdispl.c | 137 +++++++++++++++++++++
tools/xl/Makefile | 1 +
tools/xl/xl.h | 3 +
tools/xl/xl_cmdtable.c | 16 +++
tools/xl/xl_parse.c | 44 ++++++-
tools/xl/xl_parse.h | 2 +-
tools/xl/xl_vdispl.c | 162 +++++++++++++++++++++++++
14 files changed, 639 insertions(+), 5 deletions(-)
create mode 100644 tools/libxl/libxl_vdispl.c
create mode 100644 tools/xl/xl_vdispl.c
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] libxl: add PV display device driver interface
2017-03-23 10:10 [PATCH 0/2] libxl: add PV display device driver interface Oleksandr Grytsov
@ 2017-03-23 10:10 ` Oleksandr Grytsov
2017-03-23 10:10 ` [PATCH 2/2] xl: add PV display device commands Oleksandr Grytsov
2017-03-23 10:21 ` [PATCH 0/2] libxl: add PV display device driver interface Juergen Gross
2 siblings, 0 replies; 11+ messages in thread
From: Oleksandr Grytsov @ 2017-03-23 10:10 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/Makefile | 2 +-
tools/libxl/libxl.h | 21 ++++
tools/libxl/libxl_create.c | 1 +
tools/libxl/libxl_internal.h | 228 +++++++++++++++++++++++++++++++++++
tools/libxl/libxl_types.idl | 22 +++-
tools/libxl/libxl_types_internal.idl | 1 +
tools/libxl/libxl_utils.h | 4 +
tools/libxl/libxl_vdispl.c | 137 +++++++++++++++++++++
8 files changed, 413 insertions(+), 3 deletions(-)
create mode 100644 tools/libxl/libxl_vdispl.c
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index f00d9ef..cb7c17f 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -138,7 +138,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
libxl_dom_suspend.o libxl_dom_save.o libxl_usb.o \
libxl_vtpm.o libxl_nic.o libxl_disk.o libxl_console.o \
libxl_cpupool.o libxl_mem.o libxl_sched.o libxl_tmem.o \
- libxl_domain.o \
+ libxl_domain.o libxl_vdispl.o \
$(LIBXL_OBJS-y)
LIBXL_OBJS += libxl_genid.o
LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 92f1751..a4fc304 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1833,6 +1833,27 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *n
int libxl_device_vtpm_getinfo(libxl_ctx *ctx, uint32_t domid,
libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo);
+/* Virtual displays */
+int libxl_device_vdispl_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_vdispl *displ,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_vdispl_remove(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_vdispl *vdispl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_vdispl_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_vdispl *vdispl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid,
+ int *num);
+void libxl_device_vdispl_list_free(libxl_device_vdispl* list, int num);
+int libxl_device_vdispl_getinfo(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_vdispl *vdispl,
+ libxl_vdisplinfo *vdisplinfo);
+
/* Keyboard */
int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb,
const libxl_asyncop_how *ao_how)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index e741b9a..19e0683 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1445,6 +1445,7 @@ const struct libxl_device_type *device_type_tbl[] = {
&libxl__usbdev_devtype,
&libxl__pcidev_devtype,
&libxl__dtdev_devtype,
+ &libxl__vdispl_devtype,
NULL
};
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5bbede5..787bebb 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3465,6 +3465,233 @@ _hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, type, remove, 0) \
LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, type, destroy, 1)
+#define LIBXL_DEFINE_DEVICE_COMMIT(type) \
+ static int libxl__device_##type##_commit(libxl__egc *egc, \
+ uint32_t domid, libxl_device_##type *type, \
+ flexarray_t *front, flexarray_t *back, \
+ libxl__ao_device *aodev, int rc) \
+ { \
+ STATE_AO_GC(aodev->ao); \
+ libxl__device *device; \
+ xs_transaction_t t = XBT_NULL; \
+ libxl_domain_config d_config; \
+ libxl__domain_userdata_lock *lock = NULL; \
+ \
+ libxl_domain_config_init(&d_config); \
+ \
+ if (rc) goto out; \
+ \
+ if (aodev->update_json) { \
+ lock = libxl__lock_domain_userdata(gc, domid); \
+ if (!lock) { rc = ERROR_LOCK_FAIL; goto out; } \
+ \
+ rc = libxl__get_domain_configuration(gc, domid, &d_config); \
+ if (rc) goto out; \
+ \
+ DEVICE_ADD(type, type##s, domid, type, \
+ COMPARE_DEVID, &d_config); \
+ \
+ rc = libxl__dm_check_start(gc, &d_config, domid); \
+ if (rc) goto out; \
+ } \
+ \
+ GCNEW(device); \
+ rc = libxl__device_from_##type(gc, domid, type, device); \
+ if ( rc != 0 ) goto out; \
+ \
+ for (;;) { \
+ rc = libxl__xs_transaction_start(gc, &t); \
+ if (rc) goto out; \
+ \
+ rc = libxl__device_exists(gc, t, device); \
+ if (rc < 0) goto out; \
+ if (rc == 1) { \
+ LOGD(ERROR, domid, "device already exists in xenstore");\
+ aodev->action = LIBXL__DEVICE_ACTION_ADD; \
+ rc = ERROR_DEVICE_EXISTS; \
+ goto out; \
+ } \
+ \
+ if (aodev->update_json) { \
+ rc = libxl__set_domain_configuration(gc, domid, \
+ &d_config); \
+ if (rc) goto out; \
+ } \
+ \
+ libxl__device_generic_add(gc, t, device, \
+ libxl__xs_kvs_of_flexarray(gc, back), \
+ libxl__xs_kvs_of_flexarray(gc, front), \
+ NULL); \
+ \
+ rc = libxl__xs_transaction_commit(gc, &t); \
+ if (rc < 0) { rc = ERROR_FAIL; goto out; } \
+ if (!rc) break; \
+ } \
+ \
+ aodev->dev = device; \
+ aodev->action = LIBXL__DEVICE_ACTION_ADD; \
+ libxl__wait_device_connection(egc, aodev); \
+ rc = 0; \
+ \
+ out: \
+ libxl__xs_transaction_abort(gc, &t); \
+ if (lock) libxl__unlock_domain_userdata(lock); \
+ libxl_domain_config_dispose(&d_config); \
+ aodev->rc = rc; \
+ if(rc) aodev->callback(egc, aodev); \
+ return rc; \
+ }
+
+#define LIBXL_DEFINE_DEVICE_LIST_GET(type) \
+ libxl_device_##type *libxl_device_##type##_list(libxl_ctx *ctx, \
+ uint32_t domid, \
+ int *num) \
+ { \
+ GC_INIT(ctx); \
+ \
+ libxl_device_##type* types = NULL; \
+ libxl_device_##type* r = NULL; \
+ libxl_device_##type* type; \
+ char *libxl_path; \
+ char** dir = NULL; \
+ unsigned int ndirs = 0; \
+ int rc; \
+ \
+ *num = 0; \
+ \
+ libxl_path = GCSPRINTF("%s/device/%s", \
+ libxl__xs_libxl_path(gc, domid), #type); \
+ \
+ dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs); \
+ \
+ if (dir && ndirs) { \
+ types = malloc(sizeof(*types) * ndirs); \
+ libxl_device_##type* end = types + ndirs; \
+ \
+ for(type = types; type < end; ++type, ++dir) { \
+ const char* be_path = libxl__xs_read(gc, XBT_NULL, \
+ GCSPRINTF("%s/%s/backend", libxl_path, *dir)); \
+ \
+ libxl_device_##type##_init(type); \
+ \
+ type->devid = atoi(*dir); \
+ \
+ rc = libxl__backendpath_parse_domid(gc, be_path, \
+ &type->backend_domid); \
+ if (rc) goto out; \
+ } \
+ } \
+ \
+ *num = ndirs; \
+ r = types; \
+ types = NULL; \
+ \
+ out: \
+ if (types) { \
+ for(; type >= types; --type) { \
+ libxl_device_##type##_dispose(type); \
+ } \
+ free(types); \
+ } \
+ \
+ GC_FREE; \
+ \
+ return r; \
+ }
+
+#define LIBXL_DEFINE_DEVICE_LIST_FREE(type) \
+ void libxl_device_##type##_list_free(libxl_device_##type* list, \
+ int nr) \
+ { \
+ int i; \
+ for (i = 0; i < nr; i++) { \
+ libxl_device_##type##_dispose(&list[i]); \
+ } \
+ free(list); \
+ }
+
+#define LIBXL_DEFINE_DEVID_TO_DEVICE(type) \
+ int libxl_devid_to_device_##type(libxl_ctx *ctx, \
+ uint32_t domid, \
+ int devid, \
+ libxl_device_##type *type) \
+ { \
+ libxl_device_##type *types = NULL; \
+ int n, i; \
+ int rc; \
+ \
+ libxl_device_##type##_init(type); \
+ \
+ types = libxl_device_##type##_list(ctx, domid, &n); \
+ if (!types) { rc = ERROR_NOTFOUND; goto out; } \
+ \
+ for (i = 0; i < n; ++i) { \
+ if (devid == types[i].devid) { \
+ type->backend_domid = types[i].backend_domid; \
+ type->devid = types[i].devid; \
+ rc = 0; \
+ goto out; \
+ } \
+ } \
+ \
+ rc = ERROR_NOTFOUND; \
+ \
+ out: \
+ if (types) { \
+ libxl_device_##type##_list_free(types, n); \
+ } \
+ return rc; \
+ }
+
+#define LIBXL_DEFINE_DEVICE_GETINFO(type) \
+ int libxl_device_##type##_getinfo(libxl_ctx *ctx, \
+ uint32_t domid, \
+ libxl_device_##type *type, \
+ libxl_##type##info *info) \
+ { \
+ GC_INIT(ctx); \
+ char *libxl_path, *dompath, *devpath; \
+ char *val; \
+ int rc; \
+ \
+ libxl_##type##info_init(info); \
+ dompath = libxl__xs_get_dompath(gc, domid); \
+ info->devid = type->devid; \
+ \
+ devpath = GCSPRINTF("%s/device/"#type"/%d", \
+ dompath, info->devid); \
+ libxl_path = GCSPRINTF("%s/device/"#type"/%d", \
+ libxl__xs_libxl_path(gc, domid), \
+ info->devid); \
+ info->backend = xs_read(ctx->xsh, XBT_NULL, \
+ GCSPRINTF("%s/backend", libxl_path), \
+ NULL); \
+ if (!info->backend) { rc = ERROR_FAIL; goto out; } \
+ \
+ rc = libxl__backendpath_parse_domid(gc, info->backend, \
+ &info->backend_id); \
+ if (rc) { goto out; } \
+ \
+ val = libxl__xs_read(gc, XBT_NULL, \
+ GCSPRINTF("%s/state", devpath)); \
+ info->state = val ? strtoul(val, NULL, 10) : -1; \
+ \
+ info->frontend = xs_read(ctx->xsh, XBT_NULL, \
+ GCSPRINTF("%s/frontend", libxl_path), \
+ NULL); \
+ info->frontend_id = domid; \
+ \
+ rc = libxl__device_##type##_getinfo(gc, ctx->xsh, \
+ libxl_path, info); \
+ if (rc) { goto out; } \
+ \
+ rc = 0; \
+ \
+ out: \
+ GC_FREE; \
+ return rc; \
+ }
+
struct libxl_device_type {
char *type;
int skip_attach; /* Skip entry in domcreate_attach_devices() if 1 */
@@ -3524,6 +3751,7 @@ extern const struct libxl_device_type libxl__vtpm_devtype;
extern const struct libxl_device_type libxl__usbctrl_devtype;
extern const struct libxl_device_type libxl__usbdev_devtype;
extern const struct libxl_device_type libxl__pcidev_devtype;
+extern const struct libxl_device_type libxl__vdispl_devtype;
extern const struct libxl_device_type *device_type_tbl[];
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index a612d1f..46845f0 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -690,7 +690,14 @@ libxl_device_vtpm = Struct("device_vtpm", [
("backend_domname", string),
("devid", libxl_devid),
("uuid", libxl_uuid),
-])
+ ])
+
+libxl_device_vdispl = Struct("device_vdispl", [
+ ("backend_domid", libxl_domid),
+ ("backend_domname", string),
+ ("devid", libxl_devid),
+ ("be_alloc", bool),
+ ])
libxl_device_channel = Struct("device_channel", [
("backend_domid", libxl_domid),
@@ -702,7 +709,7 @@ libxl_device_channel = Struct("device_channel", [
("pty", None),
("socket", Struct(None, [("path", string)])),
])),
-])
+ ])
libxl_domain_config = Struct("domain_config", [
("c_info", libxl_domain_create_info),
@@ -716,6 +723,7 @@ libxl_domain_config = Struct("domain_config", [
("vfbs", Array(libxl_device_vfb, "num_vfbs")),
("vkbs", Array(libxl_device_vkb, "num_vkbs")),
("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
+ ("vdispls", Array(libxl_device_vdispl, "num_vdispls")),
# a channel manifests as a console with a name,
# see docs/misc/channels.txt
("channels", Array(libxl_device_channel, "num_channels")),
@@ -812,6 +820,16 @@ libxl_physinfo = Struct("physinfo", [
("cap_hvm_directio", bool),
], dir=DIR_OUT)
+libxl_vdisplinfo = Struct("vdisplinfo", [
+ ("backend", string),
+ ("backend_id", uint32),
+ ("frontend", string),
+ ("frontend_id", uint32),
+ ("devid", libxl_devid),
+ ("state", integer),
+ ("be_alloc", bool),
+ ], dir=DIR_OUT)
+
# NUMA node characteristics: size and free are how much memory it has, and how
# much of it is free, respectively. dists is an array of distances from this
# node to each other node.
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index 82e5c07..cdef4d3 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -25,6 +25,7 @@ libxl__device_kind = Enumeration("device_kind", [
(8, "VTPM"),
(9, "VUSB"),
(10, "QUSB"),
+ (11, "VDISPL")
])
libxl__console_backend = Enumeration("console_backend", [
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 25773d8..9e743dc 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -78,6 +78,10 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
int devid, libxl_device_vtpm *vtpm);
int libxl_devid_to_device_usbctrl(libxl_ctx *ctx, uint32_t domid,
int devid, libxl_device_usbctrl *usbctrl);
+
+int libxl_devid_to_device_vdispl(libxl_ctx *ctx, uint32_t domid,
+ int devid, libxl_device_vdispl *vdispl);
+
int libxl_ctrlport_to_device_usbdev(libxl_ctx *ctx, uint32_t domid,
int ctrl, int port,
libxl_device_usbdev *usbdev);
diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
new file mode 100644
index 0000000..869aaa1
--- /dev/null
+++ b/tools/libxl/libxl_vdispl.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2016 EPAM Systems Inc.
+ *
+ * This program 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; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program 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.
+ */
+
+#include "libxl_osdeps.h"
+
+#include "libxl_internal.h"
+
+static int libxl__device_vdispl_setdefault(libxl__gc *gc, domid_t domid,
+ libxl_device_vdispl *vdispl)
+{
+ int rc;
+
+ rc = libxl__resolve_domid(gc, vdispl->backend_domname,
+ &vdispl->backend_domid);
+
+ if (vdispl->devid == -1) {
+ vdispl->devid = libxl__device_nextid(gc, domid,
+ (char*)libxl__device_kind_to_string(LIBXL__DEVICE_KIND_VDISPL));
+ }
+
+ return rc;
+}
+
+static int libxl__device_from_vdispl(libxl__gc *gc, uint32_t domid,
+ libxl_device_vdispl *vdispl,
+ libxl__device *device)
+{
+ device->backend_devid = vdispl->devid;
+ device->backend_domid = vdispl->backend_domid;
+ device->backend_kind = LIBXL__DEVICE_KIND_VDISPL;
+ device->devid = vdispl->devid;
+ device->domid = domid;
+ device->kind = LIBXL__DEVICE_KIND_VDISPL;
+
+ return 0;
+}
+
+LIBXL_DEFINE_DEVICE_COMMIT(vdispl)
+
+static void libxl__device_vdispl_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_vdispl *vdispl,
+ libxl__ao_device *aodev)
+{
+ STATE_AO_GC(aodev->ao);
+ flexarray_t *front;
+ flexarray_t *back;
+ int rc;
+
+ rc = libxl__device_vdispl_setdefault(gc, domid, vdispl);
+ if (rc) goto out;
+
+ front = flexarray_make(gc, 16, 1);
+ back = flexarray_make(gc, 16, 1);
+
+ flexarray_append(back, "frontend-id");
+ flexarray_append(back, GCSPRINTF("%d", domid));
+ flexarray_append(back, "online");
+ flexarray_append(back, "1");
+ flexarray_append(back, "state");
+ flexarray_append(back, GCSPRINTF("%d", XenbusStateInitialising));
+ flexarray_append(back, "handle");
+ flexarray_append(back, GCSPRINTF("%d", vdispl->devid));
+
+ flexarray_append(front, "backend-id");
+ flexarray_append(front, GCSPRINTF("%d", vdispl->backend_domid));
+ flexarray_append(front, "state");
+ flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising));
+ flexarray_append(front, "handle");
+ flexarray_append(front, GCSPRINTF("%d", vdispl->devid));
+ flexarray_append(front, "be_alloc");
+ flexarray_append(front, GCSPRINTF("%d", vdispl->be_alloc));
+
+out:
+ libxl__device_vdispl_commit(egc, domid, vdispl, front, back, aodev, rc);
+}
+
+static int libxl__device_vdispl_getinfo(libxl__gc *gc, struct xs_handle *xsh,
+ const char* libxl_path,
+ libxl_vdisplinfo *info)
+{
+ char *val;
+
+ val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/be_alloc", libxl_path));
+ info->be_alloc = val ? strtoul(val, NULL, 10) : 0;
+
+ return 0;
+}
+
+static void libxl__update_config_vdispl(libxl__gc *gc, void *dst, void *src)
+{
+ libxl_device_vdispl *d, *s;
+
+ d = (libxl_device_vdispl*)dst;
+ s = (libxl_device_vdispl*)src;
+
+ d->devid = s->devid;
+ d->be_alloc = s->be_alloc;
+}
+
+static int libxl_device_vdispl_compare(libxl_device_vdispl *d1,
+ libxl_device_vdispl *d2)
+{
+ return COMPARE_DEVID(d1, d2);
+}
+
+LIBXL_DEFINE_DEVICE_ADD(vdispl)
+static LIBXL_DEFINE_DEVICES_ADD(vdispl)
+LIBXL_DEFINE_DEVICE_REMOVE(vdispl)
+
+LIBXL_DEFINE_DEVICE_LIST_GET(vdispl)
+LIBXL_DEFINE_DEVICE_LIST_FREE(vdispl)
+
+LIBXL_DEFINE_DEVID_TO_DEVICE(vdispl)
+LIBXL_DEFINE_DEVICE_GETINFO(vdispl)
+
+DEFINE_DEVICE_TYPE_STRUCT(vdispl,
+ .update_config = libxl__update_config_vdispl
+);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] xl: add PV display device commands
2017-03-23 10:10 [PATCH 0/2] libxl: add PV display device driver interface Oleksandr Grytsov
2017-03-23 10:10 ` [PATCH 1/2] " Oleksandr Grytsov
@ 2017-03-23 10:10 ` Oleksandr Grytsov
2017-03-23 10:21 ` [PATCH 0/2] libxl: add PV display device driver interface Juergen Gross
2 siblings, 0 replies; 11+ messages in thread
From: Oleksandr Grytsov @ 2017-03-23 10:10 UTC (permalink / raw)
To: xen-devel; +Cc: Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/xl/Makefile | 1 +
tools/xl/xl.h | 3 +
tools/xl/xl_cmdtable.c | 16 +++++
tools/xl/xl_parse.c | 44 +++++++++++++-
tools/xl/xl_parse.h | 2 +-
tools/xl/xl_vdispl.c | 162 +++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 226 insertions(+), 2 deletions(-)
create mode 100644 tools/xl/xl_vdispl.c
diff --git a/tools/xl/Makefile b/tools/xl/Makefile
index e16f877..5c784b5 100644
--- a/tools/xl/Makefile
+++ b/tools/xl/Makefile
@@ -21,6 +21,7 @@ XL_OBJS += xl_vtpm.o xl_block.o xl_nic.o xl_usb.o
XL_OBJS += xl_sched.o xl_pci.o xl_vcpu.o xl_cdrom.o xl_mem.o
XL_OBJS += xl_psr.o xl_info.o xl_console.o xl_misc.o
XL_OBJS += xl_vmcontrol.o xl_saverestore.o xl_migrate.o
+XL_OBJS += xl_vdispl.o
$(XL_OBJS): CFLAGS += $(CFLAGS_libxentoollog)
$(XL_OBJS): CFLAGS += $(CFLAGS_XL)
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index 1ad0726..370a0d4 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -165,6 +165,9 @@ int main_blockdetach(int argc, char **argv);
int main_vtpmattach(int argc, char **argv);
int main_vtpmlist(int argc, char **argv);
int main_vtpmdetach(int argc, char **argv);
+int main_vdisplattach(int argc, char **argv);
+int main_vdispllist(int argc, char **argv);
+int main_vdispldetach(int argc, char **argv);
int main_usbctrl_attach(int argc, char **argv);
int main_usbctrl_detach(int argc, char **argv);
int main_usbdev_attach(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 1219b33..1050dc1 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -372,6 +372,22 @@ struct cmd_spec cmd_table[] = {
"Destroy a domain's virtual TPM device",
"<Domain> <DevId|uuid>",
},
+ { "vdispl-attach",
+ &main_vdisplattach, 1, 1,
+ "Create a new virtual display device",
+ "<Domain> [devId=<Device>] [backend=<BackDomain>] [beAlloc=<BackAlloc>]",
+ " BackAlloc - set to 1 to allow backend allocated display buffers"
+ },
+ { "vdispl-list",
+ &main_vdispllist, 0, 0,
+ "List virtual display devices for a domain",
+ "<Domain(s)>",
+ },
+ { "vdispl-detach",
+ &main_vdispldetach, 0, 1,
+ "Destroy a domain's virtual display device",
+ "<Domain> <DevId>",
+ },
{ "uptime",
&main_uptime, 0, 0,
"Print uptime for all/some domains",
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 1ef0c27..6d378ed 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -707,6 +707,25 @@ int parse_usbdev_config(libxl_device_usbdev *usbdev, char *token)
return 0;
}
+int parse_vdispl_config(libxl_device_vdispl *vdispl, char *token)
+{
+ char *oparg;
+
+ if (MATCH_OPTION("backend", token, oparg)) {
+ vdispl->backend_domid = find_domain(oparg);
+ vdispl->backend_domname = strdup(oparg);
+ } else if (MATCH_OPTION("devId", token, oparg)) {
+ vdispl->devid = atoi(oparg);
+ } else if (MATCH_OPTION("beAlloc", token, oparg)) {
+ vdispl->be_alloc = strtoul(oparg, NULL, 0);
+ } else {
+ fprintf(stderr, "Unknown string `%s' in vdispl spec\n", token);
+ return 1;
+ }
+
+ return 0;
+}
+
void parse_config_data(const char *config_source,
const char *config_data,
int config_len,
@@ -716,7 +735,7 @@ void parse_config_data(const char *config_source,
long l, vcpus = 0;
XLU_Config *config;
XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms,
- *usbctrls, *usbdevs;
+ *usbctrls, *usbdevs, *vdispls;
XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian, *dtdevs;
int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian;
int pci_power_mgmt = 0;
@@ -1295,6 +1314,29 @@ void parse_config_data(const char *config_source,
}
}
+ if (!xlu_cfg_get_list(config, "vdispl", &vdispls, 0, 0)) {
+ d_config->num_vdispls = 0;
+ d_config->vdispls = NULL;
+ while ((buf = xlu_cfg_get_listitem(vdispls, d_config->num_vdispls)) != NULL) {
+ libxl_device_vdispl *vdispl;
+ char * buf2 = strdup(buf);
+ char *p;
+ vdispl = ARRAY_EXTEND_INIT(d_config->vdispls,
+ d_config->num_vdispls,
+ libxl_device_vdispl_init);
+ p = strtok (buf2, ",");
+ while (p != NULL)
+ {
+ while (*p == ' ') p++;
+ if (parse_vdispl_config(vdispl, p)) {
+ exit(1);
+ }
+ p = strtok (NULL, ",");
+ }
+ free(buf2);
+ }
+ }
+
if (!xlu_cfg_get_list (config, "channel", &channels, 0, 0)) {
d_config->num_channels = 0;
d_config->channels = NULL;
diff --git a/tools/xl/xl_parse.h b/tools/xl/xl_parse.h
index db8bc3f..cc459fb 100644
--- a/tools/xl/xl_parse.h
+++ b/tools/xl/xl_parse.h
@@ -33,7 +33,7 @@ int parse_usbctrl_config(libxl_device_usbctrl *usbctrl, char *token);
int parse_usbdev_config(libxl_device_usbdev *usbdev, char *token);
int parse_cpurange(const char *cpu, libxl_bitmap *cpumap);
int parse_nic_config(libxl_device_nic *nic, XLU_Config **config, char *token);
-
+int parse_vdispl_config(libxl_device_vdispl *vdispl, char *token);
int match_option_size(const char *prefix, size_t len,
char *arg, char **argopt);
diff --git a/tools/xl/xl_vdispl.c b/tools/xl/xl_vdispl.c
new file mode 100644
index 0000000..4df90c9
--- /dev/null
+++ b/tools/xl/xl_vdispl.c
@@ -0,0 +1,162 @@
+/*
+ * Copyright (C) 2016 EPAM Systems Inc.
+ *
+ * This program 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; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program 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.
+ */
+
+#include <stdlib.h>
+
+#include <libxl.h>
+#include <libxl_utils.h>
+#include <libxlutil.h>
+
+#include "xl.h"
+#include "xl_utils.h"
+#include "xl_parse.h"
+
+int main_vdisplattach(int argc, char **argv)
+{
+ int opt;
+ int rc;
+ char *oparg;
+ uint32_t domid;
+ libxl_device_vdispl vdispl;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "vdispl-attach", 1) {
+ /* No options */
+ }
+
+ libxl_device_vdispl_init(&vdispl);
+ domid = find_domain(argv[optind++]);
+
+ for (argv += optind, argc -= optind; argc > 0; ++argv, --argc) {
+ if (MATCH_OPTION("devId", *argv, oparg)) {
+ vdispl.devid = atoi(oparg);
+ } else if (MATCH_OPTION("backend", *argv, oparg)) {
+ vdispl.backend_domid = find_domain(oparg);
+ replace_string(&vdispl.backend_domname, oparg);
+ } else if (MATCH_OPTION("beAlloc", *argv, oparg)) {
+ vdispl.be_alloc = strtoul(oparg, NULL, 0);
+ } else {
+ fprintf(stderr, "unrecognized argument `%s'\n", *argv);
+ rc = 1;
+ goto out;
+ }
+ }
+
+ if (dryrun_only) {
+ char *json = libxl_device_vdispl_to_json(ctx, &vdispl);
+ printf("vdispl: %s\n", json);
+ free(json);
+ goto out;
+ }
+
+ if (libxl_device_vdispl_add(ctx, domid, &vdispl, 0)) {
+ fprintf(stderr, "libxl_device_vdispl_add failed.\n");
+ rc = ERROR_FAIL;
+ }
+
+ rc = 0;
+
+out:
+ libxl_device_vdispl_dispose(&vdispl);
+ return rc;
+}
+
+int main_vdispllist(int argc, char **argv)
+{
+ int opt;
+ int i, n;
+ libxl_device_vdispl *vdispls;
+ libxl_vdisplinfo vdisplinfo;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "vdispl-list", 1) {
+ /* No options */
+ }
+
+ /* vdisplinfo.uuid should be outputted too */
+ printf("%-5s %-3s %-6s %-5s %-8s %-40s %-40s\n",
+ "Vdev", "BE", "handle", "state", "be-alloc", "BE-path", "FE-path");
+ for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
+ uint32_t domid;
+
+ if (libxl_domain_qualifier_to_domid(ctx, *argv, &domid) < 0) {
+ fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
+ continue;
+ }
+
+ vdispls = libxl_device_vdispl_list(ctx, domid, &n);
+
+ if (!vdispls) {
+ continue;
+ }
+
+ for (i = 0; i < n; i++) {
+ libxl_vdisplinfo_init(&vdisplinfo);
+ if (libxl_device_vdispl_getinfo(ctx, domid, &vdispls[i],
+ &vdisplinfo) == 0) {
+ /* Vdev BE hdl st be-alloc BE-path FE-path*/
+ printf("%-5d %-3d %-6d %-5d %-8d %-40s %-40s\n",
+ vdisplinfo.devid, vdisplinfo.backend_id,
+ vdisplinfo.frontend_id,
+ vdisplinfo.state, vdisplinfo.be_alloc,
+ vdisplinfo.backend, vdisplinfo.frontend);
+ }
+ libxl_vdisplinfo_dispose(&vdisplinfo);
+ }
+
+ libxl_device_vdispl_list_free(vdispls, n);
+ }
+ return 0;
+}
+
+int main_vdispldetach(int argc, char **argv)
+{
+ uint32_t domid, devid;
+ int opt, rc;
+ libxl_device_vdispl vdispl;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "vdispl-detach", 2) {
+ /* No options */
+ }
+
+ domid = find_domain(argv[optind++]);
+ devid = atoi(argv[optind++]);
+
+ libxl_device_vdispl_init(&vdispl);
+
+ if (libxl_devid_to_device_vdispl(ctx, domid, devid, &vdispl)) {
+ fprintf(stderr, "Error: Device %d not connected.\n", devid);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = libxl_device_vdispl_remove(ctx, domid, &vdispl, 0);
+ if (rc) {
+ fprintf(stderr, "libxl_device_vdispl_remove failed.\n");
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = 0;
+
+out:
+ libxl_device_vdispl_dispose(&vdispl);
+ return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libxl: add PV display device driver interface
2017-03-23 10:10 [PATCH 0/2] libxl: add PV display device driver interface Oleksandr Grytsov
2017-03-23 10:10 ` [PATCH 1/2] " Oleksandr Grytsov
2017-03-23 10:10 ` [PATCH 2/2] xl: add PV display device commands Oleksandr Grytsov
@ 2017-03-23 10:21 ` Juergen Gross
2017-03-23 11:32 ` al1img .
2 siblings, 1 reply; 11+ messages in thread
From: Juergen Gross @ 2017-03-23 10:21 UTC (permalink / raw)
To: Oleksandr Grytsov, xen-devel; +Cc: Oleksandr Grytsov
On 23/03/17 11:10, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Hi all,
>
> We are working on series of PV drivers (display, sound, input etc.) and
> would like to add their support to libxl and xl. These patches add PV display
> device. PV display is based on [1] protocol.
>
> During implementation I see a lot of code duplication. This problem was
> mentioned in [2]. One of the solutions was to implement common parts in IDL
> and make them autogenerated. On my side, to minimize the copy/pasting
> I've moved common parts into macro functions: LIBXL_DEFINE_DEVICE_COMMIT,
> LIBXL_DEFINE_DEVICE_LIST_GET, LIBXL_DEFINE_DEVICE_GETINFO etc.
> Existing PV devices implementations can be reworked to use these macros as
> well. Any other proposals to avoid the duplications are welcome.
Did you look into the device type framework I introduced with commit
74e857c6c7f9 and some followups? Maybe it is possible to expand this
framework by adding more callbacks to struct libxl_device_type and
have some common function(s) in libxl_device.c?
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libxl: add PV display device driver interface
2017-03-23 10:21 ` [PATCH 0/2] libxl: add PV display device driver interface Juergen Gross
@ 2017-03-23 11:32 ` al1img .
2017-03-23 12:08 ` Juergen Gross
0 siblings, 1 reply; 11+ messages in thread
From: al1img . @ 2017-03-23 11:32 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, Oleksandr Grytsov
Hi Juergen,
I've checked the mentioned commits. And I don't see how it can be done.
The duplication I see it is in libxl_device_type.add and
libxl_device_type.list functions
for different PV devices. These functions have a lot of common code
which I've tried
to move to macros in my patches.
2017-03-23 12:21 GMT+02:00 Juergen Gross <jgross@suse.com>:
> On 23/03/17 11:10, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Hi all,
>>
>> We are working on series of PV drivers (display, sound, input etc.) and
>> would like to add their support to libxl and xl. These patches add PV display
>> device. PV display is based on [1] protocol.
>>
>> During implementation I see a lot of code duplication. This problem was
>> mentioned in [2]. One of the solutions was to implement common parts in IDL
>> and make them autogenerated. On my side, to minimize the copy/pasting
>> I've moved common parts into macro functions: LIBXL_DEFINE_DEVICE_COMMIT,
>> LIBXL_DEFINE_DEVICE_LIST_GET, LIBXL_DEFINE_DEVICE_GETINFO etc.
>> Existing PV devices implementations can be reworked to use these macros as
>> well. Any other proposals to avoid the duplications are welcome.
>
> Did you look into the device type framework I introduced with commit
> 74e857c6c7f9 and some followups? Maybe it is possible to expand this
> framework by adding more callbacks to struct libxl_device_type and
> have some common function(s) in libxl_device.c?
>
> Juergen
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libxl: add PV display device driver interface
2017-03-23 11:32 ` al1img .
@ 2017-03-23 12:08 ` Juergen Gross
2017-03-23 14:23 ` al1img .
0 siblings, 1 reply; 11+ messages in thread
From: Juergen Gross @ 2017-03-23 12:08 UTC (permalink / raw)
To: al1img .; +Cc: xen-devel, Oleksandr Grytsov
On 23/03/17 12:32, al1img . wrote:
> Hi Juergen,
>
> I've checked the mentioned commits. And I don't see how it can be done.
> The duplication I see it is in libxl_device_type.add and
> libxl_device_type.list functions
> for different PV devices. These functions have a lot of common code
> which I've tried
> to move to macros in my patches.
Okay, here an example for replacement of LIBXL_DEFINE_DEVICE_LIST_FREE()
void libxl_device_list_free(struct libxl_device_type *dt,
void *list, int nr)
{
int i;
for (i = 0; i < nr; i++)
dt->dispose(list + i * dt->dev_elem_size);
free(list);
}
BTW: exactly this pattern is to be found at the very end of
libxl_retrieve_domain_configuration().
The others should be doable, too.
Juergen
>
> 2017-03-23 12:21 GMT+02:00 Juergen Gross <jgross@suse.com>:
>> On 23/03/17 11:10, Oleksandr Grytsov wrote:
>>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>>
>>> Hi all,
>>>
>>> We are working on series of PV drivers (display, sound, input etc.) and
>>> would like to add their support to libxl and xl. These patches add PV display
>>> device. PV display is based on [1] protocol.
>>>
>>> During implementation I see a lot of code duplication. This problem was
>>> mentioned in [2]. One of the solutions was to implement common parts in IDL
>>> and make them autogenerated. On my side, to minimize the copy/pasting
>>> I've moved common parts into macro functions: LIBXL_DEFINE_DEVICE_COMMIT,
>>> LIBXL_DEFINE_DEVICE_LIST_GET, LIBXL_DEFINE_DEVICE_GETINFO etc.
>>> Existing PV devices implementations can be reworked to use these macros as
>>> well. Any other proposals to avoid the duplications are welcome.
>>
>> Did you look into the device type framework I introduced with commit
>> 74e857c6c7f9 and some followups? Maybe it is possible to expand this
>> framework by adding more callbacks to struct libxl_device_type and
>> have some common function(s) in libxl_device.c?
>>
>> Juergen
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libxl: add PV display device driver interface
2017-03-23 12:08 ` Juergen Gross
@ 2017-03-23 14:23 ` al1img .
2017-03-23 14:58 ` Juergen Gross
0 siblings, 1 reply; 11+ messages in thread
From: al1img . @ 2017-03-23 14:23 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, Oleksandr Grytsov
This example is clear. But still wrapper macro is required to make it
visible for libxen client (xl):
#define LIBXL_DEFINE_DEVICE_LIST_FREE(type)
void libxl_device_##type##_list_free(libxl_device_##type* list, int nr)
{
libxl__device_list_free(libxl__##type##_devtype, list, nr);
}
Also where should this function (libxl__device_list_free) be located?
libxl_device.c?
Another case is getting this list:
From one side each device should have its own implementation, from
other side for PV devices
there is common part like getting devId and backend domId and unique
part like getting
device specific parameters (uuid for vtpm). In this case I can do following:
Implement void *libxl__device_list(struct libxl_device_type *dt,
libxl_ctx *ctx, uint32_t domid, int *num)
where I will get devId and backend domId. Then add get_params callback
to libxl_device_type to get device specific
parameters:
void *libxl__device_pv_list(struct libxl_device_type *dt, libxl_ctx
*ctx, uint32_t domid, int *num)
{
...
if (dt->get_patams) {
dt->get_params(...);
}
}
And vtpm get list may look like:
libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t
domid, int *num)
{
return libxl__device_list(&libxl__vtpm_devtype, ctx, domid, num);
}
DEFINE_DEVICE_TYPE_STRUCT(vtpm,
.update_config = libxl_device_vtpm_update_config
.get_params = libxl_device_vtpm_get_params
);
Correct?
2017-03-23 14:08 GMT+02:00 Juergen Gross <jgross@suse.com>:
> On 23/03/17 12:32, al1img . wrote:
>> Hi Juergen,
>>
>> I've checked the mentioned commits. And I don't see how it can be done.
>> The duplication I see it is in libxl_device_type.add and
>> libxl_device_type.list functions
>> for different PV devices. These functions have a lot of common code
>> which I've tried
>> to move to macros in my patches.
>
> Okay, here an example for replacement of LIBXL_DEFINE_DEVICE_LIST_FREE()
>
> void libxl_device_list_free(struct libxl_device_type *dt,
> void *list, int nr)
> {
> int i;
>
> for (i = 0; i < nr; i++)
> dt->dispose(list + i * dt->dev_elem_size);
> free(list);
> }
>
> BTW: exactly this pattern is to be found at the very end of
> libxl_retrieve_domain_configuration().
>
> The others should be doable, too.
>
>
> Juergen
>
>>
>> 2017-03-23 12:21 GMT+02:00 Juergen Gross <jgross@suse.com>:
>>> On 23/03/17 11:10, Oleksandr Grytsov wrote:
>>>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>>>
>>>> Hi all,
>>>>
>>>> We are working on series of PV drivers (display, sound, input etc.) and
>>>> would like to add their support to libxl and xl. These patches add PV display
>>>> device. PV display is based on [1] protocol.
>>>>
>>>> During implementation I see a lot of code duplication. This problem was
>>>> mentioned in [2]. One of the solutions was to implement common parts in IDL
>>>> and make them autogenerated. On my side, to minimize the copy/pasting
>>>> I've moved common parts into macro functions: LIBXL_DEFINE_DEVICE_COMMIT,
>>>> LIBXL_DEFINE_DEVICE_LIST_GET, LIBXL_DEFINE_DEVICE_GETINFO etc.
>>>> Existing PV devices implementations can be reworked to use these macros as
>>>> well. Any other proposals to avoid the duplications are welcome.
>>>
>>> Did you look into the device type framework I introduced with commit
>>> 74e857c6c7f9 and some followups? Maybe it is possible to expand this
>>> framework by adding more callbacks to struct libxl_device_type and
>>> have some common function(s) in libxl_device.c?
>>>
>>> Juergen
>>
>>
>>
>
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libxl: add PV display device driver interface
2017-03-23 14:23 ` al1img .
@ 2017-03-23 14:58 ` Juergen Gross
2017-03-23 15:55 ` al1img .
0 siblings, 1 reply; 11+ messages in thread
From: Juergen Gross @ 2017-03-23 14:58 UTC (permalink / raw)
To: al1img .; +Cc: xen-devel, Oleksandr Grytsov
On 23/03/17 15:23, al1img . wrote:
> This example is clear. But still wrapper macro is required to make it
> visible for libxen client (xl):
>
> #define LIBXL_DEFINE_DEVICE_LIST_FREE(type)
> void libxl_device_##type##_list_free(libxl_device_##type* list, int nr)
> {
> libxl__device_list_free(libxl__##type##_devtype, list, nr);
> }
Either this or we could switch to a more generic way avoiding the need
to add multiple very similar functions to libxl:
#define LIBXL_DEVTYPE_VTPM "vtpm"
int libxl_device_list_free(const char *type, void *list, int nr)
{
int i;
for (i = 0; device_type_tbl[i]; i++) {
if (!strcmp(type, device_type_tbl[i]->type)) {
libxl__device_list_free(device_type_tbl[i], list, nr);
return 0;
}
}
return ERROR_INVAL;
}
and let xl call it via libxl_device_list_free(LIBXL_DEVTYPE_VTPM, ...)
This is subject to an Ack from the tools maintainers, of course.
>
> Also where should this function (libxl__device_list_free) be located?
> libxl_device.c?
I think so.
>
> Another case is getting this list:
>
> From one side each device should have its own implementation, from
> other side for PV devices
> there is common part like getting devId and backend domId and unique
> part like getting
> device specific parameters (uuid for vtpm). In this case I can do following:
>
> Implement void *libxl__device_list(struct libxl_device_type *dt,
> libxl_ctx *ctx, uint32_t domid, int *num)
> where I will get devId and backend domId. Then add get_params callback
> to libxl_device_type to get device specific
> parameters:
>
> void *libxl__device_pv_list(struct libxl_device_type *dt, libxl_ctx
> *ctx, uint32_t domid, int *num)
> {
> ...
> if (dt->get_patams) {
> dt->get_params(...);
> }
> }
>
> And vtpm get list may look like:
>
> libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t
> domid, int *num)
> {
> return libxl__device_list(&libxl__vtpm_devtype, ctx, domid, num);
> }
>
> DEFINE_DEVICE_TYPE_STRUCT(vtpm,
> .update_config = libxl_device_vtpm_update_config
> .get_params = libxl_device_vtpm_get_params
> );
>
> Correct?
Right. Possibly using the same trick as above.
BTW: Please don't top-post.
Juergen
>
>
> 2017-03-23 14:08 GMT+02:00 Juergen Gross <jgross@suse.com>:
>> On 23/03/17 12:32, al1img . wrote:
>>> Hi Juergen,
>>>
>>> I've checked the mentioned commits. And I don't see how it can be done.
>>> The duplication I see it is in libxl_device_type.add and
>>> libxl_device_type.list functions
>>> for different PV devices. These functions have a lot of common code
>>> which I've tried
>>> to move to macros in my patches.
>>
>> Okay, here an example for replacement of LIBXL_DEFINE_DEVICE_LIST_FREE()
>>
>> void libxl_device_list_free(struct libxl_device_type *dt,
>> void *list, int nr)
>> {
>> int i;
>>
>> for (i = 0; i < nr; i++)
>> dt->dispose(list + i * dt->dev_elem_size);
>> free(list);
>> }
>>
>> BTW: exactly this pattern is to be found at the very end of
>> libxl_retrieve_domain_configuration().
>>
>> The others should be doable, too.
>>
>>
>> Juergen
>>
>>>
>>> 2017-03-23 12:21 GMT+02:00 Juergen Gross <jgross@suse.com>:
>>>> On 23/03/17 11:10, Oleksandr Grytsov wrote:
>>>>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>>>>
>>>>> Hi all,
>>>>>
>>>>> We are working on series of PV drivers (display, sound, input etc.) and
>>>>> would like to add their support to libxl and xl. These patches add PV display
>>>>> device. PV display is based on [1] protocol.
>>>>>
>>>>> During implementation I see a lot of code duplication. This problem was
>>>>> mentioned in [2]. One of the solutions was to implement common parts in IDL
>>>>> and make them autogenerated. On my side, to minimize the copy/pasting
>>>>> I've moved common parts into macro functions: LIBXL_DEFINE_DEVICE_COMMIT,
>>>>> LIBXL_DEFINE_DEVICE_LIST_GET, LIBXL_DEFINE_DEVICE_GETINFO etc.
>>>>> Existing PV devices implementations can be reworked to use these macros as
>>>>> well. Any other proposals to avoid the duplications are welcome.
>>>>
>>>> Did you look into the device type framework I introduced with commit
>>>> 74e857c6c7f9 and some followups? Maybe it is possible to expand this
>>>> framework by adding more callbacks to struct libxl_device_type and
>>>> have some common function(s) in libxl_device.c?
>>>>
>>>> Juergen
>>>
>>>
>>>
>>
>
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libxl: add PV display device driver interface
2017-03-23 14:58 ` Juergen Gross
@ 2017-03-23 15:55 ` al1img .
2017-03-24 10:35 ` Oleksandr Grytsov
0 siblings, 1 reply; 11+ messages in thread
From: al1img . @ 2017-03-23 15:55 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, Oleksandr Grytsov
On Thu, Mar 23, 2017 at 4:58 PM, Juergen Gross <jgross@suse.com> wrote:
> On 23/03/17 15:23, al1img . wrote:
>> This example is clear. But still wrapper macro is required to make it
>> visible for libxen client (xl):
>>
>> #define LIBXL_DEFINE_DEVICE_LIST_FREE(type)
>> void libxl_device_##type##_list_free(libxl_device_##type* list, int nr)
>> {
>> libxl__device_list_free(libxl__##type##_devtype, list, nr);
>> }
>
> Either this or we could switch to a more generic way avoiding the need
> to add multiple very similar functions to libxl:
>
> #define LIBXL_DEVTYPE_VTPM "vtpm"
>
> int libxl_device_list_free(const char *type, void *list, int nr)
> {
> int i;
>
> for (i = 0; device_type_tbl[i]; i++) {
> if (!strcmp(type, device_type_tbl[i]->type)) {
> libxl__device_list_free(device_type_tbl[i], list, nr);
> return 0;
> }
> }
> return ERROR_INVAL;
> }
>
> and let xl call it via libxl_device_list_free(LIBXL_DEVTYPE_VTPM, ...)
>
> This is subject to an Ack from the tools maintainers, of course.
>
>>
>> Also where should this function (libxl__device_list_free) be located?
>> libxl_device.c?
>
> I think so.
>
>>
>> Another case is getting this list:
>>
>> From one side each device should have its own implementation, from
>> other side for PV devices
>> there is common part like getting devId and backend domId and unique
>> part like getting
>> device specific parameters (uuid for vtpm). In this case I can do following:
>>
>> Implement void *libxl__device_list(struct libxl_device_type *dt,
>> libxl_ctx *ctx, uint32_t domid, int *num)
>> where I will get devId and backend domId. Then add get_params callback
>> to libxl_device_type to get device specific
>> parameters:
>>
>> void *libxl__device_pv_list(struct libxl_device_type *dt, libxl_ctx
>> *ctx, uint32_t domid, int *num)
>> {
>> ...
>> if (dt->get_patams) {
>> dt->get_params(...);
>> }
>> }
>>
>> And vtpm get list may look like:
>>
>> libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t
>> domid, int *num)
>> {
>> return libxl__device_list(&libxl__vtpm_devtype, ctx, domid, num);
>> }
>>
>> DEFINE_DEVICE_TYPE_STRUCT(vtpm,
>> .update_config = libxl_device_vtpm_update_config
>> .get_params = libxl_device_vtpm_get_params
>> );
>>
>> Correct?
>
> Right. Possibly using the same trick as above.
>
> BTW: Please don't top-post.
>
>
> Juergen
>
>>
>>
>> 2017-03-23 14:08 GMT+02:00 Juergen Gross <jgross@suse.com>:
>>> On 23/03/17 12:32, al1img . wrote:
>>>> Hi Juergen,
>>>>
>>>> I've checked the mentioned commits. And I don't see how it can be done.
>>>> The duplication I see it is in libxl_device_type.add and
>>>> libxl_device_type.list functions
>>>> for different PV devices. These functions have a lot of common code
>>>> which I've tried
>>>> to move to macros in my patches.
>>>
>>> Okay, here an example for replacement of LIBXL_DEFINE_DEVICE_LIST_FREE()
>>>
>>> void libxl_device_list_free(struct libxl_device_type *dt,
>>> void *list, int nr)
>>> {
>>> int i;
>>>
>>> for (i = 0; i < nr; i++)
>>> dt->dispose(list + i * dt->dev_elem_size);
>>> free(list);
>>> }
>>>
>>> BTW: exactly this pattern is to be found at the very end of
>>> libxl_retrieve_domain_configuration().
>>>
>>> The others should be doable, too.
>>>
>>>
>>> Juergen
>>>
>>>>
>>>> 2017-03-23 12:21 GMT+02:00 Juergen Gross <jgross@suse.com>:
>>>>> On 23/03/17 11:10, Oleksandr Grytsov wrote:
>>>>>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> We are working on series of PV drivers (display, sound, input etc.) and
>>>>>> would like to add their support to libxl and xl. These patches add PV display
>>>>>> device. PV display is based on [1] protocol.
>>>>>>
>>>>>> During implementation I see a lot of code duplication. This problem was
>>>>>> mentioned in [2]. One of the solutions was to implement common parts in IDL
>>>>>> and make them autogenerated. On my side, to minimize the copy/pasting
>>>>>> I've moved common parts into macro functions: LIBXL_DEFINE_DEVICE_COMMIT,
>>>>>> LIBXL_DEFINE_DEVICE_LIST_GET, LIBXL_DEFINE_DEVICE_GETINFO etc.
>>>>>> Existing PV devices implementations can be reworked to use these macros as
>>>>>> well. Any other proposals to avoid the duplications are welcome.
>>>>>
>>>>> Did you look into the device type framework I introduced with commit
>>>>> 74e857c6c7f9 and some followups? Maybe it is possible to expand this
>>>>> framework by adding more callbacks to struct libxl_device_type and
>>>>> have some common function(s) in libxl_device.c?
>>>>>
>>>>> Juergen
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>
For me the drawback of more generic way is to cast device_type to
void* and back which
may be used by client in improper way.
Thanks.
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libxl: add PV display device driver interface
2017-03-23 15:55 ` al1img .
@ 2017-03-24 10:35 ` Oleksandr Grytsov
2017-03-27 15:10 ` Wei Liu
0 siblings, 1 reply; 11+ messages in thread
From: Oleksandr Grytsov @ 2017-03-24 10:35 UTC (permalink / raw)
To: Juergen Gross; +Cc: xen-devel, Oleksandr Grytsov, Ian Jackson, Wei Liu
On 23.03.17 17:55, al1img . wrote:
> On Thu, Mar 23, 2017 at 4:58 PM, Juergen Gross <jgross@suse.com> wrote:
>> On 23/03/17 15:23, al1img . wrote:
>>> This example is clear. But still wrapper macro is required to make it
>>> visible for libxen client (xl):
>>>
>>> #define LIBXL_DEFINE_DEVICE_LIST_FREE(type)
>>> void libxl_device_##type##_list_free(libxl_device_##type* list, int nr)
>>> {
>>> libxl__device_list_free(libxl__##type##_devtype, list, nr);
>>> }
>>
>> Either this or we could switch to a more generic way avoiding the need
>> to add multiple very similar functions to libxl:
>>
>> #define LIBXL_DEVTYPE_VTPM "vtpm"
>>
>> int libxl_device_list_free(const char *type, void *list, int nr)
>> {
>> int i;
>>
>> for (i = 0; device_type_tbl[i]; i++) {
>> if (!strcmp(type, device_type_tbl[i]->type)) {
>> libxl__device_list_free(device_type_tbl[i], list, nr);
>> return 0;
>> }
>> }
>> return ERROR_INVAL;
>> }
>>
>> and let xl call it via libxl_device_list_free(LIBXL_DEVTYPE_VTPM, ...)
>>
>> This is subject to an Ack from the tools maintainers, of course.
>>
>>>
>>> Also where should this function (libxl__device_list_free) be located?
>>> libxl_device.c?
>>
>> I think so.
>>
>>>
>>> Another case is getting this list:
>>>
>>> From one side each device should have its own implementation, from
>>> other side for PV devices
>>> there is common part like getting devId and backend domId and unique
>>> part like getting
>>> device specific parameters (uuid for vtpm). In this case I can do following:
>>>
>>> Implement void *libxl__device_list(struct libxl_device_type *dt,
>>> libxl_ctx *ctx, uint32_t domid, int *num)
>>> where I will get devId and backend domId. Then add get_params callback
>>> to libxl_device_type to get device specific
>>> parameters:
>>>
>>> void *libxl__device_pv_list(struct libxl_device_type *dt, libxl_ctx
>>> *ctx, uint32_t domid, int *num)
>>> {
>>> ...
>>> if (dt->get_patams) {
>>> dt->get_params(...);
>>> }
>>> }
>>>
>>> And vtpm get list may look like:
>>>
>>> libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t
>>> domid, int *num)
>>> {
>>> return libxl__device_list(&libxl__vtpm_devtype, ctx, domid, num);
>>> }
>>>
>>> DEFINE_DEVICE_TYPE_STRUCT(vtpm,
>>> .update_config = libxl_device_vtpm_update_config
>>> .get_params = libxl_device_vtpm_get_params
>>> );
>>>
>>> Correct?
>>
>> Right. Possibly using the same trick as above.
>>
>> BTW: Please don't top-post.
>>
>>
>> Juergen
>>
>>>
>>>
>>> 2017-03-23 14:08 GMT+02:00 Juergen Gross <jgross@suse.com>:
>>>> On 23/03/17 12:32, al1img . wrote:
>>>>> Hi Juergen,
>>>>>
>>>>> I've checked the mentioned commits. And I don't see how it can be done.
>>>>> The duplication I see it is in libxl_device_type.add and
>>>>> libxl_device_type.list functions
>>>>> for different PV devices. These functions have a lot of common code
>>>>> which I've tried
>>>>> to move to macros in my patches.
>>>>
>>>> Okay, here an example for replacement of LIBXL_DEFINE_DEVICE_LIST_FREE()
>>>>
>>>> void libxl_device_list_free(struct libxl_device_type *dt,
>>>> void *list, int nr)
>>>> {
>>>> int i;
>>>>
>>>> for (i = 0; i < nr; i++)
>>>> dt->dispose(list + i * dt->dev_elem_size);
>>>> free(list);
>>>> }
>>>>
>>>> BTW: exactly this pattern is to be found at the very end of
>>>> libxl_retrieve_domain_configuration().
>>>>
>>>> The others should be doable, too.
>>>>
>>>>
>>>> Juergen
>>>>
>>>>>
>>>>> 2017-03-23 12:21 GMT+02:00 Juergen Gross <jgross@suse.com>:
>>>>>> On 23/03/17 11:10, Oleksandr Grytsov wrote:
>>>>>>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>>>>>>
>>>>>>> Hi all,
>>>>>>>
>>>>>>> We are working on series of PV drivers (display, sound, input etc.) and
>>>>>>> would like to add their support to libxl and xl. These patches add PV display
>>>>>>> device. PV display is based on [1] protocol.
>>>>>>>
>>>>>>> During implementation I see a lot of code duplication. This problem was
>>>>>>> mentioned in [2]. One of the solutions was to implement common parts in IDL
>>>>>>> and make them autogenerated. On my side, to minimize the copy/pasting
>>>>>>> I've moved common parts into macro functions: LIBXL_DEFINE_DEVICE_COMMIT,
>>>>>>> LIBXL_DEFINE_DEVICE_LIST_GET, LIBXL_DEFINE_DEVICE_GETINFO etc.
>>>>>>> Existing PV devices implementations can be reworked to use these macros as
>>>>>>> well. Any other proposals to avoid the duplications are welcome.
>>>>>>
>>>>>> Did you look into the device type framework I introduced with commit
>>>>>> 74e857c6c7f9 and some followups? Maybe it is possible to expand this
>>>>>> framework by adding more callbacks to struct libxl_device_type and
>>>>>> have some common function(s) in libxl_device.c?
>>>>>>
>>>>>> Juergen
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
>
> For me the drawback of more generic way is to cast device_type to
> void* and back which
> may be used by client in improper way.
>
> Thanks.
>
To summarize:
There are two ways to rework the patches:
1. Keep interface between xl an libxl as is and put duplicated code into
libxl_device_type specific functions.
2. Change interface to call libxl_device_type specific functions
directly from xl:
libxl_device_vtpm *vtpms;
vtpms = (libxl_device_vtpm*)libxl_device_list_get(LIBXL_DEVTYPE_VTPM, ...);
...
libxl__device_list_free(LIBXL_DEVTYPE_VTPM, vtpms, nr);
But I need feedback from maintainers.
Adding maintainers to CC.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] libxl: add PV display device driver interface
2017-03-24 10:35 ` Oleksandr Grytsov
@ 2017-03-27 15:10 ` Wei Liu
0 siblings, 0 replies; 11+ messages in thread
From: Wei Liu @ 2017-03-27 15:10 UTC (permalink / raw)
To: Oleksandr Grytsov
Cc: Juergen Gross, xen-devel, Oleksandr Grytsov, Ian Jackson, Wei Liu
On Fri, Mar 24, 2017 at 12:35:22PM +0200, Oleksandr Grytsov wrote:
> To summarize:
>
> There are two ways to rework the patches:
>
> 1. Keep interface between xl an libxl as is and put duplicated code into
> libxl_device_type specific functions.
>
> 2. Change interface to call libxl_device_type specific functions directly
> from xl:
>
> libxl_device_vtpm *vtpms;
>
> vtpms = (libxl_device_vtpm*)libxl_device_list_get(LIBXL_DEVTYPE_VTPM, ...);
>
> ...
>
> libxl__device_list_free(LIBXL_DEVTYPE_VTPM, vtpms, nr);
>
> But I need feedback from maintainers.
>
> Adding maintainers to CC.
I'm leaning towards #1 because we get type-safety that way.
Ian?
Wei.
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-03-27 15:10 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-23 10:10 [PATCH 0/2] libxl: add PV display device driver interface Oleksandr Grytsov
2017-03-23 10:10 ` [PATCH 1/2] " Oleksandr Grytsov
2017-03-23 10:10 ` [PATCH 2/2] xl: add PV display device commands Oleksandr Grytsov
2017-03-23 10:21 ` [PATCH 0/2] libxl: add PV display device driver interface Juergen Gross
2017-03-23 11:32 ` al1img .
2017-03-23 12:08 ` Juergen Gross
2017-03-23 14:23 ` al1img .
2017-03-23 14:58 ` Juergen Gross
2017-03-23 15:55 ` al1img .
2017-03-24 10:35 ` Oleksandr Grytsov
2017-03-27 15:10 ` Wei Liu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.