[PATCH v3 00/11] libxl: add PV display device driver interface

[PATCH v3 00/11] libxl: add PV display device driver interface

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

These patches add PV display device to libxl and xl. To avoid code
duplication the device handling framework was extended with following
callbacks:
 * set_default - initializes libxl_device_xxxx structure with default values;
 * to_device - converts libxl_device_xxxx to libxl__device;
 * init - initializes libxl_device_xxxx structure;
 * copy - copies libxl_device_xxxx to another libxl_device_xxxx;
 * from_xenstore - sets libxl_device_xxxx by xen store settings;
 * set_xenstore_config - sets xen store device config.

Also following generic functions (based on device type) are added:
 * libxl__device_add - adds PV device;
 * libxl__device_list - returns list of existing devices;
 * libxl__device_list_free - frees list of devices.

Changes since v2:
 * devide into small patches;
 * add commit messages

Oleksandr Grytsov (11):
  libxl: add vdispl structures to idl
  libxl: add API for PV display device driver
  libxl: add generic function to get and free device list
  libxl: add generic function to add device
  libxl: add vdispl setting xen store configuration
  libxl: implement vdispl get info function
  libxl: implement device_from_vdispl and update_config_vdispl
  libxl: add libxl__vdispl_devtype to device_type_tbl
  libxl: add libxl_devid_to_device_vdispl interface function
  xl: add PV display device commands
  docs: add PV display driver information

 docs/man/xl.cfg.pod.5.in             |  54 +++++
 docs/man/xl.pod.1.in                 |  42 ++++
 tools/libxl/Makefile                 |   2 +-
 tools/libxl/libxl.h                  |  21 ++
 tools/libxl/libxl_create.c           |   3 +
 tools/libxl/libxl_device.c           | 181 ++++++++++++++++-
 tools/libxl/libxl_internal.h         |  24 +++
 tools/libxl/libxl_types.idl          |  38 +++-
 tools/libxl/libxl_types_internal.idl |   1 +
 tools/libxl/libxl_usb.c              |   2 +
 tools/libxl/libxl_utils.h            |   4 +
 tools/libxl/libxl_vdispl.c           | 370 +++++++++++++++++++++++++++++++++++
 tools/xl/Makefile                    |   1 +
 tools/xl/xl.h                        |   3 +
 tools/xl/xl_cmdtable.c               |  19 ++
 tools/xl/xl_parse.c                  |  77 +++++++-
 tools/xl/xl_parse.h                  |   2 +-
 tools/xl/xl_vdispl.c                 | 166 ++++++++++++++++
 18 files changed, 1005 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

[PATCH v3 01/11] libxl: add vdispl structures to idl

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Add libxl_device_vdispl and libxl_vdisplinfo to libxl_types.idl
Add VDISPL to libxl__device_kind enumerator

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/libxl_types.idl          | 38 +++++++++++++++++++++++++++++++++++-
 tools/libxl/libxl_types_internal.idl |  1 +
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 2204425..25563cf 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -755,7 +755,21 @@ libxl_device_vtpm = Struct("device_vtpm", [
     ("backend_domname",  string),
     ("devid",            libxl_devid),
     ("uuid",             libxl_uuid),
-])
+    ])
+
+libxl_connector_param = Struct("connector_param", [
+    ("id", string),
+    ("width", uint32),
+    ("height", uint32)
+    ])
+
+libxl_device_vdispl = Struct("device_vdispl", [
+    ("backend_domid", libxl_domid),
+    ("backend_domname", string),
+    ("devid", libxl_devid),
+    ("be_alloc", bool),
+    ("connectors", Array(libxl_connector_param, "num_connectors"))
+    ])
 
 libxl_device_p9 = Struct("device_p9", [
     ("backend_domid",    libxl_domid),
@@ -791,6 +805,7 @@ libxl_domain_config = Struct("domain_config", [
     ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
     ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
     ("p9", Array(libxl_device_p9, "num_p9s")),
+    ("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")),
@@ -887,6 +902,27 @@ libxl_physinfo = Struct("physinfo", [
     ("cap_hvm_directio", bool),
     ], dir=DIR_OUT)
 
+libxl_connectorinfo = Struct("connectorinfo", [
+    ("id", string),
+    ("width", uint32),
+    ("height", uint32),
+    ("req_evtch", integer),
+    ("req_rref", integer),
+    ("evt_evtch", integer),
+    ("evt_rref", integer),
+    ], 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),
+    ("connectors", Array(libxl_connectorinfo, "num_connectors"))
+    ], 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 7dc4d0f..673a6d5 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -26,6 +26,7 @@ libxl__device_kind = Enumeration("device_kind", [
     (9, "VUSB"),
     (10, "QUSB"),
     (11, "9PFS"),
+    (12, "VDISPL"),
     ])
 
 libxl__console_backend = Enumeration("console_backend", [
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 01/11] libxl: add vdispl structures to idl

[Wei Liu]

On Tue, Jun 27, 2017 at 01:03:17PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> 
> Add libxl_device_vdispl and libxl_vdisplinfo to libxl_types.idl
> Add VDISPL to libxl__device_kind enumerator
> 
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> ---
>  tools/libxl/libxl_types.idl          | 38 +++++++++++++++++++++++++++++++++++-
>  tools/libxl/libxl_types_internal.idl |  1 +
>  2 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 2204425..25563cf 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -755,7 +755,21 @@ libxl_device_vtpm = Struct("device_vtpm", [
>      ("backend_domname",  string),
>      ("devid",            libxl_devid),
>      ("uuid",             libxl_uuid),
> -])
> +    ])
> +
> +libxl_connector_param = Struct("connector_param", [
> +    ("id", string),
> +    ("width", uint32),
> +    ("height", uint32)
> +    ])
> +
> +libxl_device_vdispl = Struct("device_vdispl", [
> +    ("backend_domid", libxl_domid),
> +    ("backend_domname", string),
> +    ("devid", libxl_devid),
> +    ("be_alloc", bool),

After reading the doc -- use "allocator" for this field?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 01/11] libxl: add vdispl structures to idl

[Oleksandr Grytsov]

Hi Wei,

Thanks for the review.

This field is already defined in the display protocol.
To avoid misunderstanding I prefer to use same name in libxl
as well.



On Thu, Jun 29, 2017 at 8:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jun 27, 2017 at 01:03:17PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Add libxl_device_vdispl and libxl_vdisplinfo to libxl_types.idl
>> Add VDISPL to libxl__device_kind enumerator
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> ---
>>  tools/libxl/libxl_types.idl          | 38 +++++++++++++++++++++++++++++++++++-
>>  tools/libxl/libxl_types_internal.idl |  1 +
>>  2 files changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index 2204425..25563cf 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -755,7 +755,21 @@ libxl_device_vtpm = Struct("device_vtpm", [
>>      ("backend_domname",  string),
>>      ("devid",            libxl_devid),
>>      ("uuid",             libxl_uuid),
>> -])
>> +    ])
>> +
>> +libxl_connector_param = Struct("connector_param", [
>> +    ("id", string),
>> +    ("width", uint32),
>> +    ("height", uint32)
>> +    ])
>> +
>> +libxl_device_vdispl = Struct("device_vdispl", [
>> +    ("backend_domid", libxl_domid),
>> +    ("backend_domname", string),
>> +    ("devid", libxl_devid),
>> +    ("be_alloc", bool),
>
> After reading the doc -- use "allocator" for this field?



-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 01/11] libxl: add vdispl structures to idl

[Wei Liu]

On Fri, Jun 30, 2017 at 01:36:46PM +0300, Oleksandr Grytsov wrote:
> Hi Wei,
> 
> Thanks for the review.
> 
> This field is already defined in the display protocol.
> To avoid misunderstanding I prefer to use same name in libxl
> as well.
> 
> 

Never mind then. Keep it as-is.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 02/11] libxl: add API for PV display device driver

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

add libxl_vdispl.c with API for display device driver
add libxl_vdispl.o to Makefile

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/Makefile       |  2 +-
 tools/libxl/libxl.h        | 21 +++++++++++++
 tools/libxl/libxl_vdispl.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 tools/libxl/libxl_vdispl.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 1bf6b8c..6f57e65 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_9pfs.o libxl_domain.o \
+			libxl_9pfs.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 cf8687a..af8d9ce 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1851,6 +1851,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_vdispl.c b/tools/libxl/libxl_vdispl.c
new file mode 100644
index 0000000..2658e25
--- /dev/null
+++ b/tools/libxl/libxl_vdispl.c
@@ -0,0 +1,77 @@
+/*
+ * 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_internal.h"
+
+static int libxl__device_from_vdispl(libxl__gc *gc, uint32_t domid,
+                                     libxl_device_vdispl *vdispl,
+                                     libxl__device *device)
+{
+   return 0;
+}
+
+static void libxl__update_config_vdispl(libxl__gc *gc,
+                                        libxl_device_vdispl *dst,
+                                        libxl_device_vdispl *src)
+{
+
+}
+
+static int libxl_device_vdispl_compare(libxl_device_vdispl *d1,
+                                       libxl_device_vdispl *d2)
+{
+    return COMPARE_DEVID(d1, d2);
+}
+
+static void libxl__device_vdispl_add(libxl__egc *egc, uint32_t domid,
+                                     libxl_device_vdispl *vdispl,
+                                     libxl__ao_device *aodev)
+{
+
+}
+
+libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid,
+                                              int *num)
+{
+    return NULL;
+}
+
+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 *info)
+{
+     return 0;
+}
+
+LIBXL_DEFINE_DEVICE_ADD(vdispl)
+static LIBXL_DEFINE_DEVICES_ADD(vdispl)
+LIBXL_DEFINE_DEVICE_REMOVE(vdispl)
+
+DEFINE_DEVICE_TYPE_STRUCT(vdispl,
+    .update_config = (void (*)(libxl__gc *, void *, void *))
+                     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

[PATCH v3 03/11] libxl: add generic function to get and free device list

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Add libxl__device_list, libxl__device_list_free.
Device list is created from libxl xen store entries.
In order to fill libxl device structure from xen store,
the device handling framework extended with from_xenstore callback.
On this callback libxl_device shall be filled with data from
be xen store directory.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/libxl_device.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h |  8 +++++
 tools/libxl/libxl_vdispl.c   | 17 ++++++++--
 3 files changed, 98 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 00356af..8bcfa2b 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1793,6 +1793,82 @@ out:
     return AO_CREATE_FAIL(rc);
 }
 
+void* libxl__device_list(const struct libxl_device_type *dt,
+                         libxl_ctx *ctx, uint32_t domid, int *num)
+{
+    GC_INIT(ctx);
+
+    void *r = NULL;
+    void *list = NULL;
+    void *item = NULL;
+    char *libxl_path;
+    char *be_path;
+    char** dir = NULL;
+    unsigned int ndirs = 0;
+    int rc;
+
+    *num = 0;
+
+    libxl_path = GCSPRINTF("%s/device/%s",
+                           libxl__xs_libxl_path(gc, domid), dt->type);
+
+    dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
+
+    if (dir && ndirs) {
+        list = malloc(dt->dev_elem_size * ndirs);
+        void *end = (uint8_t*)list + ndirs * dt->dev_elem_size;
+        item = list;
+
+        while(item < end) {
+            be_path = libxl__xs_read(gc, XBT_NULL,
+                                     GCSPRINTF("%s/%s/backend",
+                                     libxl_path, *dir));
+
+            dt->init(item);
+
+            if (dt->from_xenstore)
+            {
+                rc = dt->from_xenstore(gc, be_path, atoi(*dir), item);
+                if (rc) goto out;
+            }
+
+            item = (uint8_t*)item + dt->dev_elem_size;
+            ++dir;
+        }
+    }
+
+    *num = ndirs;
+    r = list;
+    list = NULL;
+
+out:
+
+    if (list) {
+        *num = 0;
+        while(item >= list) {
+            item = (uint8_t*)item - dt->dev_elem_size;
+            dt->dispose(item);
+        }
+        free(list);
+    }
+
+    GC_FREE;
+
+    return r;
+}
+
+void libxl__device_list_free(const struct libxl_device_type *dt,
+                             void *list, int num)
+{
+    int i;
+
+    for (i = 0; i < num; i++) {
+        dt->dispose((uint8_t*)list + i * dt->dev_elem_size);
+    }
+
+    free(list);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index afe6652..c192559 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3484,11 +3484,13 @@ struct libxl_device_type {
     void (*add)(libxl__egc *, libxl__ao *, uint32_t, libxl_domain_config *,
                 libxl__multidev *);
     void *(*list)(libxl_ctx *, uint32_t, int *);
+    void (*init)(void *);
     void (*dispose)(void *);
     int (*compare)(void *, void *);
     void (*merge)(libxl_ctx *, void *, void *);
     int (*dm_needed)(void *, unsigned);
     void (*update_config)(libxl__gc *, void *, void *);
+    int (*from_xenstore)(libxl__gc *, const char *, uint32_t, void *);
 };
 
 #define DEFINE_DEVICE_TYPE_STRUCT_X(name, sname, ...)                          \
@@ -3500,6 +3502,7 @@ struct libxl_device_type {
         .add           = libxl__add_ ## name ## s,                             \
         .list          = (void *(*)(libxl_ctx *, uint32_t, int *))             \
                          libxl_device_ ## sname ## _list,                      \
+        .init          = (void (*)(void *))libxl_device_ ## sname ## _init,    \
         .dispose       = (void (*)(void *))libxl_device_ ## sname ## _dispose, \
         .compare       = (int (*)(void *, void *))                             \
                          libxl_device_ ## sname ## _compare,                   \
@@ -3534,6 +3537,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[];
 
@@ -4350,6 +4354,10 @@ static inline bool libxl__acpi_defbool_val(const libxl_domain_build_info *b_info
     return libxl_defbool_val(b_info->acpi) &&
            libxl_defbool_val(b_info->u.hvm.acpi);
 }
+void* libxl__device_list(const struct libxl_device_type *dt,
+                         libxl_ctx *ctx, uint32_t domid, int *num);
+void libxl__device_list_free(const struct libxl_device_type *dt,
+                             void *list, int num);
 #endif
 
 /*
diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
index 2658e25..a628adc 100644
--- a/tools/libxl/libxl_vdispl.c
+++ b/tools/libxl/libxl_vdispl.c
@@ -21,6 +21,15 @@ static int libxl__device_from_vdispl(libxl__gc *gc, uint32_t domid,
    return 0;
 }
 
+static int libxl__from_xenstore_vdispl(libxl__gc *gc, const char *be_path,
+                                       uint32_t devid,
+                                       libxl_device_vdispl *vdispl)
+{
+    vdispl->devid = devid;
+
+    return libxl__backendpath_parse_domid(gc, be_path, &vdispl->backend_domid);
+}
+
 static void libxl__update_config_vdispl(libxl__gc *gc,
                                         libxl_device_vdispl *dst,
                                         libxl_device_vdispl *src)
@@ -44,12 +53,12 @@ static void libxl__device_vdispl_add(libxl__egc *egc, uint32_t domid,
 libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid,
                                               int *num)
 {
-    return NULL;
+    return libxl__device_list(&libxl__vdispl_devtype, ctx, domid, num);
 }
 
 void libxl_device_vdispl_list_free(libxl_device_vdispl* list, int num)
 {
-
+    libxl__device_list_free(&libxl__vdispl_devtype, list, num);
 }
 
 int libxl_device_vdispl_getinfo(libxl_ctx *ctx, uint32_t domid,
@@ -65,7 +74,9 @@ LIBXL_DEFINE_DEVICE_REMOVE(vdispl)
 
 DEFINE_DEVICE_TYPE_STRUCT(vdispl,
     .update_config = (void (*)(libxl__gc *, void *, void *))
-                     libxl__update_config_vdispl
+                     libxl__update_config_vdispl,
+    .from_xenstore = (int (*)(libxl__gc *, const char *, uint32_t, void *))
+                     libxl__from_xenstore_vdispl
 );
 
 /*
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 03/11] libxl: add generic function to get and free device list

[Wei Liu]

On Tue, Jun 27, 2017 at 01:03:19PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> 
> Add libxl__device_list, libxl__device_list_free.
> Device list is created from libxl xen store entries.
> In order to fill libxl device structure from xen store,
> the device handling framework extended with from_xenstore callback.
> On this callback libxl_device shall be filled with data from
> be xen store directory.
> 
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> ---
>  tools/libxl/libxl_device.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_internal.h |  8 +++++
>  tools/libxl/libxl_vdispl.c   | 17 ++++++++--
>  3 files changed, 98 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 00356af..8bcfa2b 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -1793,6 +1793,82 @@ out:
>      return AO_CREATE_FAIL(rc);
>  }
>  
> +void* libxl__device_list(const struct libxl_device_type *dt,
> +                         libxl_ctx *ctx, uint32_t domid, int *num)

void *libxl_...

> +{
> +    GC_INIT(ctx);
> +
> +    void *r = NULL;
> +    void *list = NULL;
> +    void *item = NULL;
> +    char *libxl_path;
> +    char *be_path;
> +    char** dir = NULL;

char **dir

> +    unsigned int ndirs = 0;
> +    int rc;
> +
> +    *num = 0;
> +
> +    libxl_path = GCSPRINTF("%s/device/%s",
> +                           libxl__xs_libxl_path(gc, domid), dt->type);
> +
> +    dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
> +
> +    if (dir && ndirs) {
> +        list = malloc(dt->dev_elem_size * ndirs);
> +        void *end = (uint8_t*)list + ndirs * dt->dev_elem_size;

(uint8_t *)

> +        item = list;
> +
> +        while(item < end) {
> +            be_path = libxl__xs_read(gc, XBT_NULL,
> +                                     GCSPRINTF("%s/%s/backend",
> +                                     libxl_path, *dir));
> +
> +            dt->init(item);
> +
> +            if (dt->from_xenstore)
> +            {

Move { to previous line.

> +                rc = dt->from_xenstore(gc, be_path, atoi(*dir), item);
> +                if (rc) goto out;
> +            }
> +
> +            item = (uint8_t*)item + dt->dev_elem_size;
> +            ++dir;
> +        }
> +    }
> +
> +    *num = ndirs;
> +    r = list;
> +    list = NULL;
> +
> +out:
> +
> +    if (list) {
> +        *num = 0;
> +        while(item >= list) {

Space after while.

> +            item = (uint8_t*)item - dt->dev_elem_size;
> +            dt->dispose(item);
> +        }
> +        free(list);
> +    }
> +
> +    GC_FREE;
> +
> +    return r;
> +}
> +
> +void libxl__device_list_free(const struct libxl_device_type *dt,
> +                             void *list, int num)
> +{
> +    int i;
> +
> +    for (i = 0; i < num; i++) {
> +        dt->dispose((uint8_t*)list + i * dt->dev_elem_size);
> +    }
> +

No need to have {}.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 03/11] libxl: add generic function to get and free device list

[Oleksandr Grytsov]

Thanks, I will fix it.

On Thu, Jun 29, 2017 at 8:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jun 27, 2017 at 01:03:19PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Add libxl__device_list, libxl__device_list_free.
>> Device list is created from libxl xen store entries.
>> In order to fill libxl device structure from xen store,
>> the device handling framework extended with from_xenstore callback.
>> On this callback libxl_device shall be filled with data from
>> be xen store directory.
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> ---
>>  tools/libxl/libxl_device.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++
>>  tools/libxl/libxl_internal.h |  8 +++++
>>  tools/libxl/libxl_vdispl.c   | 17 ++++++++--
>>  3 files changed, 98 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>> index 00356af..8bcfa2b 100644
>> --- a/tools/libxl/libxl_device.c
>> +++ b/tools/libxl/libxl_device.c
>> @@ -1793,6 +1793,82 @@ out:
>>      return AO_CREATE_FAIL(rc);
>>  }
>>
>> +void* libxl__device_list(const struct libxl_device_type *dt,
>> +                         libxl_ctx *ctx, uint32_t domid, int *num)
>
> void *libxl_...
>
>> +{
>> +    GC_INIT(ctx);
>> +
>> +    void *r = NULL;
>> +    void *list = NULL;
>> +    void *item = NULL;
>> +    char *libxl_path;
>> +    char *be_path;
>> +    char** dir = NULL;
>
> char **dir
>
>> +    unsigned int ndirs = 0;
>> +    int rc;
>> +
>> +    *num = 0;
>> +
>> +    libxl_path = GCSPRINTF("%s/device/%s",
>> +                           libxl__xs_libxl_path(gc, domid), dt->type);
>> +
>> +    dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
>> +
>> +    if (dir && ndirs) {
>> +        list = malloc(dt->dev_elem_size * ndirs);
>> +        void *end = (uint8_t*)list + ndirs * dt->dev_elem_size;
>
> (uint8_t *)
>
>> +        item = list;
>> +
>> +        while(item < end) {
>> +            be_path = libxl__xs_read(gc, XBT_NULL,
>> +                                     GCSPRINTF("%s/%s/backend",
>> +                                     libxl_path, *dir));
>> +
>> +            dt->init(item);
>> +
>> +            if (dt->from_xenstore)
>> +            {
>
> Move { to previous line.
>
>> +                rc = dt->from_xenstore(gc, be_path, atoi(*dir), item);
>> +                if (rc) goto out;
>> +            }
>> +
>> +            item = (uint8_t*)item + dt->dev_elem_size;
>> +            ++dir;
>> +        }
>> +    }
>> +
>> +    *num = ndirs;
>> +    r = list;
>> +    list = NULL;
>> +
>> +out:
>> +
>> +    if (list) {
>> +        *num = 0;
>> +        while(item >= list) {
>
> Space after while.
>
>> +            item = (uint8_t*)item - dt->dev_elem_size;
>> +            dt->dispose(item);
>> +        }
>> +        free(list);
>> +    }
>> +
>> +    GC_FREE;
>> +
>> +    return r;
>> +}
>> +
>> +void libxl__device_list_free(const struct libxl_device_type *dt,
>> +                             void *list, int num)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < num; i++) {
>> +        dt->dispose((uint8_t*)list + i * dt->dev_elem_size);
>> +    }
>> +
>
> No need to have {}.



-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 03/11] libxl: add generic function to get and free device list

[Wei Liu]

On Tue, Jun 27, 2017 at 01:03:19PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> 
> Add libxl__device_list, libxl__device_list_free.
> Device list is created from libxl xen store entries.
> In order to fill libxl device structure from xen store,
> the device handling framework extended with from_xenstore callback.
> On this callback libxl_device shall be filled with data from
> be xen store directory.
> 
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> ---
>  tools/libxl/libxl_device.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/libxl_internal.h |  8 +++++
>  tools/libxl/libxl_vdispl.c   | 17 ++++++++--
>  3 files changed, 98 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 00356af..8bcfa2b 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -1793,6 +1793,82 @@ out:
>      return AO_CREATE_FAIL(rc);
>  }
>  
> +void* libxl__device_list(const struct libxl_device_type *dt,
> +                         libxl_ctx *ctx, uint32_t domid, int *num)

It should probably take a libxl__gc *gc here.

> +{
> +    GC_INIT(ctx);
> +

And omit the GC_INIT and GC_FREE.

> +    void *r = NULL;
> +    void *list = NULL;
> +    void *item = NULL;
> +    char *libxl_path;
> +    char *be_path;
> +    char** dir = NULL;
> +    unsigned int ndirs = 0;
> +    int rc;
> +
> +    *num = 0;
> +
> +    libxl_path = GCSPRINTF("%s/device/%s",
> +                           libxl__xs_libxl_path(gc, domid), dt->type);
> +
> +    dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
> +
> +    if (dir && ndirs) {
> +        list = malloc(dt->dev_elem_size * ndirs);

Also please use libxl__malloc here.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 03/11] libxl: add generic function to get and free device list

[Oleksandr Grytsov]

On Thu, Jul 6, 2017 at 6:29 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jun 27, 2017 at 01:03:19PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Add libxl__device_list, libxl__device_list_free.
>> Device list is created from libxl xen store entries.
>> In order to fill libxl device structure from xen store,
>> the device handling framework extended with from_xenstore callback.
>> On this callback libxl_device shall be filled with data from
>> be xen store directory.
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> ---
>>  tools/libxl/libxl_device.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++
>>  tools/libxl/libxl_internal.h |  8 +++++
>>  tools/libxl/libxl_vdispl.c   | 17 ++++++++--
>>  3 files changed, 98 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>> index 00356af..8bcfa2b 100644
>> --- a/tools/libxl/libxl_device.c
>> +++ b/tools/libxl/libxl_device.c
>> @@ -1793,6 +1793,82 @@ out:
>>      return AO_CREATE_FAIL(rc);
>>  }
>>
>> +void* libxl__device_list(const struct libxl_device_type *dt,
>> +                         libxl_ctx *ctx, uint32_t domid, int *num)
>
> It should probably take a libxl__gc *gc here.
>
>> +{
>> +    GC_INIT(ctx);
>> +
>
> And omit the GC_INIT and GC_FREE.
>

In this case I should move GC_INIT and GC_FREE to above function:

libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid, int *num)
{
     GC_INIT(ctx);


}

>> +    void *r = NULL;
>> +    void *list = NULL;
>> +    void *item = NULL;
>> +    char *libxl_path;
>> +    char *be_path;
>> +    char** dir = NULL;
>> +    unsigned int ndirs = 0;
>> +    int rc;
>> +
>> +    *num = 0;
>> +
>> +    libxl_path = GCSPRINTF("%s/device/%s",
>> +                           libxl__xs_libxl_path(gc, domid), dt->type);
>> +
>> +    dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
>> +
>> +    if (dir && ndirs) {
>> +        list = malloc(dt->dev_elem_size * ndirs);
>
> Also please use libxl__malloc here.



-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 03/11] libxl: add generic function to get and free device list

[Oleksandr Grytsov]

On Mon, Jul 10, 2017 at 3:22 PM, Oleksandr Grytsov <al1img@gmail.com> wrote:
> On Thu, Jul 6, 2017 at 6:29 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> On Tue, Jun 27, 2017 at 01:03:19PM +0300, Oleksandr Grytsov wrote:
>>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>>
>>> Add libxl__device_list, libxl__device_list_free.
>>> Device list is created from libxl xen store entries.
>>> In order to fill libxl device structure from xen store,
>>> the device handling framework extended with from_xenstore callback.
>>> On this callback libxl_device shall be filled with data from
>>> be xen store directory.
>>>
>>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>> ---
>>>  tools/libxl/libxl_device.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++
>>>  tools/libxl/libxl_internal.h |  8 +++++
>>>  tools/libxl/libxl_vdispl.c   | 17 ++++++++--
>>>  3 files changed, 98 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>>> index 00356af..8bcfa2b 100644
>>> --- a/tools/libxl/libxl_device.c
>>> +++ b/tools/libxl/libxl_device.c
>>> @@ -1793,6 +1793,82 @@ out:
>>>      return AO_CREATE_FAIL(rc);
>>>  }
>>>
>>> +void* libxl__device_list(const struct libxl_device_type *dt,
>>> +                         libxl_ctx *ctx, uint32_t domid, int *num)
>>
>> It should probably take a libxl__gc *gc here.
>>
>>> +{
>>> +    GC_INIT(ctx);
>>> +
>>
>> And omit the GC_INIT and GC_FREE.
>>
>
> In this case I should move GC_INIT and GC_FREE to above function:
>
> libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid, int *num)
> {
>      GC_INIT(ctx);
>
>
> }
>

It means for each device where getting device list is required there will be
GC_INIT(ctc)

libxl__device_list(gc, ...)

GC_FREE

instead of just:

libxl__device_list(ctx, ...);

>>> +    void *r = NULL;
>>> +    void *list = NULL;
>>> +    void *item = NULL;
>>> +    char *libxl_path;
>>> +    char *be_path;
>>> +    char** dir = NULL;
>>> +    unsigned int ndirs = 0;
>>> +    int rc;
>>> +
>>> +    *num = 0;
>>> +
>>> +    libxl_path = GCSPRINTF("%s/device/%s",
>>> +                           libxl__xs_libxl_path(gc, domid), dt->type);
>>> +
>>> +    dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
>>> +
>>> +    if (dir && ndirs) {
>>> +        list = malloc(dt->dev_elem_size * ndirs);
>>
>> Also please use libxl__malloc here.
>
>
>
> --
> Best Regards,
> Oleksandr Grytsov.



-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 03/11] libxl: add generic function to get and free device list

[Wei Liu]

On Mon, Jul 10, 2017 at 03:26:12PM +0300, Oleksandr Grytsov wrote:
> It means for each device where getting device list is required there will be
> GC_INIT(ctc)
> 
> libxl__device_list(gc, ...)
> 
> GC_FREE
> 
> instead of just:
> 
> libxl__device_list(ctx, ...);

I think this is worth it because we might need to use the
libl__device_list function internally.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 03/11] libxl: add generic function to get and free device list

[Oleksandr Grytsov]

On Wed, Jul 12, 2017 at 12:51 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Mon, Jul 10, 2017 at 03:26:12PM +0300, Oleksandr Grytsov wrote:
>> It means for each device where getting device list is required there will be
>> GC_INIT(ctc)
>>
>> libxl__device_list(gc, ...)
>>
>> GC_FREE
>>
>> instead of just:
>>
>> libxl__device_list(ctx, ...);
>
> I think this is worth it because we might need to use the
> libl__device_list function internally.

I've reworked the patch series and done it in following way:

libxl__device_list takes gc and interface function init CTX:

libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t
domid, int *num)
{
    libxl_device_disk *r;

    GC_INIT(ctx);

    r = libxl__device_list(gc, &libxl__disk_devtype, domid, "disk", num);

    GC_FREE;

    return r;
}

There was comment to use libxl_malloc instead of malloc in libxl__device_list.
But it can't be used because calling GC_FREE frees the list.
So I've left malloc and free the list in libxl__device_list_free.

-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 03/11] libxl: add generic function to get and free device list

[Wei Liu]

On Wed, Jul 12, 2017 at 04:43:23PM +0300, Oleksandr Grytsov wrote:
> On Wed, Jul 12, 2017 at 12:51 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Mon, Jul 10, 2017 at 03:26:12PM +0300, Oleksandr Grytsov wrote:
> >> It means for each device where getting device list is required there will be
> >> GC_INIT(ctc)
> >>
> >> libxl__device_list(gc, ...)
> >>
> >> GC_FREE
> >>
> >> instead of just:
> >>
> >> libxl__device_list(ctx, ...);
> >
> > I think this is worth it because we might need to use the
> > libl__device_list function internally.
> 
> I've reworked the patch series and done it in following way:
> 
> libxl__device_list takes gc and interface function init CTX:
> 
> libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t
> domid, int *num)
> {
>     libxl_device_disk *r;
> 
>     GC_INIT(ctx);
> 
>     r = libxl__device_list(gc, &libxl__disk_devtype, domid, "disk", num);
> 
>     GC_FREE;
> 
>     return r;
> }
> 
> There was comment to use libxl_malloc instead of malloc in libxl__device_list.
> But it can't be used because calling GC_FREE frees the list.
> So I've left malloc and free the list in libxl__device_list_free.
> 

You can pass in NO_GC (NOGC) to libxl__malloc.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 03/11] libxl: add generic function to get and free device list

[Wei Liu]

On Mon, Jul 10, 2017 at 03:22:19PM +0300, Oleksandr Grytsov wrote:
> On Thu, Jul 6, 2017 at 6:29 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Tue, Jun 27, 2017 at 01:03:19PM +0300, Oleksandr Grytsov wrote:
> >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >>
> >> Add libxl__device_list, libxl__device_list_free.
> >> Device list is created from libxl xen store entries.
> >> In order to fill libxl device structure from xen store,
> >> the device handling framework extended with from_xenstore callback.
> >> On this callback libxl_device shall be filled with data from
> >> be xen store directory.
> >>
> >> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >> ---
> >>  tools/libxl/libxl_device.c   | 76 ++++++++++++++++++++++++++++++++++++++++++++
> >>  tools/libxl/libxl_internal.h |  8 +++++
> >>  tools/libxl/libxl_vdispl.c   | 17 ++++++++--
> >>  3 files changed, 98 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> >> index 00356af..8bcfa2b 100644
> >> --- a/tools/libxl/libxl_device.c
> >> +++ b/tools/libxl/libxl_device.c
> >> @@ -1793,6 +1793,82 @@ out:
> >>      return AO_CREATE_FAIL(rc);
> >>  }
> >>
> >> +void* libxl__device_list(const struct libxl_device_type *dt,
> >> +                         libxl_ctx *ctx, uint32_t domid, int *num)
> >
> > It should probably take a libxl__gc *gc here.
> >
> >> +{
> >> +    GC_INIT(ctx);
> >> +
> >
> > And omit the GC_INIT and GC_FREE.
> >
> 
> In this case I should move GC_INIT and GC_FREE to above function:
> 
> libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid, int *num)
> {
>      GC_INIT(ctx);
> 

Yes that's what I meant.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Add libxl__device_add functio.
Almost all devices have similar libxl__device_xxxx_add function.
This generic function implements same functionality but
using the device handling framework. The device specific
part this is setting xen store configuration. This part
is moved to set_xenstore_config callback of the device framework.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/libxl_create.c   |   2 +
 tools/libxl/libxl_device.c   | 103 +++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.h |  15 +++++++
 tools/libxl/libxl_usb.c      |   2 +
 tools/libxl/libxl_vdispl.c   |  17 ++++++-
 5 files changed, 138 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bffbc45..4e5ba29 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1430,6 +1430,8 @@ out:
 
 #define libxl_device_dtdev_list NULL
 #define libxl_device_dtdev_compare NULL
+#define libxl__device_from_dtdev NULL
+#define libxl__device_dtdev_setdefault NULL
 static DEFINE_DEVICE_TYPE_STRUCT(dtdev);
 
 const struct libxl_device_type *device_type_tbl[] = {
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 8bcfa2b..e202503 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1793,6 +1793,109 @@ out:
     return AO_CREATE_FAIL(rc);
 }
 
+static int device_add_domain_config(libxl__gc *gc, uint32_t domid,
+                                    const struct libxl_device_type *dt,
+                                    void *type)
+{
+    int rc;
+    libxl_domain_config d_config;
+    libxl__domain_userdata_lock *lock = NULL;
+    int *num_dev;
+    int i;
+    void *item = NULL;
+
+    libxl_domain_config_init(&d_config);
+
+    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;
+
+    num_dev = libxl__device_type_get_num(dt, &d_config);
+
+    /* Check for existing device */
+    for (i = 0; i < *num_dev; i++) {
+        if (dt->compare(libxl__device_type_get_elem(dt, &d_config, i), type)) {
+            item = libxl__device_type_get_elem(dt, &d_config, i);
+        }
+    }
+
+    if (!item) {
+        void **devs= libxl__device_type_get_ptr(dt, &d_config);
+        *devs = libxl__realloc(NOGC, *devs,
+                               dt->dev_elem_size * (*num_dev + 1));
+        item = libxl__device_type_get_elem(dt, &d_config, *num_dev);
+        (*num_dev)++;
+    } else {
+        dt->dispose(item);
+    }
+
+    dt->init(item);
+    dt->copy(CTX, item, type);
+
+    rc = libxl__dm_check_start(gc, &d_config, domid);
+    if (rc) goto out;
+
+    rc = libxl__set_domain_configuration(gc, domid, &d_config);
+    if (rc) goto out;
+
+    rc = 0;
+
+out:
+    if (lock) libxl__unlock_domain_userdata(lock);
+    libxl_domain_config_dispose(&d_config);
+    return rc;
+}
+
+void libxl__device_add(libxl__egc *egc, uint32_t domid,
+                       const struct libxl_device_type *dt, void *type,
+                       libxl__ao_device *aodev)
+{
+    STATE_AO_GC(aodev->ao);
+    libxl__device *device;
+    int rc;
+
+    rc = dt->set_default(gc, domid, type);
+    if (rc) goto out;
+
+    GCNEW(device);
+    rc = dt->to_device(gc, domid, type, device);
+    if ( rc != 0 ) goto out;
+
+    rc = libxl__device_exists(gc, XBT_NULL, device);
+    if (rc < 0) goto out;
+    if (rc == 1) {              /* already exists in xenstore */
+        LOGD(ERROR, domid, "device already exists in xenstore");
+        aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */
+        rc = ERROR_DEVICE_EXISTS;
+        goto out;
+    }
+
+    if (aodev->update_json) {
+        rc = device_add_domain_config(gc, domid, dt, type);
+        if (rc) goto out;
+    }
+
+    if (dt->set_xenstore_config) {
+        rc = dt->set_xenstore_config(gc, domid, type);
+        if (rc) goto out;
+    }
+
+    aodev->dev = device;
+    aodev->action = LIBXL__DEVICE_ACTION_ADD;
+    libxl__wait_device_connection(egc, aodev);
+
+    rc = 0;
+
+out:
+    aodev->rc = rc;
+    if(rc) aodev->callback(egc, aodev);
+    return;
+}
+
 void* libxl__device_list(const struct libxl_device_type *dt,
                          libxl_ctx *ctx, uint32_t domid, int *num)
 {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c192559..3397655 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3484,13 +3484,17 @@ struct libxl_device_type {
     void (*add)(libxl__egc *, libxl__ao *, uint32_t, libxl_domain_config *,
                 libxl__multidev *);
     void *(*list)(libxl_ctx *, uint32_t, int *);
+    int (*set_default)(libxl__gc *, uint32_t, void *);
+    int (*to_device)(libxl__gc *, uint32_t, void *, libxl__device *);
     void (*init)(void *);
+    void (*copy)(libxl_ctx *, void *, void *);
     void (*dispose)(void *);
     int (*compare)(void *, void *);
     void (*merge)(libxl_ctx *, void *, void *);
     int (*dm_needed)(void *, unsigned);
     void (*update_config)(libxl__gc *, void *, void *);
     int (*from_xenstore)(libxl__gc *, const char *, uint32_t, void *);
+    int (*set_xenstore_config)(libxl__gc *, uint32_t, void *);
 };
 
 #define DEFINE_DEVICE_TYPE_STRUCT_X(name, sname, ...)                          \
@@ -3502,7 +3506,14 @@ struct libxl_device_type {
         .add           = libxl__add_ ## name ## s,                             \
         .list          = (void *(*)(libxl_ctx *, uint32_t, int *))             \
                          libxl_device_ ## sname ## _list,                      \
+        .set_default   = (int (*)(libxl__gc *, uint32_t, void *))              \
+                         libxl__device_ ## sname ## _setdefault,               \
+        .to_device     = (int (*)(libxl__gc *, uint32_t,                       \
+                                  void *, libxl__device *))                    \
+                         libxl__device_from_ ## name,                          \
         .init          = (void (*)(void *))libxl_device_ ## sname ## _init,    \
+        .copy          = (void (*)(libxl_ctx *, void *, void *))               \
+                         libxl_device_ ## sname ## _copy,                      \
         .dispose       = (void (*)(void *))libxl_device_ ## sname ## _dispose, \
         .compare       = (int (*)(void *, void *))                             \
                          libxl_device_ ## sname ## _compare,                   \
@@ -4354,6 +4365,10 @@ static inline bool libxl__acpi_defbool_val(const libxl_domain_build_info *b_info
     return libxl_defbool_val(b_info->acpi) &&
            libxl_defbool_val(b_info->u.hvm.acpi);
 }
+
+void libxl__device_add(libxl__egc *egc, uint32_t domid,
+                       const struct libxl_device_type *dt, void *type,
+                       libxl__ao_device *aodev);
 void* libxl__device_list(const struct libxl_device_type *dt,
                          libxl_ctx *ctx, uint32_t domid, int *num);
 void libxl__device_list_free(const struct libxl_device_type *dt,
diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index d8948d5..d1ec28f 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -1968,6 +1968,8 @@ void libxl_device_usbdev_list_free(libxl_device_usbdev *list, int nr)
 DEFINE_DEVICE_TYPE_STRUCT(usbctrl,
     .dm_needed = libxl_device_usbctrl_dm_needed
 );
+
+#define libxl__device_from_usbdev NULL
 DEFINE_DEVICE_TYPE_STRUCT(usbdev);
 
 /*
diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
index a628adc..c79bcda 100644
--- a/tools/libxl/libxl_vdispl.c
+++ b/tools/libxl/libxl_vdispl.c
@@ -14,6 +14,21 @@
 
 #include "libxl_internal.h"
 
+static int libxl__device_vdispl_setdefault(libxl__gc *gc, uint32_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, "vdispl");
+    }
+
+    return rc;
+}
+
 static int libxl__device_from_vdispl(libxl__gc *gc, uint32_t domid,
                                      libxl_device_vdispl *vdispl,
                                      libxl__device *device)
@@ -47,7 +62,7 @@ static void libxl__device_vdispl_add(libxl__egc *egc, uint32_t domid,
                                      libxl_device_vdispl *vdispl,
                                      libxl__ao_device *aodev)
 {
-
+    libxl__device_add(egc, domid, &libxl__vdispl_devtype, vdispl, aodev);
 }
 
 libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid,
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Wei Liu]

On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> 
> Add libxl__device_add functio.

function

> Almost all devices have similar libxl__device_xxxx_add function.
> This generic function implements same functionality but
> using the device handling framework. The device specific
> part this is setting xen store configuration. This part
> is moved to set_xenstore_config callback of the device framework.
> 

Right. I think this is a good idea in general.

I don't see exiting device ported to the new framework, why?

We really don't want two sets of code that does the same thing in libxl.
That's a recipe for bugs.

> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
[...]
>  /*
> diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
> index a628adc..c79bcda 100644
> --- a/tools/libxl/libxl_vdispl.c
> +++ b/tools/libxl/libxl_vdispl.c
> @@ -14,6 +14,21 @@
>  
>  #include "libxl_internal.h"
>  
> +static int libxl__device_vdispl_setdefault(libxl__gc *gc, uint32_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, "vdispl");
> +    }
> +

No need to have {}.

> +    return rc;
> +}
> +
>  static int libxl__device_from_vdispl(libxl__gc *gc, uint32_t domid,
>                                       libxl_device_vdispl *vdispl,
>                                       libxl__device *device)
> @@ -47,7 +62,7 @@ static void libxl__device_vdispl_add(libxl__egc *egc, uint32_t domid,
>                                       libxl_device_vdispl *vdispl,
>                                       libxl__ao_device *aodev)
>  {
> -
> +    libxl__device_add(egc, domid, &libxl__vdispl_devtype, vdispl, aodev);
>  }
>  
>  libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid,
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov]

On Thu, Jun 29, 2017 at 8:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Add libxl__device_add functio.
>
> function
>
>> Almost all devices have similar libxl__device_xxxx_add function.
>> This generic function implements same functionality but
>> using the device handling framework. The device specific
>> part this is setting xen store configuration. This part
>> is moved to set_xenstore_config callback of the device framework.
>>
>
> Right. I think this is a good idea in general.
>
> I don't see exiting device ported to the new framework, why?

Good question. I think it is a little dangerous and may introduce regression.
But definitely it should be done. I can do these changes but I don't have
visibility how to check each device.

>
> We really don't want two sets of code that does the same thing in libxl.
> That's a recipe for bugs.



>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> [...]
>>  /*
>> diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
>> index a628adc..c79bcda 100644
>> --- a/tools/libxl/libxl_vdispl.c
>> +++ b/tools/libxl/libxl_vdispl.c
>> @@ -14,6 +14,21 @@
>>
>>  #include "libxl_internal.h"
>>
>> +static int libxl__device_vdispl_setdefault(libxl__gc *gc, uint32_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, "vdispl");
>> +    }
>> +
>
> No need to have {}.
>
>> +    return rc;
>> +}
>> +
>>  static int libxl__device_from_vdispl(libxl__gc *gc, uint32_t domid,
>>                                       libxl_device_vdispl *vdispl,
>>                                       libxl__device *device)
>> @@ -47,7 +62,7 @@ static void libxl__device_vdispl_add(libxl__egc *egc, uint32_t domid,
>>                                       libxl_device_vdispl *vdispl,
>>                                       libxl__ao_device *aodev)
>>  {
>> -
>> +    libxl__device_add(egc, domid, &libxl__vdispl_devtype, vdispl, aodev);
>>  }
>>
>>  libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid,
>> --
>> 2.7.4
>>



-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Wei Liu]

On Fri, Jun 30, 2017 at 04:24:23PM +0300, Oleksandr Grytsov wrote:
> On Thu, Jun 29, 2017 at 8:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
> >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >>
> >> Add libxl__device_add functio.
> >
> > function
> >
> >> Almost all devices have similar libxl__device_xxxx_add function.
> >> This generic function implements same functionality but
> >> using the device handling framework. The device specific
> >> part this is setting xen store configuration. This part
> >> is moved to set_xenstore_config callback of the device framework.
> >>
> >
> > Right. I think this is a good idea in general.
> >
> > I don't see exiting device ported to the new framework, why?
> 
> Good question. I think it is a little dangerous and may introduce regression.
> But definitely it should be done. I can do these changes but I don't have
> visibility how to check each device.

Please just do it. We have a lot of time during development and RC
period for people to test your changes.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Wei Liu]

On Fri, Jun 30, 2017 at 03:16:38PM +0100, Wei Liu wrote:
> On Fri, Jun 30, 2017 at 04:24:23PM +0300, Oleksandr Grytsov wrote:
> > On Thu, Jun 29, 2017 at 8:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > > On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
> > >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> > >>
> > >> Add libxl__device_add functio.
> > >
> > > function
> > >
> > >> Almost all devices have similar libxl__device_xxxx_add function.
> > >> This generic function implements same functionality but
> > >> using the device handling framework. The device specific
> > >> part this is setting xen store configuration. This part
> > >> is moved to set_xenstore_config callback of the device framework.
> > >>
> > >
> > > Right. I think this is a good idea in general.
> > >
> > > I don't see exiting device ported to the new framework, why?
> > 
> > Good question. I think it is a little dangerous and may introduce regression.
> > But definitely it should be done. I can do these changes but I don't have
> > visibility how to check each device.
> 
> Please just do it. We have a lot of time during development and RC
> period for people to test your changes.

And I forget to say, please use one patch for one device type.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov]

On Fri, Jun 30, 2017 at 5:18 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Fri, Jun 30, 2017 at 03:16:38PM +0100, Wei Liu wrote:
>> On Fri, Jun 30, 2017 at 04:24:23PM +0300, Oleksandr Grytsov wrote:
>> > On Thu, Jun 29, 2017 at 8:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> > > On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
>> > >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> > >>
>> > >> Add libxl__device_add functio.
>> > >
>> > > function
>> > >
>> > >> Almost all devices have similar libxl__device_xxxx_add function.
>> > >> This generic function implements same functionality but
>> > >> using the device handling framework. The device specific
>> > >> part this is setting xen store configuration. This part
>> > >> is moved to set_xenstore_config callback of the device framework.
>> > >>
>> > >
>> > > Right. I think this is a good idea in general.
>> > >
>> > > I don't see exiting device ported to the new framework, why?
>> >
>> > Good question. I think it is a little dangerous and may introduce regression.
>> > But definitely it should be done. I can do these changes but I don't have
>> > visibility how to check each device.
>>
>> Please just do it. We have a lot of time during development and RC
>> period for people to test your changes.
>
> And I forget to say, please use one patch for one device type.

Should it be in this patch set or better to create new one for each device?

-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Wei Liu]

On Mon, Jul 03, 2017 at 03:53:08PM +0300, Oleksandr Grytsov wrote:
> On Fri, Jun 30, 2017 at 5:18 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Fri, Jun 30, 2017 at 03:16:38PM +0100, Wei Liu wrote:
> >> On Fri, Jun 30, 2017 at 04:24:23PM +0300, Oleksandr Grytsov wrote:
> >> > On Thu, Jun 29, 2017 at 8:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> >> > > On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
> >> > >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >> > >>
> >> > >> Add libxl__device_add functio.
> >> > >
> >> > > function
> >> > >
> >> > >> Almost all devices have similar libxl__device_xxxx_add function.
> >> > >> This generic function implements same functionality but
> >> > >> using the device handling framework. The device specific
> >> > >> part this is setting xen store configuration. This part
> >> > >> is moved to set_xenstore_config callback of the device framework.
> >> > >>
> >> > >
> >> > > Right. I think this is a good idea in general.
> >> > >
> >> > > I don't see exiting device ported to the new framework, why?
> >> >
> >> > Good question. I think it is a little dangerous and may introduce regression.
> >> > But definitely it should be done. I can do these changes but I don't have
> >> > visibility how to check each device.
> >>
> >> Please just do it. We have a lot of time during development and RC
> >> period for people to test your changes.
> >
> > And I forget to say, please use one patch for one device type.
> 
> Should it be in this patch set or better to create new one for each device?
> 

Those patches should be in this series.  One for each device for ease of
review please, and arrange it a way such that I can partially apply this
series.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov]

>> >> > > I don't see exiting device ported to the new framework, why?
>> >> >
>> >> > Good question. I think it is a little dangerous and may introduce regression.
>> >> > But definitely it should be done. I can do these changes but I don't have
>> >> > visibility how to check each device.
>> >>
>> >> Please just do it. We have a lot of time during development and RC
>> >> period for people to test your changes.
>> >
>> > And I forget to say, please use one patch for one device type.
>>
>> Should it be in this patch set or better to create new one for each device?
>>
>
> Those patches should be in this series.  One for each device for ease of
> review please, and arrange it a way such that I can partially apply this
> series.

Ok. I will wait for your feedback about this series and will prepare v4 with
fixes and changes for other devices.

Thanks.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov]

On Tue, Jul 4, 2017 at 12:41 PM, Oleksandr Grytsov <al1img@gmail.com> wrote:
>>> >> > > I don't see exiting device ported to the new framework, why?
>>> >> >
>>> >> > Good question. I think it is a little dangerous and may introduce regression.
>>> >> > But definitely it should be done. I can do these changes but I don't have
>>> >> > visibility how to check each device.
>>> >>
>>> >> Please just do it. We have a lot of time during development and RC
>>> >> period for people to test your changes.
>>> >
>>> > And I forget to say, please use one patch for one device type.
>>>
>>> Should it be in this patch set or better to create new one for each device?
>>>
>>
>> Those patches should be in this series.  One for each device for ease of
>> review please, and arrange it a way such that I can partially apply this
>> series.
>
> Ok. I will wait for your feedback about this series and will prepare v4 with
> fixes and changes for other devices.
>
> Thanks.

Hi Wei,

I've prepared new patch set. It is on my github [1].
I would appreciate if you review it before I send it.

The main changes are:
* libxl__device_add renamed to libxl__device_add_async and reworked
  to match the former design;
* libxl__device_add used for devices which don't require updating domain
  config but simple write to Xen Store (9pfs, vkb, vfb);
* following devices are changed to use the libxl__device_add:
  9pfs, vkb, vfb, nic, vtpm. Other device (console, pci, usb, disk) have
  very different adding pattern and require to unreasonable extend
  libxl__device_add_async and its parameters;
* disk device list changed to use libxl__device_list;
* small previous comments are applied.

[1] https://github.com/al1img/xen/tree/xl-vdispl-v4

-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Wei Liu]

On Wed, Jul 12, 2017 at 07:13:34PM +0300, Oleksandr Grytsov wrote:
> On Tue, Jul 4, 2017 at 12:41 PM, Oleksandr Grytsov <al1img@gmail.com> wrote:
> >>> >> > > I don't see exiting device ported to the new framework, why?
> >>> >> >
> >>> >> > Good question. I think it is a little dangerous and may introduce regression.
> >>> >> > But definitely it should be done. I can do these changes but I don't have
> >>> >> > visibility how to check each device.
> >>> >>
> >>> >> Please just do it. We have a lot of time during development and RC
> >>> >> period for people to test your changes.
> >>> >
> >>> > And I forget to say, please use one patch for one device type.
> >>>
> >>> Should it be in this patch set or better to create new one for each device?
> >>>
> >>
> >> Those patches should be in this series.  One for each device for ease of
> >> review please, and arrange it a way such that I can partially apply this
> >> series.
> >
> > Ok. I will wait for your feedback about this series and will prepare v4 with
> > fixes and changes for other devices.
> >
> > Thanks.
> 
> Hi Wei,
> 
> I've prepared new patch set. It is on my github [1].
> I would appreciate if you review it before I send it.
> 
> The main changes are:
> * libxl__device_add renamed to libxl__device_add_async and reworked
>   to match the former design;
> * libxl__device_add used for devices which don't require updating domain
>   config but simple write to Xen Store (9pfs, vkb, vfb);
> * following devices are changed to use the libxl__device_add:
>   9pfs, vkb, vfb, nic, vtpm. Other device (console, pci, usb, disk) have
>   very different adding pattern and require to unreasonable extend
>   libxl__device_add_async and its parameters;
> * disk device list changed to use libxl__device_list;
> * small previous comments are applied.
> 
> [1] https://github.com/al1img/xen/tree/xl-vdispl-v4
> 

Please just send the patches. It would be easier for me to make comments
via emails.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Wei Liu]

On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> 
> Add libxl__device_add functio.
> Almost all devices have similar libxl__device_xxxx_add function.
> This generic function implements same functionality but
> using the device handling framework. The device specific
> part this is setting xen store configuration. This part
> is moved to set_xenstore_config callback of the device framework.
> 
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
[...]
> +
> +void libxl__device_add(libxl__egc *egc, uint32_t domid,
> +                       const struct libxl_device_type *dt, void *type,
> +                       libxl__ao_device *aodev)
> +{
> +    STATE_AO_GC(aodev->ao);
> +    libxl__device *device;
> +    int rc;
> +
> +    rc = dt->set_default(gc, domid, type);
> +    if (rc) goto out;
> +
> +    GCNEW(device);
> +    rc = dt->to_device(gc, domid, type, device);
> +    if ( rc != 0 ) goto out;
> +
> +    rc = libxl__device_exists(gc, XBT_NULL, device);
> +    if (rc < 0) goto out;
> +    if (rc == 1) {              /* already exists in xenstore */
> +        LOGD(ERROR, domid, "device already exists in xenstore");
> +        aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */
> +        rc = ERROR_DEVICE_EXISTS;
> +        goto out;
> +    }
> +
> +    if (aodev->update_json) {
> +        rc = device_add_domain_config(gc, domid, dt, type);
> +        if (rc) goto out;
> +    }
> +
> +    if (dt->set_xenstore_config) {
> +        rc = dt->set_xenstore_config(gc, domid, type);
> +        if (rc) goto out;
> +    }
> +

This has changed the locking hierarchy we define in libxl_internal.h.
See libxl_internal.h:L2592.

Either you need to preserve the hierarchy or you need to prove the
correctness of the new approach. The former is probably easier.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov]

On Thu, Jul 6, 2017 at 6:51 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Add libxl__device_add functio.
>> Almost all devices have similar libxl__device_xxxx_add function.
>> This generic function implements same functionality but
>> using the device handling framework. The device specific
>> part this is setting xen store configuration. This part
>> is moved to set_xenstore_config callback of the device framework.
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> [...]
>> +
>> +void libxl__device_add(libxl__egc *egc, uint32_t domid,
>> +                       const struct libxl_device_type *dt, void *type,
>> +                       libxl__ao_device *aodev)
>> +{
>> +    STATE_AO_GC(aodev->ao);
>> +    libxl__device *device;
>> +    int rc;
>> +
>> +    rc = dt->set_default(gc, domid, type);
>> +    if (rc) goto out;
>> +
>> +    GCNEW(device);
>> +    rc = dt->to_device(gc, domid, type, device);
>> +    if ( rc != 0 ) goto out;
>> +
>> +    rc = libxl__device_exists(gc, XBT_NULL, device);
>> +    if (rc < 0) goto out;
>> +    if (rc == 1) {              /* already exists in xenstore */
>> +        LOGD(ERROR, domid, "device already exists in xenstore");
>> +        aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */
>> +        rc = ERROR_DEVICE_EXISTS;
>> +        goto out;
>> +    }
>> +
>> +    if (aodev->update_json) {
>> +        rc = device_add_domain_config(gc, domid, dt, type);
>> +        if (rc) goto out;
>> +    }
>> +
>> +    if (dt->set_xenstore_config) {
>> +        rc = dt->set_xenstore_config(gc, domid, type);
>> +        if (rc) goto out;
>> +    }
>> +
>
> This has changed the locking hierarchy we define in libxl_internal.h.
> See libxl_internal.h:L2592.
>
> Either you need to preserve the hierarchy or you need to prove the
> correctness of the new approach. The former is probably easier.

Actually my the first patch probably was done on the old codebase
which doesn't have locking in add function. So new approach is
definitely wrong and I will use former one.

-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov]

On Fri, Jul 7, 2017 at 12:49 PM, Oleksandr Grytsov <al1img@gmail.com> wrote:
> On Thu, Jul 6, 2017 at 6:51 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> On Tue, Jun 27, 2017 at 01:03:20PM +0300, Oleksandr Grytsov wrote:
>>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>>
>>> Add libxl__device_add functio.
>>> Almost all devices have similar libxl__device_xxxx_add function.
>>> This generic function implements same functionality but
>>> using the device handling framework. The device specific
>>> part this is setting xen store configuration. This part
>>> is moved to set_xenstore_config callback of the device framework.
>>>
>>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> [...]
>>> +
>>> +void libxl__device_add(libxl__egc *egc, uint32_t domid,
>>> +                       const struct libxl_device_type *dt, void *type,
>>> +                       libxl__ao_device *aodev)
>>> +{
>>> +    STATE_AO_GC(aodev->ao);
>>> +    libxl__device *device;
>>> +    int rc;
>>> +
>>> +    rc = dt->set_default(gc, domid, type);
>>> +    if (rc) goto out;
>>> +
>>> +    GCNEW(device);
>>> +    rc = dt->to_device(gc, domid, type, device);
>>> +    if ( rc != 0 ) goto out;
>>> +
>>> +    rc = libxl__device_exists(gc, XBT_NULL, device);
>>> +    if (rc < 0) goto out;
>>> +    if (rc == 1) {              /* already exists in xenstore */
>>> +        LOGD(ERROR, domid, "device already exists in xenstore");
>>> +        aodev->action = LIBXL__DEVICE_ACTION_ADD; /* for error message */
>>> +        rc = ERROR_DEVICE_EXISTS;
>>> +        goto out;
>>> +    }
>>> +
>>> +    if (aodev->update_json) {
>>> +        rc = device_add_domain_config(gc, domid, dt, type);
>>> +        if (rc) goto out;
>>> +    }
>>> +
>>> +    if (dt->set_xenstore_config) {
>>> +        rc = dt->set_xenstore_config(gc, domid, type);
>>> +        if (rc) goto out;
>>> +    }
>>> +
>>
>> This has changed the locking hierarchy we define in libxl_internal.h.
>> See libxl_internal.h:L2592.
>>
>> Either you need to preserve the hierarchy or you need to prove the
>> correctness of the new approach. The former is probably easier.
>
> Actually my the first patch probably was done on the old codebase
> which doesn't have locking in add function. So new approach is
> definitely wrong and I will use former one.

Please ignore my above comment. Actually it looks like my new approach
changes former behavior. I will rework this function to match former one.

Actually new approach

>
> --
> Best Regards,
> Oleksandr Grytsov.



-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Wei Liu]

On Fri, Jul 07, 2017 at 01:29:39PM +0300, Oleksandr Grytsov wrote:
 > Actually my the first patch probably was done on the old codebase
> > which doesn't have locking in add function. So new approach is
> > definitely wrong and I will use former one.
> 
> Please ignore my above comment. Actually it looks like my new approach
> changes former behavior. I will rework this function to match former one.
> 
> Actually new approach

Hit "Send" too soon?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov]

On Fri, Jul 7, 2017 at 1:32 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Fri, Jul 07, 2017 at 01:29:39PM +0300, Oleksandr Grytsov wrote:
>  > Actually my the first patch probably was done on the old codebase
>> > which doesn't have locking in add function. So new approach is
>> > definitely wrong and I will use former one.
>>
>> Please ignore my above comment. Actually it looks like my new approach
>> changes former behavior. I will rework this function to match former one.
>>
>> Actually new approach
>
> Hit "Send" too soon?

Just forgot to remove this line. So, I will rework this part.

-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Oleksandr Grytsov]

On Fri, Jul 7, 2017 at 1:56 PM, Oleksandr Grytsov <al1img@gmail.com> wrote:
> On Fri, Jul 7, 2017 at 1:32 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> On Fri, Jul 07, 2017 at 01:29:39PM +0300, Oleksandr Grytsov wrote:
>>  > Actually my the first patch probably was done on the old codebase
>>> > which doesn't have locking in add function. So new approach is
>>> > definitely wrong and I will use former one.
>>>
>>> Please ignore my above comment. Actually it looks like my new approach
>>> changes former behavior. I will rework this function to match former one.
>>>
>>> Actually new approach
>>
>> Hit "Send" too soon?
>
> Just forgot to remove this line. So, I will rework this part.
>

Few questions about former implementation (I address vtpm as reference
but questions are related to all devices):

1. Using of libxl_device_vtpm vtpm_saved variable. It is unclear why
we need this additional variable.
There is no any rollback or cancellation with this variable.
It is used to be added to the domain config but vtpm from input
parameter can be used for this reason as well.

2. Why libxl__update_config_vtpm(gc, &vtpm_saved, vtpm); is called if
just before we copied
vtpm_saved from vtpm?

    libxl_device_vtpm_init(&vtpm_saved);
    libxl_device_vtpm_copy(CTX, &vtpm_saved, vtpm);

I see that dev id is updated but it could be done before copy operation.

3. What is reason to call libxl__set_domain_configuration(gc, domid,
&d_config); in each
xen store transaction attempt?

Thanks.

> --
> Best Regards,
> Oleksandr Grytsov.



-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 04/11] libxl: add generic function to add device

[Wei Liu]

On Mon, Jul 10, 2017 at 03:41:28PM +0300, Oleksandr Grytsov wrote:
> On Fri, Jul 7, 2017 at 1:56 PM, Oleksandr Grytsov <al1img@gmail.com> wrote:
> > On Fri, Jul 7, 2017 at 1:32 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> >> On Fri, Jul 07, 2017 at 01:29:39PM +0300, Oleksandr Grytsov wrote:
> >>  > Actually my the first patch probably was done on the old codebase
> >>> > which doesn't have locking in add function. So new approach is
> >>> > definitely wrong and I will use former one.
> >>>
> >>> Please ignore my above comment. Actually it looks like my new approach
> >>> changes former behavior. I will rework this function to match former one.
> >>>
> >>> Actually new approach
> >>
> >> Hit "Send" too soon?
> >
> > Just forgot to remove this line. So, I will rework this part.
> >
> 
> Few questions about former implementation (I address vtpm as reference
> but questions are related to all devices):
> 
> 1. Using of libxl_device_vtpm vtpm_saved variable. It is unclear why
> we need this additional variable.
> There is no any rollback or cancellation with this variable.
> It is used to be added to the domain config but vtpm from input
> parameter can be used for this reason as well.

The vtpm_saved variable is a copy of the structure passed in by the caller.

We then pass vtpm to the _setdefault function which touches some of the
fields inside.

But then not all the fields changed by the _setdefault function need to
be written to our persistent domain configuration on disk. The fields we
care about are copied back  to vtpm_saved by libxl__update_config_vtpm.
Then we save vtpm_saved.

For your particular device, you should provide a similar update_config
function.

> 
> 2. Why libxl__update_config_vtpm(gc, &vtpm_saved, vtpm); is called if
> just before we copied
> vtpm_saved from vtpm?
> 
>     libxl_device_vtpm_init(&vtpm_saved);
>     libxl_device_vtpm_copy(CTX, &vtpm_saved, vtpm);
> 
> I see that dev id is updated but it could be done before copy operation.
> 

But for different device type there are different things to save. Vtpm
is a bit more of a simplistic one. See also other update_config
function.

The code structure is deliberated. It is a pattern applicable for all
devices --  the implementer can easily follow the pattern.

> 3. What is reason to call libxl__set_domain_configuration(gc, domid,
> &d_config); in each
> xen store transaction attempt?
> 

That would ensure the eventual consistency of the file written on disk
and the content in xenstore.

Keep in mind that there could be several threads competing with each
other to manipulate both the file on disk and xenstore.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 05/11] libxl: add vdispl setting xen store configuration

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Implemet set_xenstore_config callback  for vdispl
device driver.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/libxl_device.c   |   2 +-
 tools/libxl/libxl_internal.h |   1 +
 tools/libxl/libxl_vdispl.c   | 117 ++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 118 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index e202503..5d7e189 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -18,7 +18,7 @@
 
 #include "libxl_internal.h"
 
-static char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device)
+char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device)
 {
     char *dom_path = libxl__xs_get_dompath(gc, device->domid);
 
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3397655..c0dd17e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1209,6 +1209,7 @@ _hidden int libxl__device_exists(libxl__gc *gc, xs_transaction_t t,
                                  libxl__device *device);
 _hidden int libxl__device_generic_add(libxl__gc *gc, xs_transaction_t t,
         libxl__device *device, char **bents, char **fents, char **ro_fents);
+_hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device *device);
 _hidden char *libxl__device_libxl_path(libxl__gc *gc, libxl__device *device);
 _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
index c79bcda..4eceb71 100644
--- a/tools/libxl/libxl_vdispl.c
+++ b/tools/libxl/libxl_vdispl.c
@@ -65,6 +65,119 @@ static void libxl__device_vdispl_add(libxl__egc *egc, uint32_t domid,
     libxl__device_add(egc, domid, &libxl__vdispl_devtype, vdispl, aodev);
 }
 
+static int libxl__set_xenstore_connectors(libxl__gc *gc, xs_transaction_t t,
+                                          libxl__device *device,
+                                          libxl_device_vdispl *vdispl)
+{
+    struct xs_permissions perms[2];
+    char *frontend_path = NULL;
+    flexarray_t *connector;
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    int i;
+    int rc;
+
+    frontend_path = libxl__device_frontend_path(gc, device);
+
+    perms[0].id = device->domid;
+    perms[0].perms = XS_PERM_NONE;
+    perms[1].id = device->backend_domid;
+    perms[1].perms = XS_PERM_READ;
+
+    connector = flexarray_make(gc, 2, 1);
+    flexarray_append(connector, "resolution");
+    flexarray_append(connector, "");
+    flexarray_append(connector, "id");
+    flexarray_append(connector, "");
+
+    for (i = 0; i < vdispl->num_connectors; i++) {
+        char *connector_path = GCSPRINTF("%s/%d", frontend_path, i);
+
+        if (!xs_mkdir(ctx->xsh, t, connector_path)) {
+            rc = ERROR_FAIL; goto out;
+        }
+
+        if (!xs_set_permissions(ctx->xsh, t, connector_path, perms,
+                                ARRAY_SIZE(perms))) {
+            rc = ERROR_FAIL; goto out;
+        }
+
+        flexarray_set(connector, 1,
+                      GCSPRINTF("%dx%d", vdispl->connectors[i].width,
+                                 vdispl->connectors[i].height));
+        flexarray_set(connector, 3, vdispl->connectors[i].id);
+
+        rc = libxl__xs_writev(gc, t, connector_path,
+                              libxl__xs_kvs_of_flexarray(gc, connector));
+        if (rc) goto out;
+    }
+
+    rc = 0;
+
+out:
+    return rc;
+}
+
+static int libxl__set_xenstore_vdispl(libxl__gc *gc, uint32_t domid,
+                                      libxl_device_vdispl *vdispl)
+{
+    flexarray_t *front;
+    flexarray_t *back;
+
+    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));
+
+    libxl__device *device;
+    xs_transaction_t t = XBT_NULL;
+    int rc;
+
+    GCNEW(device);
+
+    rc = libxl__device_from_vdispl(gc, domid, vdispl, device);
+    if (rc) goto out;
+
+    for (;;) {
+        rc = libxl__xs_transaction_start(gc, &t);
+        if (rc) goto out;
+
+        rc = libxl__device_generic_add(gc, t, device,
+                                       libxl__xs_kvs_of_flexarray(gc, back),
+                                       libxl__xs_kvs_of_flexarray(gc, front),
+                                       NULL);
+        if (rc) goto out;
+
+        rc = libxl__set_xenstore_connectors(gc, t, device, vdispl);
+        if (rc) goto out;
+
+        rc = libxl__xs_transaction_commit(gc, &t);
+        if (!rc) break;
+        if (rc < 0) goto out;
+    }
+
+    rc = 0;
+
+out:
+    libxl__xs_transaction_abort(gc, &t);
+    return rc;
+}
+
 libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid,
                                               int *num)
 {
@@ -91,7 +204,9 @@ DEFINE_DEVICE_TYPE_STRUCT(vdispl,
     .update_config = (void (*)(libxl__gc *, void *, void *))
                      libxl__update_config_vdispl,
     .from_xenstore = (int (*)(libxl__gc *, const char *, uint32_t, void *))
-                     libxl__from_xenstore_vdispl
+                     libxl__from_xenstore_vdispl,
+    .set_xenstore_config = (int (*)(libxl__gc *, uint32_t, void *))
+                           libxl__set_xenstore_vdispl
 );
 
 /*
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 06/11] libxl: implement vdispl get info function

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Add implementation of libxl_device_vdispl_getinfo.
This function returns extended information about
selected vdispl device.

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/libxl_vdispl.c | 115 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
index 4eceb71..1a6c8b7 100644
--- a/tools/libxl/libxl_vdispl.c
+++ b/tools/libxl/libxl_vdispl.c
@@ -189,11 +189,124 @@ void libxl_device_vdispl_list_free(libxl_device_vdispl* list, int num)
     libxl__device_list_free(&libxl__vdispl_devtype, list, num);
 }
 
+static int libxl__device_vdispl_getconnectors(libxl_ctx *ctx,
+                                              const char *path,
+                                              libxl_vdisplinfo *info)
+{
+    GC_INIT(ctx);
+    char *connector = NULL;
+    char *connector_path = NULL;
+    int i, rc;
+
+    GCNEW_ARRAY(connector_path, 128);
+
+    info->num_connectors = 0;
+
+    rc = snprintf(connector_path, 128, "%s/%d", path, info->num_connectors);
+    if (rc < 0) goto out;
+
+    while((connector = xs_read(ctx->xsh, XBT_NULL, connector_path, NULL))
+          != NULL) {
+        free(connector);
+
+        rc = snprintf(connector_path, 128, "%s/%d",
+                      path, ++info->num_connectors);
+        if (rc < 0) goto out;
+    }
+
+    info->connectors = libxl__calloc(NOGC, info->num_connectors,
+                                     sizeof(*info->connectors));
+
+    for (i = 0; i < info->num_connectors; i++) {
+        char *value;
+
+        snprintf(connector_path, 128, "%s/%d/id", path, i);
+        info->connectors[i].id = xs_read(ctx->xsh, XBT_NULL,
+                                         connector_path, NULL);
+        if (info->connectors[i].id == NULL) { rc = ERROR_FAIL; goto out; }
+
+        snprintf(connector_path, 128, "%s/%d/resolution", path, i);
+        value = xs_read(ctx->xsh, XBT_NULL, connector_path, NULL);
+        if (value == NULL) { rc = ERROR_FAIL; goto out; }
+
+        rc = sscanf(value, "%ux%u", &info->connectors[i].width,
+                   &info->connectors[i].height);
+        free(value);
+        if (rc != 2) {
+            rc = ERROR_FAIL; goto out;
+        }
+
+        snprintf(connector_path, 128, "%s/%d/req-ring-ref", path, i);
+        value = xs_read(ctx->xsh, XBT_NULL, connector_path, NULL);
+        info->connectors[i].req_rref = value ? strtoul(value, NULL, 10) : -1;
+        free(value);
+
+        snprintf(connector_path, 128, "%s/%d/req-event-channel", path, i);
+        value = xs_read(ctx->xsh, XBT_NULL, connector_path, NULL);
+        info->connectors[i].req_evtch = value ? strtoul(value, NULL, 10) : -1;
+        free(value);
+
+        snprintf(connector_path, 128, "%s/%d/evt-ring-ref", path, i);
+        value = xs_read(ctx->xsh, XBT_NULL, connector_path, NULL);
+        info->connectors[i].evt_rref = value ? strtoul(value, NULL, 10) : -1;
+        free(value);
+
+        snprintf(connector_path, 128, "%s/%d/evt-event-channel", path, i);
+        value = xs_read(ctx->xsh, XBT_NULL, connector_path, NULL);
+        info->connectors[i].evt_evtch = value ? strtoul(value, NULL, 10) : -1;
+        free(value);
+    }
+
+    rc = 0;
+
+out:
+    return rc;
+}
+
 int libxl_device_vdispl_getinfo(libxl_ctx *ctx, uint32_t domid,
                                 libxl_device_vdispl *vdispl,
                                 libxl_vdisplinfo *info)
 {
-     return 0;
+    GC_INIT(ctx);
+    char *libxl_path, *dompath, *devpath;
+    char *val;
+    int rc;
+
+    libxl_vdisplinfo_init(info);
+    dompath = libxl__xs_get_dompath(gc, domid);
+    info->devid = vdispl->devid;
+
+    devpath = GCSPRINTF("%s/device/vdispl/%d", dompath, info->devid);
+    libxl_path = GCSPRINTF("%s/device/vdispl/%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;
+
+    val = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/be_alloc", devpath));
+    info->be_alloc = val ? strtoul(val, NULL, 10) : 0;
+
+    rc = libxl__device_vdispl_getconnectors(ctx, devpath, info);
+    if (rc) goto out;
+
+    rc = 0;
+
+out:
+     GC_FREE;
+     return rc;
 }
 
 LIBXL_DEFINE_DEVICE_ADD(vdispl)
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 07/11] libxl: implement device_from_vdispl and update_config_vdispl

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/libxl_vdispl.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
index 1a6c8b7..ab90cb1 100644
--- a/tools/libxl/libxl_vdispl.c
+++ b/tools/libxl/libxl_vdispl.c
@@ -33,6 +33,13 @@ 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;
 }
 
@@ -49,7 +56,8 @@ static void libxl__update_config_vdispl(libxl__gc *gc,
                                         libxl_device_vdispl *dst,
                                         libxl_device_vdispl *src)
 {
-
+    dst->devid = src->devid;
+    dst->be_alloc = src->be_alloc;
 }
 
 static int libxl_device_vdispl_compare(libxl_device_vdispl *d1,
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 08/11] libxl: add libxl__vdispl_devtype to device_type_tbl

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/libxl_create.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 4e5ba29..ba3943f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1442,6 +1442,7 @@ const struct libxl_device_type *device_type_tbl[] = {
     &libxl__usbdev_devtype,
     &libxl__pcidev_devtype,
     &libxl__dtdev_devtype,
+    &libxl__vdispl_devtype,
     NULL
 };
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 09/11] libxl: add libxl_devid_to_device_vdispl interface function

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/libxl/libxl_utils.h  |  4 ++++
 tools/libxl/libxl_vdispl.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

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
index ab90cb1..b86465d 100644
--- a/tools/libxl/libxl_vdispl.c
+++ b/tools/libxl/libxl_vdispl.c
@@ -317,6 +317,37 @@ out:
      return rc;
 }
 
+int libxl_devid_to_device_vdispl(libxl_ctx *ctx, uint32_t domid,
+                                 int devid, libxl_device_vdispl *vdispl)
+{
+    libxl_device_vdispl *vdispls = NULL;
+    int n, i;
+    int rc;
+
+    libxl_device_vdispl_init(vdispl);
+
+    vdispls = libxl_device_vdispl_list(ctx, domid, &n);
+
+    if (!vdispls) { rc = ERROR_NOTFOUND; goto out; }
+
+    for (i = 0; i < n; ++i) {
+        if (devid == vdispls[i].devid) {
+            libxl_device_vdispl_copy(ctx, vdispl, &vdispls[i]);
+            rc = 0;
+            goto out;
+        }
+    }
+
+    rc = ERROR_NOTFOUND;
+
+out:
+
+    if (vdispls) {
+        libxl_device_vdispl_list_free(vdispls, n);
+    }
+    return rc;
+}
+
 LIBXL_DEFINE_DEVICE_ADD(vdispl)
 static LIBXL_DEFINE_DEVICES_ADD(vdispl)
 LIBXL_DEFINE_DEVICE_REMOVE(vdispl)
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v3 10/11] xl: add PV display device commands

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Add commands: vdispl-attach, vdispl-list, vdispl-detach
and domain config vdispl parser

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 tools/xl/Makefile      |   1 +
 tools/xl/xl.h          |   3 +
 tools/xl/xl_cmdtable.c |  19 ++++++
 tools/xl/xl_parse.c    |  77 ++++++++++++++++++++++-
 tools/xl/xl_parse.h    |   2 +-
 tools/xl/xl_vdispl.c   | 166 +++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 266 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 aa95b77..d361241 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -166,6 +166,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 30eb93c..d21261f 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -372,6 +372,25 @@ 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>] [connectors='<Connectors>']",
+      "    BackAlloc - set to 1 to allow backend allocated display buffers\n"
+      "    Connectors - list of connector's description in ID:WxH format,\n"
+      "                 where: ID - unique connector ID, W - connector width,\n"
+      "                 H - connector height: connectors='id0:800x600;id1:1024x768'\n"
+    },
+    { "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 856a304..8b80d58 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -803,6 +803,53 @@ int parse_usbdev_config(libxl_device_usbdev *usbdev, char *token)
     return 0;
 }
 
+int parse_vdispl_config(libxl_device_vdispl *vdispl, char *token)
+{
+    char *oparg;
+    libxl_string_list connectors = NULL;
+    int i;
+    int rc;
+
+    if (MATCH_OPTION("backend", token, 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 if (MATCH_OPTION("connectors", token, oparg)) {
+        split_string_into_string_list(oparg, ";", &connectors);
+
+        vdispl->num_connectors = libxl_string_list_length(&connectors);
+        vdispl->connectors = calloc(vdispl->num_connectors,
+                                    sizeof(*vdispl->connectors));
+
+        for(i = 0; i < vdispl->num_connectors; i++)
+        {
+            char *resolution;
+
+            rc = split_string_into_pair(connectors[i], ":",
+                                        &vdispl->connectors[i].id,
+                                        &resolution);
+
+            rc= sscanf(resolution, "%ux%u", &vdispl->connectors[i].width,
+                       &vdispl->connectors[i].height);
+            if (rc != 2) {
+                fprintf(stderr, "Can't parse connector resolution\n");
+                goto out;
+            }
+        }
+    } else {
+        fprintf(stderr, "Unknown string \"%s\" in vdispl spec\n", token);
+        rc = 1; goto out;
+    }
+
+    rc = 0;
+
+out:
+    libxl_string_list_dispose(&connectors);
+    return rc;
+}
+
 void parse_config_data(const char *config_source,
                        const char *config_data,
                        int config_len,
@@ -812,7 +859,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, *p9devs;
+                   *usbctrls, *usbdevs, *p9devs, *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;
@@ -1462,6 +1509,34 @@ 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)) {
+                    free(buf2);
+                    exit(1);
+                }
+                p = strtok (NULL, ",");
+            }
+            free(buf2);
+            if (vdispl->num_connectors == 0) {
+                fprintf(stderr, "At least one connector should be specified.\n");
+                exit(1);
+            }
+        }
+    }
+
     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..104512b
--- /dev/null
+++ b/tools/xl/xl_vdispl.c
@@ -0,0 +1,166 @@
+/*
+ * 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;
+    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) {
+        rc = parse_vdispl_config(&vdispl, *argv);
+        if (rc) goto out;
+    }
+
+    if (vdispl.num_connectors == 0) {
+        fprintf(stderr, "At least one connector should be specified.\n");
+        rc = ERROR_FAIL; 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; goto out;
+    }
+
+    rc = 0;
+
+out:
+    libxl_device_vdispl_dispose(&vdispl);
+    return rc;
+}
+
+int main_vdispllist(int argc, char **argv)
+{
+   int opt;
+   int i, j, n;
+   libxl_device_vdispl *vdispls;
+   libxl_vdisplinfo vdisplinfo;
+
+   SWITCH_FOREACH_OPT(opt, "", NULL, "vdispl-list", 1) {
+       /* No options */
+   }
+
+   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) {
+               printf("DevId: %d, BE: %d, handle: %d, state: %d, "
+                      "be-alloc: %d, BE-path: %s, FE-path: %s\n",
+                       vdisplinfo.devid, vdisplinfo.backend_id,
+                       vdisplinfo.frontend_id,
+                       vdisplinfo.state, vdisplinfo.be_alloc,
+                       vdisplinfo.backend, vdisplinfo.frontend);
+
+               for (j = 0; j < vdisplinfo.num_connectors; j++) {
+                   printf("\tConnector: %d, id: %s, width: %d, height: %d, "
+                          "req-rref: %d, req-evtch: %d, "
+                          "evt-rref: %d, evt-evtch: %d\n",
+                          j, vdisplinfo.connectors[j].id,
+                          vdisplinfo.connectors[j].width,
+                          vdisplinfo.connectors[j].height,
+                          vdisplinfo.connectors[j].req_rref,
+                          vdisplinfo.connectors[j].req_evtch,
+                          vdisplinfo.connectors[j].evt_rref,
+                          vdisplinfo.connectors[j].evt_evtch);
+               }
+           }
+           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

[PATCH v3 11/11] docs: add PV display driver information

[Oleksandr Grytsov]

From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>

Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
 docs/man/xl.cfg.pod.5.in | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
 docs/man/xl.pod.1.in     | 42 +++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 13167ff..f181f34 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1096,6 +1096,60 @@ FIFO-based event channel ABI support up to 131,071 event channels.
 Other guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit
 x86).
 
+=item B<vdispl=[ "VDISPL_SPEC_STRING", "VDISPL_SPEC_STRING", ...]>
+
+Specifies the virtual display devices to be provided to the guest.
+
+Each B<VDISPL_SPEC_STRING> is a comma-separated list of C<KEY=VALUE>
+settings, from the following list:
+
+=over 4
+
+=item C<backend=DOMAIN>
+
+Specifies the backend domain name or id. If not specified Domain-0 is used.
+
+=item C<devId=device-id>
+
+Specified virtual display device ID. If not specified will be assigned
+automatically.
+
+=item C<beAlloc=BOOLEAN>
+
+Indicates if backend can be a buffer provider/allocator for this domain. See
+display protocol for details.
+
+=item C<connectors=CONNECTORS>
+
+Specifies virtual connectors for the device in following format
+<id>:<W>x<H>;<id>:<W>x<H>... where:
+
+=over 4
+
+=item C<id>
+
+String connector ID. Space, comma symbols are not allowed.
+
+=item C<W>
+
+Connector width in pixels.
+
+=item C<H>
+
+Connector height in pixels.
+
+=back
+
+B<EXAMPLE>
+
+=over 4
+
+connectors=id0:1920x1080;id1:800x600;id2:640x480
+
+=back
+
+=back
+
 =back
 
 =head2 Paravirtualised (PV) Guest Specific Options
diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index 78bf884..a6093c3 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -1451,6 +1451,48 @@ List virtual trusted platform modules for a domain.
 
 =back
 
+=head2 VDISPL DEVICES
+
+=over 4
+
+=item B<vdispl-attach> I<domain-id> I<vdispl-device>
+
+Creates a new vdispl device in the domain specified by I<domain-id>.
+I<vdispl-device> describes the device to attach, using the same format as the
+B<vdispl> string in the domain config file. See L<xl.cfg> for
+more information.
+
+B<NOTES>
+
+=over 4
+
+As in I<vdispl-device> string semicolon is used then put quotes or escaping
+when using from the shell.
+
+B<EXAMPLE>
+
+=over 4
+
+xl vdispl-attach DomU connectors='id0:1920x1080;id1:800x600;id2:640x480'
+
+or
+
+xl vdispl-attach DomU connectors=id0:1920x1080\;id1:800x600\;id2:640x480
+
+=back
+
+=back
+
+=item B<vdispl-detach> I<domain-id> I<dev-id>
+
+Removes the vdispl device specified by I<dev-id> from the domain specified by I<domain-id>.
+
+=item B<vdispl-list> I<domain-id>
+
+List virtual displays for a domain.
+
+=back
+
 =head1 PCI PASS-THROUGH
 
 =over 4
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 11/11] docs: add PV display driver information

[Wei Liu]

On Tue, Jun 27, 2017 at 01:03:27PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> 
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> ---
>  docs/man/xl.cfg.pod.5.in | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
>  docs/man/xl.pod.1.in     | 42 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 96 insertions(+)
> 
> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> index 13167ff..f181f34 100644
> --- a/docs/man/xl.cfg.pod.5.in
> +++ b/docs/man/xl.cfg.pod.5.in
> @@ -1096,6 +1096,60 @@ FIFO-based event channel ABI support up to 131,071 event channels.
>  Other guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit
>  x86).
>  
> +=item B<vdispl=[ "VDISPL_SPEC_STRING", "VDISPL_SPEC_STRING", ...]>
> +
> +Specifies the virtual display devices to be provided to the guest.
> +
> +Each B<VDISPL_SPEC_STRING> is a comma-separated list of C<KEY=VALUE>
> +settings, from the following list:
> +
> +=over 4
> +
> +=item C<backend=DOMAIN>
> +
> +Specifies the backend domain name or id. If not specified Domain-0 is used.
> +
> +=item C<devId=device-id>
> +

Can we avoid camel case?

> +Specified virtual display device ID. If not specified will be assigned
> +automatically.
> +
> +=item C<beAlloc=BOOLEAN>
> +

Just "allocator"?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 11/11] docs: add PV display driver information

[Oleksandr Grytsov]

Same as for idl file. I prefer to have name similar to one in the protocol.
To avoid camel case it can be renamed to be-alloc.


On Thu, Jun 29, 2017 at 8:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jun 27, 2017 at 01:03:27PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> ---
>>  docs/man/xl.cfg.pod.5.in | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  docs/man/xl.pod.1.in     | 42 +++++++++++++++++++++++++++++++++++++
>>  2 files changed, 96 insertions(+)
>>
>> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
>> index 13167ff..f181f34 100644
>> --- a/docs/man/xl.cfg.pod.5.in
>> +++ b/docs/man/xl.cfg.pod.5.in
>> @@ -1096,6 +1096,60 @@ FIFO-based event channel ABI support up to 131,071 event channels.
>>  Other guests are limited to 4095 (64-bit x86 and ARM) or 1023 (32-bit
>>  x86).
>>
>> +=item B<vdispl=[ "VDISPL_SPEC_STRING", "VDISPL_SPEC_STRING", ...]>
>> +
>> +Specifies the virtual display devices to be provided to the guest.
>> +
>> +Each B<VDISPL_SPEC_STRING> is a comma-separated list of C<KEY=VALUE>
>> +settings, from the following list:
>> +
>> +=over 4
>> +
>> +=item C<backend=DOMAIN>
>> +
>> +Specifies the backend domain name or id. If not specified Domain-0 is used.
>> +
>> +=item C<devId=device-id>
>> +
>
> Can we avoid camel case?
>
>> +Specified virtual display device ID. If not specified will be assigned
>> +automatically.
>> +
>> +=item C<beAlloc=BOOLEAN>
>> +
>
> Just "allocator"?



-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 00/11] libxl: add PV display device driver interface

[Wei Liu]

Can you push this series to a git tree? It would be easier for me to
check your changes to the device framework. The device specific handlers
mostly look OK to me.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 00/11] libxl: add PV display device driver interface

[Oleksandr Grytsov]

Sure. It is in my private github repository [1].

[1] https://github.com/al1img/xen/tree/xl-vdispl-v3

On Thu, Jun 29, 2017 at 8:38 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> Can you push this series to a git tree? It would be easier for me to
> check your changes to the device framework. The device specific handlers
> mostly look OK to me.



-- 
Best Regards,
Oleksandr Grytsov.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v3 00/11] libxl: add PV display device driver interface

[Wei Liu]

On Fri, Jun 30, 2017 at 01:45:06PM +0300, Oleksandr Grytsov wrote:
> Sure. It is in my private github repository [1].
> 
> [1] https://github.com/al1img/xen/tree/xl-vdispl-v3
> 

Thanks. Will check the code some time next week.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel