* [PATCH v4 01/13] libxl: add generic function to add device
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 11:47 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 02/13] libxl: add generic functions to get and free device list Oleksandr Grytsov
` (13 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Add libxl__device_add to simple write XenStore device conifg
and libxl__device_add_async to update domain configuration
and write XenStore device config asynchroniously.
Almost all devices have similar libxl__device_xxxx_add function.
This generic functions implement same functionality but
using the device handling framework. Th device specific
part such as setting xen store configurationis moved
to set_xenstore_config callback of the device framework.
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl_create.c | 3 +
tools/libxl/libxl_device.c | 198 +++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_disk.c | 2 +
tools/libxl/libxl_internal.h | 36 ++++++++
tools/libxl/libxl_nic.c | 2 +
tools/libxl/libxl_pci.c | 2 +
tools/libxl/libxl_usb.c | 6 ++
tools/libxl/libxl_vtpm.c | 2 +
8 files changed, 251 insertions(+)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bffbc45..b2163cd 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1430,6 +1430,9 @@ 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
+#define libxl__device_dtdev_update_devid 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 00356af..07165f0 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1793,6 +1793,204 @@ out:
return AO_CREATE_FAIL(rc);
}
+static void device_add_domain_config(libxl__gc *gc,
+ libxl_domain_config *d_config,
+ const struct libxl_device_type *dt,
+ void *type)
+{
+ int *num_dev;
+ int i;
+ void *item = NULL;
+
+ 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);
+}
+
+void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
+ const struct libxl_device_type *dt, void *type,
+ libxl__ao_device *aodev)
+{
+ STATE_AO_GC(aodev->ao);
+ flexarray_t *back;
+ flexarray_t *front, *ro_front;
+ libxl__device *device;
+ xs_transaction_t t = XBT_NULL;
+ libxl_domain_config d_config;
+ void *type_saved;
+ libxl__domain_userdata_lock *lock = NULL;
+ int rc;
+
+ libxl_domain_config_init(&d_config);
+
+ type_saved = libxl__malloc(gc, dt->dev_elem_size);
+
+ dt->init(type_saved);
+ dt->copy(CTX, type_saved, type);
+
+ if (dt->set_default) {
+ rc = dt->set_default(gc, domid, type, aodev->update_json);
+ if (rc) goto out;
+ }
+
+ if (dt->update_devid) {
+ rc = dt->update_devid(gc, domid, type);
+ if (rc) goto out;
+ }
+
+ if (dt->update_config)
+ dt->update_config(gc, type_saved, type);
+
+ GCNEW(device);
+ rc = dt->to_device(gc, domid, type, device);
+ if (rc) goto out;
+
+ if (aodev->update_json) {
+
+ lock = libxl__lock_domain_userdata(gc, domid);
+ if (!lock) {
+ rc = ERROR_LOCK_FAIL;
+ goto out;
+ }
+
+ rc = libxl__get_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+
+ device_add_domain_config(gc, &d_config, dt, type_saved);
+
+ rc = libxl__dm_check_start(gc, &d_config, domid);
+ if (rc) goto out;
+ }
+
+ back = flexarray_make(gc, 16, 1);
+ front = flexarray_make(gc, 16, 1);
+ ro_front = flexarray_make(gc, 16, 1);
+
+ flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid));
+ flexarray_append_pair(back, "online", "1");
+ flexarray_append_pair(back, "state",
+ GCSPRINTF("%d", XenbusStateInitialising));
+
+ flexarray_append_pair(front, "backend-id",
+ GCSPRINTF("%d", device->backend_domid));
+ flexarray_append_pair(front, "state",
+ GCSPRINTF("%d", XenbusStateInitialising));
+
+ if (dt->set_xenstore_config)
+ dt->set_xenstore_config(gc, domid, type, back, front, ro_front);
+
+ for (;;) {
+ rc = libxl__xs_transaction_start(gc, &t);
+ if (rc) goto out;
+
+ rc = libxl__device_exists(gc, t, device);
+ if (rc < 0) goto out;
+ if (rc == 1) { /* 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 = libxl__set_domain_configuration(gc, domid, &d_config);
+ if (rc) goto out;
+ }
+
+ libxl__device_generic_add(gc, t, device,
+ libxl__xs_kvs_of_flexarray(gc, back),
+ libxl__xs_kvs_of_flexarray(gc, front),
+ libxl__xs_kvs_of_flexarray(gc, ro_front));
+
+ rc = libxl__xs_transaction_commit(gc, &t);
+ if (!rc) break;
+ if (rc < 0) goto out;
+ }
+
+ aodev->dev = device;
+ aodev->action = LIBXL__DEVICE_ACTION_ADD;
+ libxl__wait_device_connection(egc, aodev);
+
+ rc = 0;
+
+out:
+ libxl__xs_transaction_abort(gc, &t);
+ if (lock) libxl__unlock_domain_userdata(lock);
+ dt->dispose(type_saved);
+ libxl_domain_config_dispose(&d_config);
+ aodev->rc = rc;
+ if (rc) aodev->callback(egc, aodev);
+ return;
+}
+
+int libxl__device_add(libxl__gc *gc, uint32_t domid,
+ const struct libxl_device_type *dt, void *type)
+{
+ flexarray_t *back;
+ flexarray_t *front, *ro_front;
+ libxl__device *device;
+ int rc;
+
+ if (dt->set_default) {
+ rc = dt->set_default(gc, domid, type, false);
+ if (rc) goto out;
+ }
+
+ if (dt->update_devid) {
+ rc = dt->update_devid(gc, domid, type);
+ if (rc) goto out;
+ }
+
+ GCNEW(device);
+ rc = dt->to_device(gc, domid, type, device);
+ if (rc) goto out;
+
+ back = flexarray_make(gc, 16, 1);
+ front = flexarray_make(gc, 16, 1);
+ ro_front = flexarray_make(gc, 16, 1);
+
+ flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid));
+ flexarray_append_pair(back, "online", "1");
+ flexarray_append_pair(back, "state",
+ GCSPRINTF("%d", XenbusStateInitialising));
+ flexarray_append_pair(front, "backend-id",
+ libxl__sprintf(gc, "%d", device->backend_domid));
+ flexarray_append_pair(front, "state",
+ GCSPRINTF("%d", XenbusStateInitialising));
+
+ if (dt->set_xenstore_config)
+ dt->set_xenstore_config(gc, domid, type, back, front, ro_front);
+
+ rc = libxl__device_generic_add(gc, XBT_NULL, device,
+ libxl__xs_kvs_of_flexarray(gc, back),
+ libxl__xs_kvs_of_flexarray(gc, front),
+ libxl__xs_kvs_of_flexarray(gc, ro_front));
+ if (rc) goto out;
+
+ rc = 0;
+
+out:
+ return rc;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index 63de75c..f2f3635 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -1244,6 +1244,8 @@ static int libxl_device_disk_dm_needed(void *e, unsigned domid)
elem->backend_domid == domid;
}
+#define libxl__device_disk_update_devid NULL
+
DEFINE_DEVICE_TYPE_STRUCT(disk,
.merge = libxl_device_disk_merge,
.dm_needed = libxl_device_disk_dm_needed,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index afe6652..075dfe3 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3467,6 +3467,18 @@ _hidden void libxl__bootloader_run(libxl__egc*, libxl__bootloader_state *st);
return AO_INPROGRESS; \
}
+#define LIBXL_DEFINE_UPDATE_DEVID(type, name) \
+ int libxl__device_##type##_update_devid(libxl__gc *gc, \
+ uint32_t domid, \
+ libxl_device_##type *type) \
+ { \
+ if (type->devid == -1) \
+ type->devid = libxl__device_nextid(gc, domid, name); \
+ if (type->devid < 0) \
+ return ERROR_FAIL; \
+ return 0; \
+ }
+
#define LIBXL_DEFINE_DEVICE_REMOVE(type) \
LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, generic, remove, 0) \
LIBXL_DEFINE_DEVICE_REMOVE_EXT(type, generic, destroy, 1)
@@ -3484,11 +3496,18 @@ 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 *, bool);
+ 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 (*update_devid)(libxl__gc *, uint32_t, void *);
+ int (*set_xenstore_config)(libxl__gc *, uint32_t, void *, flexarray_t *,
+ flexarray_t *, flexarray_t *);
};
#define DEFINE_DEVICE_TYPE_STRUCT_X(name, sname, ...) \
@@ -3500,9 +3519,19 @@ 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 *, bool hotplug))\
+ 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, \
+ .update_devid = (int (*)(libxl__gc *, uint32_t, void *)) \
+ libxl__device_ ## sname ## _update_devid, \
__VA_ARGS__ \
}
@@ -4350,6 +4379,13 @@ 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_async(libxl__egc *egc, uint32_t domid,
+ const struct libxl_device_type *dt, void *type,
+ libxl__ao_device *aodev);
+int libxl__device_add(libxl__gc *gc, uint32_t domid,
+ const struct libxl_device_type *dt, void *type);
+
#endif
/*
diff --git a/tools/libxl/libxl_nic.c b/tools/libxl/libxl_nic.c
index 4b6e8c0..dd07a6c 100644
--- a/tools/libxl/libxl_nic.c
+++ b/tools/libxl/libxl_nic.c
@@ -669,6 +669,8 @@ LIBXL_DEFINE_DEVICE_ADD(nic)
LIBXL_DEFINE_DEVICES_ADD(nic)
LIBXL_DEFINE_DEVICE_REMOVE(nic)
+#define libxl__device_nic_update_devid NULL
+
DEFINE_DEVICE_TYPE_STRUCT(nic,
.update_config = libxl_device_nic_update_config
);
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index b14df16..c3f1e5c 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1708,6 +1708,8 @@ static int libxl_device_pci_compare(libxl_device_pci *d1,
return COMPARE_PCI(d1, d2);
}
+#define libxl__device_pci_update_devid NULL
+
DEFINE_DEVICE_TYPE_STRUCT_X(pcidev, pci);
/*
diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index d8948d5..07fb202 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -1965,9 +1965,15 @@ void libxl_device_usbdev_list_free(libxl_device_usbdev *list, int nr)
free(list);
}
+#define libxl__device_usbctrl_update_devid NULL
+
DEFINE_DEVICE_TYPE_STRUCT(usbctrl,
.dm_needed = libxl_device_usbctrl_dm_needed
);
+
+#define libxl__device_from_usbdev NULL
+#define libxl__device_usbdev_update_devid NULL
+
DEFINE_DEVICE_TYPE_STRUCT(usbdev);
/*
diff --git a/tools/libxl/libxl_vtpm.c b/tools/libxl/libxl_vtpm.c
index 9ee8cce..cbd5461 100644
--- a/tools/libxl/libxl_vtpm.c
+++ b/tools/libxl/libxl_vtpm.c
@@ -364,6 +364,8 @@ LIBXL_DEFINE_DEVICE_ADD(vtpm)
static LIBXL_DEFINE_DEVICES_ADD(vtpm)
LIBXL_DEFINE_DEVICE_REMOVE(vtpm)
+#define libxl__device_vtpm_update_devid NULL
+
DEFINE_DEVICE_TYPE_STRUCT(vtpm,
.update_config = libxl_device_vtpm_update_config
);
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v4 01/13] libxl: add generic function to add device
2017-07-18 14:25 ` [PATCH v4 01/13] libxl: add generic function to add device Oleksandr Grytsov
@ 2017-09-05 11:47 ` Wei Liu
2017-09-05 16:44 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-09-05 11:47 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, wei.liu2, ian.jackson, Oleksandr Grytsov
On Tue, Jul 18, 2017 at 05:25:18PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Add libxl__device_add to simple write XenStore device conifg
> and libxl__device_add_async to update domain configuration
> and write XenStore device config asynchroniously.
> Almost all devices have similar libxl__device_xxxx_add function.
> This generic functions implement same functionality but
> using the device handling framework. Th device specific
> part such as setting xen store configurationis moved
> to set_xenstore_config callback of the device framework.
>
The two add functions look correct.
Some comments below.
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> ---
> tools/libxl/libxl_create.c | 3 +
> tools/libxl/libxl_device.c | 198 +++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_disk.c | 2 +
> tools/libxl/libxl_internal.h | 36 ++++++++
> tools/libxl/libxl_nic.c | 2 +
> tools/libxl/libxl_pci.c | 2 +
> tools/libxl/libxl_usb.c | 6 ++
> tools/libxl/libxl_vtpm.c | 2 +
> 8 files changed, 251 insertions(+)
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index bffbc45..b2163cd 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -1430,6 +1430,9 @@ 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
> +#define libxl__device_dtdev_update_devid 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 00356af..07165f0 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -1793,6 +1793,204 @@ out:
> return AO_CREATE_FAIL(rc);
> }
>
> +static void device_add_domain_config(libxl__gc *gc,
> + libxl_domain_config *d_config,
> + const struct libxl_device_type *dt,
> + void *type)
> +{
> + int *num_dev;
> + int i;
unsigned int please.
> + void *item = NULL;
> +
> + 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);
Space after devs.
> + *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);
> +}
> +
> +void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
> + const struct libxl_device_type *dt, void *type,
> + libxl__ao_device *aodev)
> +{
> + STATE_AO_GC(aodev->ao);
> + flexarray_t *back;
> + flexarray_t *front, *ro_front;
> + libxl__device *device;
> + xs_transaction_t t = XBT_NULL;
> + libxl_domain_config d_config;
> + void *type_saved;
> + libxl__domain_userdata_lock *lock = NULL;
> + int rc;
> +
> + libxl_domain_config_init(&d_config);
> +
> + type_saved = libxl__malloc(gc, dt->dev_elem_size);
> +
> + dt->init(type_saved);
> + dt->copy(CTX, type_saved, type);
> +
> + if (dt->set_default) {
> + rc = dt->set_default(gc, domid, type, aodev->update_json);
> + if (rc) goto out;
> + }
> +
> + if (dt->update_devid) {
> + rc = dt->update_devid(gc, domid, type);
> + if (rc) goto out;
> + }
> +
> + if (dt->update_config)
> + dt->update_config(gc, type_saved, type);
> +
> + GCNEW(device);
> + rc = dt->to_device(gc, domid, type, device);
> + if (rc) goto out;
> +
> + if (aodev->update_json) {
> +
Extraneous empty line here.
> + lock = libxl__lock_domain_userdata(gc, domid);
> + if (!lock) {
> + rc = ERROR_LOCK_FAIL;
> + goto out;
> + }
> +
> + rc = libxl__get_domain_configuration(gc, domid, &d_config);
> + if (rc) goto out;
> +
> + device_add_domain_config(gc, &d_config, dt, type_saved);
> +
> + rc = libxl__dm_check_start(gc, &d_config, domid);
> + if (rc) goto out;
> + }
> +
> + back = flexarray_make(gc, 16, 1);
> + front = flexarray_make(gc, 16, 1);
> + ro_front = flexarray_make(gc, 16, 1);
> +
> + flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid));
> + flexarray_append_pair(back, "online", "1");
> + flexarray_append_pair(back, "state",
> + GCSPRINTF("%d", XenbusStateInitialising));
> +
> + flexarray_append_pair(front, "backend-id",
> + GCSPRINTF("%d", device->backend_domid));
> + flexarray_append_pair(front, "state",
> + GCSPRINTF("%d", XenbusStateInitialising));
> +
> + if (dt->set_xenstore_config)
> + dt->set_xenstore_config(gc, domid, type, back, front, ro_front);
> +
> + for (;;) {
> + rc = libxl__xs_transaction_start(gc, &t);
> + if (rc) goto out;
> +
> + rc = libxl__device_exists(gc, t, device);
> + if (rc < 0) goto out;
> + if (rc == 1) { /* 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 = libxl__set_domain_configuration(gc, domid, &d_config);
> + if (rc) goto out;
> + }
> +
> + libxl__device_generic_add(gc, t, device,
> + libxl__xs_kvs_of_flexarray(gc, back),
> + libxl__xs_kvs_of_flexarray(gc, front),
> + libxl__xs_kvs_of_flexarray(gc, ro_front));
> +
> + rc = libxl__xs_transaction_commit(gc, &t);
> + if (!rc) break;
> + if (rc < 0) goto out;
> + }
> +
> + aodev->dev = device;
> + aodev->action = LIBXL__DEVICE_ACTION_ADD;
> + libxl__wait_device_connection(egc, aodev);
> +
> + rc = 0;
> +
> +out:
> + libxl__xs_transaction_abort(gc, &t);
> + if (lock) libxl__unlock_domain_userdata(lock);
> + dt->dispose(type_saved);
> + libxl_domain_config_dispose(&d_config);
> + aodev->rc = rc;
> + if (rc) aodev->callback(egc, aodev);
> + return;
> +}
> +
> +int libxl__device_add(libxl__gc *gc, uint32_t domid,
> + const struct libxl_device_type *dt, void *type)
> +{
> + flexarray_t *back;
> + flexarray_t *front, *ro_front;
> + libxl__device *device;
> + int rc;
> +
> + if (dt->set_default) {
> + rc = dt->set_default(gc, domid, type, false);
> + if (rc) goto out;
> + }
> +
> + if (dt->update_devid) {
> + rc = dt->update_devid(gc, domid, type);
> + if (rc) goto out;
> + }
> +
> + GCNEW(device);
> + rc = dt->to_device(gc, domid, type, device);
> + if (rc) goto out;
> +
> + back = flexarray_make(gc, 16, 1);
> + front = flexarray_make(gc, 16, 1);
> + ro_front = flexarray_make(gc, 16, 1);
> +
> + flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid));
> + flexarray_append_pair(back, "online", "1");
> + flexarray_append_pair(back, "state",
> + GCSPRINTF("%d", XenbusStateInitialising));
> + flexarray_append_pair(front, "backend-id",
> + libxl__sprintf(gc, "%d", device->backend_domid));
> + flexarray_append_pair(front, "state",
> + GCSPRINTF("%d", XenbusStateInitialising));
> +
> + if (dt->set_xenstore_config)
> + dt->set_xenstore_config(gc, domid, type, back, front, ro_front);
> +
> + rc = libxl__device_generic_add(gc, XBT_NULL, device,
> + libxl__xs_kvs_of_flexarray(gc, back),
> + libxl__xs_kvs_of_flexarray(gc, front),
> + libxl__xs_kvs_of_flexarray(gc, ro_front));
> + if (rc) goto out;
> +
> + rc = 0;
> +
> +out:
> + return rc;
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
> index 63de75c..f2f3635 100644
> --- a/tools/libxl/libxl_disk.c
> +++ b/tools/libxl/libxl_disk.c
> @@ -1244,6 +1244,8 @@ static int libxl_device_disk_dm_needed(void *e, unsigned domid)
> elem->backend_domid == domid;
> }
>
> +#define libxl__device_disk_update_devid NULL
> +
Is this correct for disk (and other device types as well)?
Since you've defined LIBXL_DEFINE_UPDATE_DEVID, you should be able to
use that immediately?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 01/13] libxl: add generic function to add device
2017-09-05 11:47 ` Wei Liu
@ 2017-09-05 16:44 ` Oleksandr Grytsov
2017-09-06 9:36 ` Wei Liu
0 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-09-05 16:44 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Tue, Sep 5, 2017 at 2:47 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jul 18, 2017 at 05:25:18PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Add libxl__device_add to simple write XenStore device conifg
>> and libxl__device_add_async to update domain configuration
>> and write XenStore device config asynchroniously.
>> Almost all devices have similar libxl__device_xxxx_add function.
>> This generic functions implement same functionality but
>> using the device handling framework. Th device specific
>> part such as setting xen store configurationis moved
>> to set_xenstore_config callback of the device framework.
>>
>
> The two add functions look correct.
>
> Some comments below.
>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> ---
>> tools/libxl/libxl_create.c | 3 +
>> tools/libxl/libxl_device.c | 198 +++++++++++++++++++++++++++++++++++++++++++
>> tools/libxl/libxl_disk.c | 2 +
>> tools/libxl/libxl_internal.h | 36 ++++++++
>> tools/libxl/libxl_nic.c | 2 +
>> tools/libxl/libxl_pci.c | 2 +
>> tools/libxl/libxl_usb.c | 6 ++
>> tools/libxl/libxl_vtpm.c | 2 +
>> 8 files changed, 251 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index bffbc45..b2163cd 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -1430,6 +1430,9 @@ 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
>> +#define libxl__device_dtdev_update_devid 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 00356af..07165f0 100644
>> --- a/tools/libxl/libxl_device.c
>> +++ b/tools/libxl/libxl_device.c
>> @@ -1793,6 +1793,204 @@ out:
>> return AO_CREATE_FAIL(rc);
>> }
>>
>> +static void device_add_domain_config(libxl__gc *gc,
>> + libxl_domain_config *d_config,
>> + const struct libxl_device_type *dt,
>> + void *type)
>> +{
>> + int *num_dev;
>> + int i;
>
> unsigned int please.
For "i" counter only or for num_dev as well?
For "i" is ok but num_dev better to keep int.
>
>> + void *item = NULL;
>> +
>> + 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);
>
> Space after devs.
>
>> + *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);
>> +}
>> +
>> +void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
>> + const struct libxl_device_type *dt, void *type,
>> + libxl__ao_device *aodev)
>> +{
>> + STATE_AO_GC(aodev->ao);
>> + flexarray_t *back;
>> + flexarray_t *front, *ro_front;
>> + libxl__device *device;
>> + xs_transaction_t t = XBT_NULL;
>> + libxl_domain_config d_config;
>> + void *type_saved;
>> + libxl__domain_userdata_lock *lock = NULL;
>> + int rc;
>> +
>> + libxl_domain_config_init(&d_config);
>> +
>> + type_saved = libxl__malloc(gc, dt->dev_elem_size);
>> +
>> + dt->init(type_saved);
>> + dt->copy(CTX, type_saved, type);
>> +
>> + if (dt->set_default) {
>> + rc = dt->set_default(gc, domid, type, aodev->update_json);
>> + if (rc) goto out;
>> + }
>> +
>> + if (dt->update_devid) {
>> + rc = dt->update_devid(gc, domid, type);
>> + if (rc) goto out;
>> + }
>> +
>> + if (dt->update_config)
>> + dt->update_config(gc, type_saved, type);
>> +
>> + GCNEW(device);
>> + rc = dt->to_device(gc, domid, type, device);
>> + if (rc) goto out;
>> +
>> + if (aodev->update_json) {
>> +
>
> Extraneous empty line here.
>
>> + lock = libxl__lock_domain_userdata(gc, domid);
>> + if (!lock) {
>> + rc = ERROR_LOCK_FAIL;
>> + goto out;
>> + }
>> +
>> + rc = libxl__get_domain_configuration(gc, domid, &d_config);
>> + if (rc) goto out;
>> +
>> + device_add_domain_config(gc, &d_config, dt, type_saved);
>> +
>> + rc = libxl__dm_check_start(gc, &d_config, domid);
>> + if (rc) goto out;
>> + }
>> +
>> + back = flexarray_make(gc, 16, 1);
>> + front = flexarray_make(gc, 16, 1);
>> + ro_front = flexarray_make(gc, 16, 1);
>> +
>> + flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid));
>> + flexarray_append_pair(back, "online", "1");
>> + flexarray_append_pair(back, "state",
>> + GCSPRINTF("%d", XenbusStateInitialising));
>> +
>> + flexarray_append_pair(front, "backend-id",
>> + GCSPRINTF("%d", device->backend_domid));
>> + flexarray_append_pair(front, "state",
>> + GCSPRINTF("%d", XenbusStateInitialising));
>> +
>> + if (dt->set_xenstore_config)
>> + dt->set_xenstore_config(gc, domid, type, back, front, ro_front);
>> +
>> + for (;;) {
>> + rc = libxl__xs_transaction_start(gc, &t);
>> + if (rc) goto out;
>> +
>> + rc = libxl__device_exists(gc, t, device);
>> + if (rc < 0) goto out;
>> + if (rc == 1) { /* 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 = libxl__set_domain_configuration(gc, domid, &d_config);
>> + if (rc) goto out;
>> + }
>> +
>> + libxl__device_generic_add(gc, t, device,
>> + libxl__xs_kvs_of_flexarray(gc, back),
>> + libxl__xs_kvs_of_flexarray(gc, front),
>> + libxl__xs_kvs_of_flexarray(gc, ro_front));
>> +
>> + rc = libxl__xs_transaction_commit(gc, &t);
>> + if (!rc) break;
>> + if (rc < 0) goto out;
>> + }
>> +
>> + aodev->dev = device;
>> + aodev->action = LIBXL__DEVICE_ACTION_ADD;
>> + libxl__wait_device_connection(egc, aodev);
>> +
>> + rc = 0;
>> +
>> +out:
>> + libxl__xs_transaction_abort(gc, &t);
>> + if (lock) libxl__unlock_domain_userdata(lock);
>> + dt->dispose(type_saved);
>> + libxl_domain_config_dispose(&d_config);
>> + aodev->rc = rc;
>> + if (rc) aodev->callback(egc, aodev);
>> + return;
>> +}
>> +
>> +int libxl__device_add(libxl__gc *gc, uint32_t domid,
>> + const struct libxl_device_type *dt, void *type)
>> +{
>> + flexarray_t *back;
>> + flexarray_t *front, *ro_front;
>> + libxl__device *device;
>> + int rc;
>> +
>> + if (dt->set_default) {
>> + rc = dt->set_default(gc, domid, type, false);
>> + if (rc) goto out;
>> + }
>> +
>> + if (dt->update_devid) {
>> + rc = dt->update_devid(gc, domid, type);
>> + if (rc) goto out;
>> + }
>> +
>> + GCNEW(device);
>> + rc = dt->to_device(gc, domid, type, device);
>> + if (rc) goto out;
>> +
>> + back = flexarray_make(gc, 16, 1);
>> + front = flexarray_make(gc, 16, 1);
>> + ro_front = flexarray_make(gc, 16, 1);
>> +
>> + flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid));
>> + flexarray_append_pair(back, "online", "1");
>> + flexarray_append_pair(back, "state",
>> + GCSPRINTF("%d", XenbusStateInitialising));
>> + flexarray_append_pair(front, "backend-id",
>> + libxl__sprintf(gc, "%d", device->backend_domid));
>> + flexarray_append_pair(front, "state",
>> + GCSPRINTF("%d", XenbusStateInitialising));
>> +
>> + if (dt->set_xenstore_config)
>> + dt->set_xenstore_config(gc, domid, type, back, front, ro_front);
>> +
>> + rc = libxl__device_generic_add(gc, XBT_NULL, device,
>> + libxl__xs_kvs_of_flexarray(gc, back),
>> + libxl__xs_kvs_of_flexarray(gc, front),
>> + libxl__xs_kvs_of_flexarray(gc, ro_front));
>> + if (rc) goto out;
>> +
>> + rc = 0;
>> +
>> +out:
>> + return rc;
>> +}
>> +
>> /*
>> * Local variables:
>> * mode: C
>> diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
>> index 63de75c..f2f3635 100644
>> --- a/tools/libxl/libxl_disk.c
>> +++ b/tools/libxl/libxl_disk.c
>> @@ -1244,6 +1244,8 @@ static int libxl_device_disk_dm_needed(void *e, unsigned domid)
>> elem->backend_domid == domid;
>> }
>>
>> +#define libxl__device_disk_update_devid NULL
>> +
>
> Is this correct for disk (and other device types as well)?
What exactly is correct? libxl__device_disk_update_devid NULL or
libxl__device_add_async function?
>
> Since you've defined LIBXL_DEFINE_UPDATE_DEVID, you should be able to
> use that immediately?
Actually disk doesn't call update dev ID. So assigning it to NULL I
guess is ok here.
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 01/13] libxl: add generic function to add device
2017-09-05 16:44 ` Oleksandr Grytsov
@ 2017-09-06 9:36 ` Wei Liu
2017-09-06 12:08 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-09-06 9:36 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, Wei Liu, Ian Jackson, Oleksandr Grytsov
On Tue, Sep 05, 2017 at 07:44:34PM +0300, Oleksandr Grytsov wrote:
> On Tue, Sep 5, 2017 at 2:47 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Tue, Jul 18, 2017 at 05:25:18PM +0300, Oleksandr Grytsov wrote:
> >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >>
> >> Add libxl__device_add to simple write XenStore device conifg
> >> and libxl__device_add_async to update domain configuration
> >> and write XenStore device config asynchroniously.
> >> Almost all devices have similar libxl__device_xxxx_add function.
> >> This generic functions implement same functionality but
> >> using the device handling framework. Th device specific
> >> part such as setting xen store configurationis moved
> >> to set_xenstore_config callback of the device framework.
> >>
> >
> > The two add functions look correct.
> >
> > Some comments below.
> >
> >> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >> ---
> >> tools/libxl/libxl_create.c | 3 +
> >> tools/libxl/libxl_device.c | 198 +++++++++++++++++++++++++++++++++++++++++++
> >> tools/libxl/libxl_disk.c | 2 +
> >> tools/libxl/libxl_internal.h | 36 ++++++++
> >> tools/libxl/libxl_nic.c | 2 +
> >> tools/libxl/libxl_pci.c | 2 +
> >> tools/libxl/libxl_usb.c | 6 ++
> >> tools/libxl/libxl_vtpm.c | 2 +
> >> 8 files changed, 251 insertions(+)
> >>
> >> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> >> index bffbc45..b2163cd 100644
> >> --- a/tools/libxl/libxl_create.c
> >> +++ b/tools/libxl/libxl_create.c
> >> @@ -1430,6 +1430,9 @@ 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
> >> +#define libxl__device_dtdev_update_devid 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 00356af..07165f0 100644
> >> --- a/tools/libxl/libxl_device.c
> >> +++ b/tools/libxl/libxl_device.c
> >> @@ -1793,6 +1793,204 @@ out:
> >> return AO_CREATE_FAIL(rc);
> >> }
> >>
> >> +static void device_add_domain_config(libxl__gc *gc,
> >> + libxl_domain_config *d_config,
> >> + const struct libxl_device_type *dt,
> >> + void *type)
> >> +{
> >> + int *num_dev;
> >> + int i;
> >
> > unsigned int please.
>
> For "i" counter only or for num_dev as well?
> For "i" is ok but num_dev better to keep int.
>
For i only.
> >> * mode: C
> >> diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
> >> index 63de75c..f2f3635 100644
> >> --- a/tools/libxl/libxl_disk.c
> >> +++ b/tools/libxl/libxl_disk.c
> >> @@ -1244,6 +1244,8 @@ static int libxl_device_disk_dm_needed(void *e, unsigned domid)
> >> elem->backend_domid == domid;
> >> }
> >>
> >> +#define libxl__device_disk_update_devid NULL
> >> +
> >
> > Is this correct for disk (and other device types as well)?
>
> What exactly is correct? libxl__device_disk_update_devid NULL or
> libxl__device_add_async function?
>
Defining all the update_devid functions to be NULL. They should be
defined with the macros now, right?
I notice in later patches they are changed, so I'm not too fuss either
way. If you want to keep them to be defined as NULL please say so in
commit message.
> >
> > Since you've defined LIBXL_DEFINE_UPDATE_DEVID, you should be able to
> > use that immediately?
>
> Actually disk doesn't call update dev ID. So assigning it to NULL I
> guess is ok here.
>
Yes. I think so. Please consider other device types then.
> --
> Best Regards,
> Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 01/13] libxl: add generic function to add device
2017-09-06 9:36 ` Wei Liu
@ 2017-09-06 12:08 ` Oleksandr Grytsov
0 siblings, 0 replies; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-09-06 12:08 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Wed, Sep 6, 2017 at 12:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Sep 05, 2017 at 07:44:34PM +0300, Oleksandr Grytsov wrote:
>> On Tue, Sep 5, 2017 at 2:47 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> > On Tue, Jul 18, 2017 at 05:25:18PM +0300, Oleksandr Grytsov wrote:
>> >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> >>
>> >> Add libxl__device_add to simple write XenStore device conifg
>> >> and libxl__device_add_async to update domain configuration
>> >> and write XenStore device config asynchroniously.
>> >> Almost all devices have similar libxl__device_xxxx_add function.
>> >> This generic functions implement same functionality but
>> >> using the device handling framework. Th device specific
>> >> part such as setting xen store configurationis moved
>> >> to set_xenstore_config callback of the device framework.
>> >>
>> >
>> > The two add functions look correct.
>> >
>> > Some comments below.
>> >
>> >> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> >> ---
>> >> tools/libxl/libxl_create.c | 3 +
>> >> tools/libxl/libxl_device.c | 198 +++++++++++++++++++++++++++++++++++++++++++
>> >> tools/libxl/libxl_disk.c | 2 +
>> >> tools/libxl/libxl_internal.h | 36 ++++++++
>> >> tools/libxl/libxl_nic.c | 2 +
>> >> tools/libxl/libxl_pci.c | 2 +
>> >> tools/libxl/libxl_usb.c | 6 ++
>> >> tools/libxl/libxl_vtpm.c | 2 +
>> >> 8 files changed, 251 insertions(+)
>> >>
>> >> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> >> index bffbc45..b2163cd 100644
>> >> --- a/tools/libxl/libxl_create.c
>> >> +++ b/tools/libxl/libxl_create.c
>> >> @@ -1430,6 +1430,9 @@ 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
>> >> +#define libxl__device_dtdev_update_devid 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 00356af..07165f0 100644
>> >> --- a/tools/libxl/libxl_device.c
>> >> +++ b/tools/libxl/libxl_device.c
>> >> @@ -1793,6 +1793,204 @@ out:
>> >> return AO_CREATE_FAIL(rc);
>> >> }
>> >>
>> >> +static void device_add_domain_config(libxl__gc *gc,
>> >> + libxl_domain_config *d_config,
>> >> + const struct libxl_device_type *dt,
>> >> + void *type)
>> >> +{
>> >> + int *num_dev;
>> >> + int i;
>> >
>> > unsigned int please.
>>
>> For "i" counter only or for num_dev as well?
>> For "i" is ok but num_dev better to keep int.
>>
>
> For i only.
>
>> >> * mode: C
>> >> diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
>> >> index 63de75c..f2f3635 100644
>> >> --- a/tools/libxl/libxl_disk.c
>> >> +++ b/tools/libxl/libxl_disk.c
>> >> @@ -1244,6 +1244,8 @@ static int libxl_device_disk_dm_needed(void *e, unsigned domid)
>> >> elem->backend_domid == domid;
>> >> }
>> >>
>> >> +#define libxl__device_disk_update_devid NULL
>> >> +
>> >
>> > Is this correct for disk (and other device types as well)?
>>
>> What exactly is correct? libxl__device_disk_update_devid NULL or
>> libxl__device_add_async function?
>>
>
> Defining all the update_devid functions to be NULL. They should be
> defined with the macros now, right?
>
> I notice in later patches they are changed, so I'm not too fuss either
> way. If you want to keep them to be defined as NULL please say so in
> commit message.
>
>> >
>> > Since you've defined LIBXL_DEFINE_UPDATE_DEVID, you should be able to
>> > use that immediately?
>>
>> Actually disk doesn't call update dev ID. So assigning it to NULL I
>> guess is ok here.
>>
>
> Yes. I think so. Please consider other device types then.
Ok, I will use the macro in every device which need update devid in this patch.
Also I will call this function in add device function.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v4 02/13] libxl: add generic functions to get and free device list
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
2017-07-18 14:25 ` [PATCH v4 01/13] libxl: add generic function to add device Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 11:51 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 03/13] libxl: add vdispl device Oleksandr Grytsov
` (12 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Add libxl__device_list and libxl__device_list_free
functions to handle device list using the device
framework.
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl_device.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_internal.h | 8 ++++++
2 files changed, 74 insertions(+)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 07165f0..f1d4848 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1991,6 +1991,72 @@ out:
return rc;
}
+void *libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
+ uint32_t domid, const char* name, int *num)
+{
+ void *r = NULL;
+ void *list = NULL;
+ void *item = NULL;
+ char *libxl_path;
+ char **dir = NULL;
+ unsigned int ndirs = 0;
+ int rc;
+
+ *num = 0;
+
+ libxl_path = GCSPRINTF("%s/device/%s",
+ libxl__xs_libxl_path(gc, domid), name);
+
+ dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
+
+ if (dir && ndirs) {
+ list = libxl__malloc(NOGC, dt->dev_elem_size * ndirs);
+ void *end = (uint8_t *)list + ndirs * dt->dev_elem_size;
+ item = list;
+
+ while (item < end) {
+ dt->init(item);
+
+ if (dt->from_xenstore) {
+ char* device_libxl_path = GCSPRINTF("%s/%s", libxl_path, *dir);
+ rc = dt->from_xenstore(gc, device_libxl_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) {
+ dt->dispose(item);
+ item = (uint8_t*)item - dt->dev_elem_size;
+ }
+ free(list);
+ }
+
+ 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 075dfe3..271ac89 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3506,6 +3506,7 @@ struct libxl_device_type {
int (*dm_needed)(void *, unsigned);
void (*update_config)(libxl__gc *, void *, void *);
int (*update_devid)(libxl__gc *, uint32_t, void *);
+ int (*from_xenstore)(libxl__gc *, const char *, libxl_devid, void *);
int (*set_xenstore_config)(libxl__gc *, uint32_t, void *, flexarray_t *,
flexarray_t *, flexarray_t *);
};
@@ -4386,6 +4387,13 @@ void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
int libxl__device_add(libxl__gc *gc, uint32_t domid,
const struct libxl_device_type *dt, void *type);
+/* Caller is responsible for freeing the memory by calling
+ * libxl__device_list_free
+ */
+void* libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
+ uint32_t domid, const char* name, int *num);
+void libxl__device_list_free(const struct libxl_device_type *dt,
+ void *list, int num);
#endif
/*
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v4 02/13] libxl: add generic functions to get and free device list
2017-07-18 14:25 ` [PATCH v4 02/13] libxl: add generic functions to get and free device list Oleksandr Grytsov
@ 2017-09-05 11:51 ` Wei Liu
2017-09-06 12:31 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-09-05 11:51 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, wei.liu2, ian.jackson, Oleksandr Grytsov
On Tue, Jul 18, 2017 at 05:25:19PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Add libxl__device_list and libxl__device_list_free
> functions to handle device list using the device
> framework.
>
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> ---
> tools/libxl/libxl_device.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_internal.h | 8 ++++++
> 2 files changed, 74 insertions(+)
>
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 07165f0..f1d4848 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -1991,6 +1991,72 @@ out:
> return rc;
> }
>
> +void *libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
> + uint32_t domid, const char* name, int *num)
> +{
> + void *r = NULL;
> + void *list = NULL;
> + void *item = NULL;
> + char *libxl_path;
> + char **dir = NULL;
> + unsigned int ndirs = 0;
> + int rc;
> +
> + *num = 0;
> +
> + libxl_path = GCSPRINTF("%s/device/%s",
> + libxl__xs_libxl_path(gc, domid), name);
> +
> + dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
> +
> + if (dir && ndirs) {
> + list = libxl__malloc(NOGC, dt->dev_elem_size * ndirs);
> + void *end = (uint8_t *)list + ndirs * dt->dev_elem_size;
> + item = list;
> +
> + while (item < end) {
> + dt->init(item);
> +
> + if (dt->from_xenstore) {
> + char* device_libxl_path = GCSPRINTF("%s/%s", libxl_path, *dir);
> + rc = dt->from_xenstore(gc, device_libxl_path, atoi(*dir), item);
> + if (rc) goto out;
> + }
> +
> + item = (uint8_t*)item + dt->dev_elem_size;
Space before *.
> + ++dir;
> + }
> + }
> +
> + *num = ndirs;
> + r = list;
> + list = NULL;
> +
> +out:
> +
> + if (list) {
> + *num = 0;
> + while(item >= list) {
Space after while, but ...
> + dt->dispose(item);
> + item = (uint8_t*)item - dt->dev_elem_size;
> + }
> + free(list);
You should be able to use libxl__device_list_free here.
> + }
> +
> + 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 075dfe3..271ac89 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3506,6 +3506,7 @@ struct libxl_device_type {
> int (*dm_needed)(void *, unsigned);
> void (*update_config)(libxl__gc *, void *, void *);
> int (*update_devid)(libxl__gc *, uint32_t, void *);
> + int (*from_xenstore)(libxl__gc *, const char *, libxl_devid, void *);
> int (*set_xenstore_config)(libxl__gc *, uint32_t, void *, flexarray_t *,
> flexarray_t *, flexarray_t *);
> };
> @@ -4386,6 +4387,13 @@ void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
> int libxl__device_add(libxl__gc *gc, uint32_t domid,
> const struct libxl_device_type *dt, void *type);
>
> +/* Caller is responsible for freeing the memory by calling
> + * libxl__device_list_free
> + */
> +void* libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
> + uint32_t domid, const char* name, int *num);
> +void libxl__device_list_free(const struct libxl_device_type *dt,
> + void *list, int num);
> #endif
>
> /*
> --
> 2.7.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 02/13] libxl: add generic functions to get and free device list
2017-09-05 11:51 ` Wei Liu
@ 2017-09-06 12:31 ` Oleksandr Grytsov
0 siblings, 0 replies; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-09-06 12:31 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Tue, Sep 5, 2017 at 2:51 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jul 18, 2017 at 05:25:19PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Add libxl__device_list and libxl__device_list_free
>> functions to handle device list using the device
>> framework.
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> ---
>> tools/libxl/libxl_device.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
>> tools/libxl/libxl_internal.h | 8 ++++++
>> 2 files changed, 74 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
>> index 07165f0..f1d4848 100644
>> --- a/tools/libxl/libxl_device.c
>> +++ b/tools/libxl/libxl_device.c
>> @@ -1991,6 +1991,72 @@ out:
>> return rc;
>> }
>>
>> +void *libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
>> + uint32_t domid, const char* name, int *num)
>> +{
>> + void *r = NULL;
>> + void *list = NULL;
>> + void *item = NULL;
>> + char *libxl_path;
>> + char **dir = NULL;
>> + unsigned int ndirs = 0;
>> + int rc;
>> +
>> + *num = 0;
>> +
>> + libxl_path = GCSPRINTF("%s/device/%s",
>> + libxl__xs_libxl_path(gc, domid), name);
>> +
>> + dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
>> +
>> + if (dir && ndirs) {
>> + list = libxl__malloc(NOGC, dt->dev_elem_size * ndirs);
>> + void *end = (uint8_t *)list + ndirs * dt->dev_elem_size;
>> + item = list;
>> +
>> + while (item < end) {
>> + dt->init(item);
>> +
>> + if (dt->from_xenstore) {
>> + char* device_libxl_path = GCSPRINTF("%s/%s", libxl_path, *dir);
>> + rc = dt->from_xenstore(gc, device_libxl_path, atoi(*dir), item);
>> + if (rc) goto out;
>> + }
>> +
>> + item = (uint8_t*)item + dt->dev_elem_size;
>
> Space before *.
>
>> + ++dir;
>> + }
>> + }
>> +
>> + *num = ndirs;
>> + r = list;
>> + list = NULL;
>> +
>> +out:
>> +
>> + if (list) {
>> + *num = 0;
>> + while(item >= list) {
>
> Space after while, but ...
>
>> + dt->dispose(item);
>> + item = (uint8_t*)item - dt->dev_elem_size;
>> + }
>> + free(list);
>
> You should be able to use libxl__device_list_free here.
Good catch. I will use list_free here. This requires slight changes in
above loop:
i need to count initialized elements in order to pass it to list_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 075dfe3..271ac89 100644
>> --- a/tools/libxl/libxl_internal.h
>> +++ b/tools/libxl/libxl_internal.h
>> @@ -3506,6 +3506,7 @@ struct libxl_device_type {
>> int (*dm_needed)(void *, unsigned);
>> void (*update_config)(libxl__gc *, void *, void *);
>> int (*update_devid)(libxl__gc *, uint32_t, void *);
>> + int (*from_xenstore)(libxl__gc *, const char *, libxl_devid, void *);
>> int (*set_xenstore_config)(libxl__gc *, uint32_t, void *, flexarray_t *,
>> flexarray_t *, flexarray_t *);
>> };
>> @@ -4386,6 +4387,13 @@ void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
>> int libxl__device_add(libxl__gc *gc, uint32_t domid,
>> const struct libxl_device_type *dt, void *type);
>>
>> +/* Caller is responsible for freeing the memory by calling
>> + * libxl__device_list_free
>> + */
>> +void* libxl__device_list(libxl__gc *gc, const struct libxl_device_type *dt,
>> + uint32_t domid, const char* name, int *num);
>> +void libxl__device_list_free(const struct libxl_device_type *dt,
>> + void *list, int num);
>> #endif
>>
>> /*
>> --
>> 2.7.4
>>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v4 03/13] libxl: add vdispl device
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
2017-07-18 14:25 ` [PATCH v4 01/13] libxl: add generic function to add device Oleksandr Grytsov
2017-07-18 14:25 ` [PATCH v4 02/13] libxl: add generic functions to get and free device list Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 12:52 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 04/13] xl: add PV display device commands Oleksandr Grytsov
` (11 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/Makefile | 2 +-
tools/libxl/libxl.h | 24 +++
tools/libxl/libxl_create.c | 1 +
tools/libxl/libxl_internal.h | 1 +
tools/libxl/libxl_types.idl | 38 ++++-
tools/libxl/libxl_types_internal.idl | 1 +
tools/libxl/libxl_utils.h | 4 +
tools/libxl/libxl_vdispl.c | 289 +++++++++++++++++++++++++++++++++++
8 files changed, 358 insertions(+), 2 deletions(-)
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..4c0d612 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1851,6 +1851,30 @@ 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)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+void libxl_device_vdispl_list_free(libxl_device_vdispl* list, int num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_vdispl_getinfo(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_vdispl *vdispl,
+ libxl_vdisplinfo *vdisplinfo)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
/* Keyboard */
int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb,
const libxl_asyncop_how *ao_how)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index b2163cd..912bd21 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1443,6 +1443,7 @@ const struct libxl_device_type *device_type_tbl[] = {
&libxl__usbdev_devtype,
&libxl__pcidev_devtype,
&libxl__dtdev_devtype,
+ &libxl__vdispl_devtype,
NULL
};
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 271ac89..68c08aa 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3564,6 +3564,7 @@ extern const struct libxl_device_type libxl__vtpm_devtype;
extern const struct libxl_device_type libxl__usbctrl_devtype;
extern const struct libxl_device_type libxl__usbdev_devtype;
extern const struct libxl_device_type libxl__pcidev_devtype;
+extern const struct libxl_device_type libxl__vdispl_devtype;
extern const struct libxl_device_type *device_type_tbl[];
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 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", [
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 25773d8..9e743dc 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -78,6 +78,10 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
int devid, libxl_device_vtpm *vtpm);
int libxl_devid_to_device_usbctrl(libxl_ctx *ctx, uint32_t domid,
int devid, libxl_device_usbctrl *usbctrl);
+
+int libxl_devid_to_device_vdispl(libxl_ctx *ctx, uint32_t domid,
+ int devid, libxl_device_vdispl *vdispl);
+
int libxl_ctrlport_to_device_usbdev(libxl_ctx *ctx, uint32_t domid,
int ctrl, int port,
libxl_device_usbdev *usbdev);
diff --git a/tools/libxl/libxl_vdispl.c b/tools/libxl/libxl_vdispl.c
new file mode 100644
index 0000000..e75e797
--- /dev/null
+++ b/tools/libxl/libxl_vdispl.c
@@ -0,0 +1,289 @@
+/*
+ * 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_vdispl_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_vdispl *vdispl,
+ bool hotplug)
+{
+ return libxl__resolve_domid(gc, vdispl->backend_domname,
+ &vdispl->backend_domid);
+}
+
+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;
+}
+
+static int libxl__vdispl_from_xenstore(libxl__gc *gc, const char *libxl_path,
+ libxl_devid devid,
+ libxl_device_vdispl *vdispl)
+{
+ char *be_path;
+
+ vdispl->devid = devid;
+ be_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/backend", libxl_path));
+
+ 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)
+{
+ dst->devid = src->devid;
+ dst->be_alloc = src->be_alloc;
+}
+
+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_add_async(egc, domid, &libxl__vdispl_devtype, vdispl, aodev);
+}
+
+static int libxl__set_xenstore_vdispl(libxl__gc *gc, uint32_t domid,
+ libxl_device_vdispl *vdispl,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front)
+{
+ int i;
+
+ flexarray_append_pair(ro_front, "be-alloc",
+ GCSPRINTF("%d", vdispl->be_alloc));
+
+ for (i = 0; i < vdispl->num_connectors; i++) {
+ flexarray_append_pair(ro_front, GCSPRINTF("%d/resolution", i),
+ GCSPRINTF("%dx%d", vdispl->connectors[i].width,
+ vdispl->connectors[i].height));
+ flexarray_append_pair(ro_front, GCSPRINTF("%d/id", i),
+ vdispl->connectors[i].id);
+ }
+
+ return 0;
+}
+
+libxl_device_vdispl *libxl_device_vdispl_list(libxl_ctx *ctx, uint32_t domid,
+ int *num)
+{
+ libxl_device_vdispl *r;
+
+ GC_INIT(ctx);
+
+ r = libxl__device_list(gc, &libxl__vdispl_devtype, domid, "vdispl", num);
+
+ GC_FREE;
+
+ return r;
+}
+
+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)
+{
+ 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;
+}
+
+int libxl_devid_to_device_vdispl(libxl_ctx *ctx, uint32_t domid,
+ int devid, libxl_device_vdispl *vdispl)
+{
+ GC_INIT(ctx);
+
+ libxl_device_vdispl *vdispls = NULL;
+ int n, i;
+ int rc;
+
+ libxl_device_vdispl_init(vdispl);
+
+ vdispls = libxl__device_list(gc, &libxl__vdispl_devtype,
+ domid, "vdispl", &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_list_free(&libxl__vdispl_devtype, vdispls, n);
+
+ GC_FREE;
+ return rc;
+}
+
+LIBXL_DEFINE_DEVICE_ADD(vdispl)
+static LIBXL_DEFINE_DEVICES_ADD(vdispl)
+LIBXL_DEFINE_DEVICE_REMOVE(vdispl)
+static LIBXL_DEFINE_UPDATE_DEVID(vdispl, "vdispl")
+
+DEFINE_DEVICE_TYPE_STRUCT(vdispl,
+ .update_config = (void (*)(libxl__gc *, void *, void *))
+ libxl__update_config_vdispl,
+ .from_xenstore = (int (*)(libxl__gc *, const char *, libxl_devid, void *))
+ libxl__vdispl_from_xenstore,
+ .set_xenstore_config = (int (*)(libxl__gc *, uint32_t, void *,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front))
+ libxl__set_xenstore_vdispl
+);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v4 03/13] libxl: add vdispl device
2017-07-18 14:25 ` [PATCH v4 03/13] libxl: add vdispl device Oleksandr Grytsov
@ 2017-09-05 12:52 ` Wei Liu
2017-09-05 12:58 ` Ian Jackson
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-09-05 12:52 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, wei.liu2, ian.jackson, Oleksandr Grytsov
On Tue, Jul 18, 2017 at 05:25:20PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> ---
> tools/libxl/Makefile | 2 +-
> tools/libxl/libxl.h | 24 +++
> tools/libxl/libxl_create.c | 1 +
> tools/libxl/libxl_internal.h | 1 +
> tools/libxl/libxl_types.idl | 38 ++++-
> tools/libxl/libxl_types_internal.idl | 1 +
> tools/libxl/libxl_utils.h | 4 +
> tools/libxl/libxl_vdispl.c | 289 +++++++++++++++++++++++++++++++++++
Only skim-read this patch because changes are rather self-contained.
> +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);
Using char[128] should be fine.
> +
> + 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))
Coding style.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 03/13] libxl: add vdispl device
2017-09-05 12:52 ` Wei Liu
@ 2017-09-05 12:58 ` Ian Jackson
2017-09-05 13:04 ` Wei Liu
0 siblings, 1 reply; 56+ messages in thread
From: Ian Jackson @ 2017-09-05 12:58 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Oleksandr Grytsov, Oleksandr Grytsov
Wei Liu writes ("Re: [PATCH v4 03/13] libxl: add vdispl device"):
> > + rc = snprintf(connector_path, 128, "%s/%d", path, info->num_connectors);
Why not use GCSPRINTF ? These statically sized buffers etc. are an
invitation to bugs.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 03/13] libxl: add vdispl device
2017-09-05 12:58 ` Ian Jackson
@ 2017-09-05 13:04 ` Wei Liu
2017-09-06 13:02 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-09-05 13:04 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Oleksandr Grytsov, Wei Liu, Oleksandr Grytsov
On Tue, Sep 05, 2017 at 01:58:53PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH v4 03/13] libxl: add vdispl device"):
> > > + rc = snprintf(connector_path, 128, "%s/%d", path, info->num_connectors);
>
> Why not use GCSPRINTF ? These statically sized buffers etc. are an
> invitation to bugs.
Right, that's a better suggestion.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 03/13] libxl: add vdispl device
2017-09-05 13:04 ` Wei Liu
@ 2017-09-06 13:02 ` Oleksandr Grytsov
2017-09-06 13:39 ` Wei Liu
0 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-09-06 13:02 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Tue, Sep 5, 2017 at 4:04 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Sep 05, 2017 at 01:58:53PM +0100, Ian Jackson wrote:
>> Wei Liu writes ("Re: [PATCH v4 03/13] libxl: add vdispl device"):
>> > > + rc = snprintf(connector_path, 128, "%s/%d", path, info->num_connectors);
>>
>> Why not use GCSPRINTF ? These statically sized buffers etc. are an
>> invitation to bugs.
>
> Right, that's a better suggestion.
I reuse connector_path buffer as path to read connector settings from xen store.
So if I use GCSPRINTF for each setting then this function will allocate
about 256 bytes for one connector. In typical scenario each frontend will
have 1 or 2 connectors.
If it is ok I will use GCSPRINTF.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 03/13] libxl: add vdispl device
2017-09-06 13:02 ` Oleksandr Grytsov
@ 2017-09-06 13:39 ` Wei Liu
0 siblings, 0 replies; 56+ messages in thread
From: Wei Liu @ 2017-09-06 13:39 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: Ian Jackson, Wei Liu, Oleksandr Grytsov, xen-devel
On Wed, Sep 06, 2017 at 04:02:23PM +0300, Oleksandr Grytsov wrote:
> On Tue, Sep 5, 2017 at 4:04 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Tue, Sep 05, 2017 at 01:58:53PM +0100, Ian Jackson wrote:
> >> Wei Liu writes ("Re: [PATCH v4 03/13] libxl: add vdispl device"):
> >> > > + rc = snprintf(connector_path, 128, "%s/%d", path, info->num_connectors);
> >>
> >> Why not use GCSPRINTF ? These statically sized buffers etc. are an
> >> invitation to bugs.
> >
> > Right, that's a better suggestion.
>
> I reuse connector_path buffer as path to read connector settings from xen store.
> So if I use GCSPRINTF for each setting then this function will allocate
> about 256 bytes for one connector. In typical scenario each frontend will
> have 1 or 2 connectors.
>
That's OK I think.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v4 04/13] xl: add PV display device commands
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (2 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 03/13] libxl: add vdispl device Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-07-28 14:11 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 05/13] docs: add PV display driver information Oleksandr Grytsov
` (10 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, 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 | 75 ++++++++++++++++++++++-
tools/xl/xl_parse.h | 2 +-
tools/xl/xl_vdispl.c | 163 +++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 261 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..5ae2477 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> [backend=<BackDomain>] [be-alloc=<BackAlloc>] [connectors='<Connectors>']",
+ " BackAlloc - set to 1 to if backend allocates 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..2140905 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -803,6 +803,51 @@ 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("be-alloc", 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 +857,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 +1507,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..3cc99b6
--- /dev/null
+++ b/tools/xl/xl_vdispl.c
@@ -0,0 +1,163 @@
+/*
+ * 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
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v4 05/13] docs: add PV display driver information
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (3 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 04/13] xl: add PV display device commands Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-07-28 14:11 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 06/13] libxl: change p9 to use generec add function Oleksandr Grytsov
` (9 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, 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 | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
docs/man/xl.pod.1.in | 42 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+)
diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 13167ff..7eb5be4 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1096,6 +1096,55 @@ 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<be-alloc=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
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (4 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 05/13] docs: add PV display driver information Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-07-28 14:11 ` Wei Liu
2017-09-05 12:53 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 07/13] libxl: change vkb " Oleksandr Grytsov
` (8 subsequent siblings)
14 siblings, 2 replies; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl_9pfs.c | 67 +++++++++++++++-----------------------------
tools/libxl/libxl_create.c | 2 +-
tools/libxl/libxl_internal.h | 7 +----
tools/libxl/libxl_types.idl | 2 +-
tools/xl/xl_parse.c | 4 +--
5 files changed, 28 insertions(+), 54 deletions(-)
diff --git a/tools/libxl/libxl_9pfs.c b/tools/libxl/libxl_9pfs.c
index 07e3e5f..394d8b4 100644
--- a/tools/libxl/libxl_9pfs.c
+++ b/tools/libxl/libxl_9pfs.c
@@ -17,12 +17,10 @@
#include "libxl_internal.h"
-int libxl__device_p9_setdefault(libxl__gc *gc, libxl_device_p9 *p9)
+static int libxl__device_p9_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_p9 *p9, bool hotplug)
{
- int rc;
-
- rc = libxl__resolve_domid(gc, p9->backend_domname, &p9->backend_domid);
- return rc;
+ return libxl__resolve_domid(gc, p9->backend_domname, &p9->backend_domid);
}
static int libxl__device_from_p9(libxl__gc *gc, uint32_t domid,
@@ -39,49 +37,30 @@ static int libxl__device_from_p9(libxl__gc *gc, uint32_t domid,
return 0;
}
-
-int libxl__device_p9_add(libxl__gc *gc, uint32_t domid,
- libxl_device_p9 *p9)
+static int libxl__set_xenstore_p9(libxl__gc *gc, uint32_t domid,
+ libxl_device_p9 *p9,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front)
{
- flexarray_t *front;
- flexarray_t *back;
- libxl__device device;
- int rc;
-
- rc = libxl__device_p9_setdefault(gc, p9);
- if (rc) goto out;
-
- front = flexarray_make(gc, 16, 1);
- back = flexarray_make(gc, 16, 1);
-
- if (p9->devid == -1) {
- if ((p9->devid = libxl__device_nextid(gc, domid, "9pfs")) < 0) {
- rc = ERROR_FAIL;
- goto out;
- }
- }
-
- rc = libxl__device_from_p9(gc, domid, p9, &device);
- if (rc != 0) goto out;
-
- flexarray_append_pair(back, "frontend-id", libxl__sprintf(gc, "%d", domid));
- flexarray_append_pair(back, "online", "1");
- flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateInitialising));
- flexarray_append_pair(front, "backend-id",
- libxl__sprintf(gc, "%d", p9->backend_domid));
- flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising));
- flexarray_append_pair(front, "tag", p9->tag);
flexarray_append_pair(back, "path", p9->path);
flexarray_append_pair(back, "security_model", p9->security_model);
- libxl__device_generic_add(gc, XBT_NULL, &device,
- libxl__xs_kvs_of_flexarray(gc, back),
- libxl__xs_kvs_of_flexarray(gc, front),
- NULL);
- rc = 0;
-out:
- return rc;
+ flexarray_append_pair(front, "tag", p9->tag);
+
+ return 0;
}
-LIBXL_DEFINE_DEVICE_REMOVE(p9)
+#define libxl__add_p9s NULL
+#define libxl_device_p9_list NULL
+#define libxl_device_p9_compare NULL
+LIBXL_DEFINE_DEVICE_REMOVE(p9)
+static LIBXL_DEFINE_UPDATE_DEVID(p9, "9pfs")
+
+DEFINE_DEVICE_TYPE_STRUCT(p9,
+ .skip_attach = 1,
+ .set_xenstore_config = (int (*)(libxl__gc *, uint32_t, void *,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front))
+ libxl__set_xenstore_p9
+);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 912bd21..f483475 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1325,7 +1325,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
}
for (i = 0; i < d_config->num_p9s; i++)
- libxl__device_p9_add(gc, domid, &d_config->p9[i]);
+ libxl__device_add(gc, domid, &libxl__p9_devtype, &d_config->p9s[i]);
switch (d_config->c_info.type) {
case LIBXL_DOMAIN_TYPE_HVM:
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 68c08aa..c53bbd1 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1252,8 +1252,6 @@ _hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
_hidden void libxl__rdm_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info);
-_hidden int libxl__device_p9_setdefault(libxl__gc *gc,
- libxl_device_p9 *p9);
_hidden const char *libxl__device_nic_devname(libxl__gc *gc,
uint32_t domid,
@@ -2668,10 +2666,6 @@ _hidden int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
_hidden int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid,
libxl_device_vfb *vfb);
-/* Internal function to connect a 9pfs device */
-_hidden int libxl__device_p9_add(libxl__gc *gc, uint32_t domid,
- libxl_device_p9 *p9);
-
/* Waits for the passed device to reach state XenbusStateInitWait.
* This is not really useful by itself, but is important when executing
* hotplug scripts, since we need to be sure the device is in the correct
@@ -3565,6 +3559,7 @@ 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 libxl__p9_devtype;
extern const struct libxl_device_type *device_type_tbl[];
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 25563cf..96dbaed 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [
("vfbs", Array(libxl_device_vfb, "num_vfbs")),
("vkbs", Array(libxl_device_vkb, "num_vkbs")),
("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
- ("p9", Array(libxl_device_p9, "num_p9s")),
+ ("p9s", 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
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 2140905..19eadd9 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1419,9 +1419,9 @@ void parse_config_data(const char *config_source,
char *p, *p2, *buf2;
d_config->num_p9s = 0;
- d_config->p9 = NULL;
+ d_config->p9s = NULL;
while ((buf = xlu_cfg_get_listitem (p9devs, d_config->num_p9s)) != NULL) {
- p9 = ARRAY_EXTEND_INIT(d_config->p9,
+ p9 = ARRAY_EXTEND_INIT(d_config->p9s,
d_config->num_p9s,
libxl_device_p9_init);
libxl_device_p9_init(p9);
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-07-18 14:25 ` [PATCH v4 06/13] libxl: change p9 to use generec add function Oleksandr Grytsov
@ 2017-07-28 14:11 ` Wei Liu
2017-07-28 16:23 ` Wei Liu
2017-09-05 12:53 ` Wei Liu
1 sibling, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-07-28 14:11 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, wei.liu2, ian.jackson, Oleksandr Grytsov
On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote:
[...]
> /* Waits for the passed device to reach state XenbusStateInitWait.
> * This is not really useful by itself, but is important when executing
> * hotplug scripts, since we need to be sure the device is in the correct
> @@ -3565,6 +3559,7 @@ 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 libxl__p9_devtype;
>
> extern const struct libxl_device_type *device_type_tbl[];
>
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 25563cf..96dbaed 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [
> ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
> ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
> ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
> - ("p9", Array(libxl_device_p9, "num_p9s")),
> + ("p9s", Array(libxl_device_p9, "num_p9s")),
Oh, no, please don't do this. We can't change the name of the fields.
There is already on irregular device type -- the PCI device. I suppose
you probably need another hook somewhere. And please convert PCI devices
if you can.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-07-28 14:11 ` Wei Liu
@ 2017-07-28 16:23 ` Wei Liu
2017-07-30 18:42 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-07-28 16:23 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, wei.liu2, ian.jackson, Oleksandr Grytsov
On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote:
> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote:
> [...]
> > /* Waits for the passed device to reach state XenbusStateInitWait.
> > * This is not really useful by itself, but is important when executing
> > * hotplug scripts, since we need to be sure the device is in the correct
> > @@ -3565,6 +3559,7 @@ 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 libxl__p9_devtype;
> >
> > extern const struct libxl_device_type *device_type_tbl[];
> >
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 25563cf..96dbaed 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [
> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
> > - ("p9", Array(libxl_device_p9, "num_p9s")),
> > + ("p9s", Array(libxl_device_p9, "num_p9s")),
>
> Oh, no, please don't do this. We can't change the name of the fields.
>
> There is already on irregular device type -- the PCI device. I suppose
> you probably need another hook somewhere. And please convert PCI devices
> if you can.
OK, going through the code I think we need to come to a conclusion if we
want an extra callback to handle the irregular device names first
because that's likely to affect the code of the framework in previous
patch.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-07-28 16:23 ` Wei Liu
@ 2017-07-30 18:42 ` Oleksandr Grytsov
2017-07-31 14:36 ` Wei Liu
0 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-30 18:42 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote:
>> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote:
>> [...]
>> > /* Waits for the passed device to reach state XenbusStateInitWait.
>> > * This is not really useful by itself, but is important when executing
>> > * hotplug scripts, since we need to be sure the device is in the correct
>> > @@ -3565,6 +3559,7 @@ 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 libxl__p9_devtype;
>> >
>> > extern const struct libxl_device_type *device_type_tbl[];
>> >
>> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> > index 25563cf..96dbaed 100644
>> > --- a/tools/libxl/libxl_types.idl
>> > +++ b/tools/libxl/libxl_types.idl
>> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [
>> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
>> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
>> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
>> > - ("p9", Array(libxl_device_p9, "num_p9s")),
>> > + ("p9s", Array(libxl_device_p9, "num_p9s")),
>>
>> Oh, no, please don't do this. We can't change the name of the fields.
>>
>> There is already on irregular device type -- the PCI device. I suppose
>> you probably need another hook somewhere. And please convert PCI devices
>> if you can.
>
> OK, going through the code I think we need to come to a conclusion if we
> want an extra callback to handle the irregular device names first
> because that's likely to affect the code of the framework in previous
> patch.
Actually creating new callback to handle irregular device name looks
not so good.
There is the pattern which all namings should follow. May be it has to
be documented
somewhere. p9 was added recently we can ask the author to review this rename.
From other side this rename touches only internals changes: no changes
in config file
or CLI interface.
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-07-30 18:42 ` Oleksandr Grytsov
@ 2017-07-31 14:36 ` Wei Liu
2017-08-01 11:58 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-07-31 14:36 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, Wei Liu, Ian Jackson, Oleksandr Grytsov
On Sun, Jul 30, 2017 at 09:42:09PM +0300, Oleksandr Grytsov wrote:
> On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote:
> >> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote:
> >> [...]
> >> > /* Waits for the passed device to reach state XenbusStateInitWait.
> >> > * This is not really useful by itself, but is important when executing
> >> > * hotplug scripts, since we need to be sure the device is in the correct
> >> > @@ -3565,6 +3559,7 @@ 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 libxl__p9_devtype;
> >> >
> >> > extern const struct libxl_device_type *device_type_tbl[];
> >> >
> >> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> >> > index 25563cf..96dbaed 100644
> >> > --- a/tools/libxl/libxl_types.idl
> >> > +++ b/tools/libxl/libxl_types.idl
> >> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [
> >> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
> >> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
> >> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
> >> > - ("p9", Array(libxl_device_p9, "num_p9s")),
> >> > + ("p9s", Array(libxl_device_p9, "num_p9s")),
> >>
> >> Oh, no, please don't do this. We can't change the name of the fields.
> >>
> >> There is already on irregular device type -- the PCI device. I suppose
> >> you probably need another hook somewhere. And please convert PCI devices
> >> if you can.
> >
> > OK, going through the code I think we need to come to a conclusion if we
> > want an extra callback to handle the irregular device names first
> > because that's likely to affect the code of the framework in previous
> > patch.
>
> Actually creating new callback to handle irregular device name looks
> not so good.
> There is the pattern which all namings should follow. May be it has to
> be documented
The normal pattern is DEVTYPEs.
> somewhere. p9 was added recently we can ask the author to review this rename.
Once it is released we can't change it, of course unless we deem it
unstable. I'm two minded here. P9 was released in 4.9, which was only a
few months old.
But we definitely can't change the PCI type. It has been around since
forever. And there is provision in code to deal with that.
> From other side this rename touches only internals changes: no changes
> in config file
> or CLI interface.
>
As said, the framework need to be ready to deal with PCI anyway.
What sort of issues do you foresee here?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-07-31 14:36 ` Wei Liu
@ 2017-08-01 11:58 ` Oleksandr Grytsov
2017-08-01 13:00 ` Wei Liu
0 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-08-01 11:58 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Mon, Jul 31, 2017 at 5:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Sun, Jul 30, 2017 at 09:42:09PM +0300, Oleksandr Grytsov wrote:
>> On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> > On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote:
>> >> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote:
>> >> [...]
>> >> > /* Waits for the passed device to reach state XenbusStateInitWait.
>> >> > * This is not really useful by itself, but is important when executing
>> >> > * hotplug scripts, since we need to be sure the device is in the correct
>> >> > @@ -3565,6 +3559,7 @@ 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 libxl__p9_devtype;
>> >> >
>> >> > extern const struct libxl_device_type *device_type_tbl[];
>> >> >
>> >> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> >> > index 25563cf..96dbaed 100644
>> >> > --- a/tools/libxl/libxl_types.idl
>> >> > +++ b/tools/libxl/libxl_types.idl
>> >> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [
>> >> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
>> >> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
>> >> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
>> >> > - ("p9", Array(libxl_device_p9, "num_p9s")),
>> >> > + ("p9s", Array(libxl_device_p9, "num_p9s")),
>> >>
>> >> Oh, no, please don't do this. We can't change the name of the fields.
>> >>
>> >> There is already on irregular device type -- the PCI device. I suppose
>> >> you probably need another hook somewhere. And please convert PCI devices
>> >> if you can.
>> >
>> > OK, going through the code I think we need to come to a conclusion if we
>> > want an extra callback to handle the irregular device names first
>> > because that's likely to affect the code of the framework in previous
>> > patch.
>>
>> Actually creating new callback to handle irregular device name looks
>> not so good.
>> There is the pattern which all namings should follow. May be it has to
>> be documented
>
> The normal pattern is DEVTYPEs.
>
>> somewhere. p9 was added recently we can ask the author to review this rename.
>
> Once it is released we can't change it, of course unless we deem it
> unstable. I'm two minded here. P9 was released in 4.9, which was only a
> few months old.
>
IMHO this the bug I mean wrong naming and it should be fixed.
> But we definitely can't change the PCI type. It has been around since
> forever. And there is provision in code to deal with that.
Agree, I didn't touch PCI.
>> From other side this rename touches only internals changes: no changes
>> in config file
>> or CLI interface.
>>
>
> As said, the framework need to be ready to deal with PCI anyway.
>
> What sort of issues do you foresee here?
Do you mean in case to rework PCI to use the device framework?
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-08-01 11:58 ` Oleksandr Grytsov
@ 2017-08-01 13:00 ` Wei Liu
2017-08-02 11:37 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-08-01 13:00 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, Wei Liu, Ian Jackson, Oleksandr Grytsov
On Tue, Aug 01, 2017 at 02:58:19PM +0300, Oleksandr Grytsov wrote:
> On Mon, Jul 31, 2017 at 5:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Sun, Jul 30, 2017 at 09:42:09PM +0300, Oleksandr Grytsov wrote:
> >> On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> >> > On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote:
> >> >> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote:
> >> >> [...]
> >> >> > /* Waits for the passed device to reach state XenbusStateInitWait.
> >> >> > * This is not really useful by itself, but is important when executing
> >> >> > * hotplug scripts, since we need to be sure the device is in the correct
> >> >> > @@ -3565,6 +3559,7 @@ 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 libxl__p9_devtype;
> >> >> >
> >> >> > extern const struct libxl_device_type *device_type_tbl[];
> >> >> >
> >> >> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> >> >> > index 25563cf..96dbaed 100644
> >> >> > --- a/tools/libxl/libxl_types.idl
> >> >> > +++ b/tools/libxl/libxl_types.idl
> >> >> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [
> >> >> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
> >> >> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
> >> >> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
> >> >> > - ("p9", Array(libxl_device_p9, "num_p9s")),
> >> >> > + ("p9s", Array(libxl_device_p9, "num_p9s")),
> >> >>
> >> >> Oh, no, please don't do this. We can't change the name of the fields.
> >> >>
> >> >> There is already on irregular device type -- the PCI device. I suppose
> >> >> you probably need another hook somewhere. And please convert PCI devices
> >> >> if you can.
> >> >
> >> > OK, going through the code I think we need to come to a conclusion if we
> >> > want an extra callback to handle the irregular device names first
> >> > because that's likely to affect the code of the framework in previous
> >> > patch.
> >>
> >> Actually creating new callback to handle irregular device name looks
> >> not so good.
> >> There is the pattern which all namings should follow. May be it has to
> >> be documented
> >
> > The normal pattern is DEVTYPEs.
> >
> >> somewhere. p9 was added recently we can ask the author to review this rename.
> >
> > Once it is released we can't change it, of course unless we deem it
> > unstable. I'm two minded here. P9 was released in 4.9, which was only a
> > few months old.
> >
>
> IMHO this the bug I mean wrong naming and it should be fixed.
>
> > But we definitely can't change the PCI type. It has been around since
> > forever. And there is provision in code to deal with that.
>
> Agree, I didn't touch PCI.
>
> >> From other side this rename touches only internals changes: no changes
> >> in config file
> >> or CLI interface.
> >>
> >
> > As said, the framework need to be ready to deal with PCI anyway.
> >
> > What sort of issues do you foresee here?
>
> Do you mean in case to rework PCI to use the device framework?
>
No, I mean adding the new hook. You said "handle irregular device name
looks not so good"
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-08-01 13:00 ` Wei Liu
@ 2017-08-02 11:37 ` Oleksandr Grytsov
2017-08-04 11:53 ` Wei Liu
0 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-08-02 11:37 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Tue, Aug 1, 2017 at 4:00 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Aug 01, 2017 at 02:58:19PM +0300, Oleksandr Grytsov wrote:
>> On Mon, Jul 31, 2017 at 5:36 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> > On Sun, Jul 30, 2017 at 09:42:09PM +0300, Oleksandr Grytsov wrote:
>> >> On Fri, Jul 28, 2017 at 7:23 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> >> > On Fri, Jul 28, 2017 at 03:11:34PM +0100, Wei Liu wrote:
>> >> >> On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote:
>> >> >> [...]
>> >> >> > /* Waits for the passed device to reach state XenbusStateInitWait.
>> >> >> > * This is not really useful by itself, but is important when executing
>> >> >> > * hotplug scripts, since we need to be sure the device is in the correct
>> >> >> > @@ -3565,6 +3559,7 @@ 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 libxl__p9_devtype;
>> >> >> >
>> >> >> > extern const struct libxl_device_type *device_type_tbl[];
>> >> >> >
>> >> >> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> >> >> > index 25563cf..96dbaed 100644
>> >> >> > --- a/tools/libxl/libxl_types.idl
>> >> >> > +++ b/tools/libxl/libxl_types.idl
>> >> >> > @@ -804,7 +804,7 @@ libxl_domain_config = Struct("domain_config", [
>> >> >> > ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
>> >> >> > ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
>> >> >> > ("vtpms", Array(libxl_device_vtpm, "num_vtpms")),
>> >> >> > - ("p9", Array(libxl_device_p9, "num_p9s")),
>> >> >> > + ("p9s", Array(libxl_device_p9, "num_p9s")),
>> >> >>
>> >> >> Oh, no, please don't do this. We can't change the name of the fields.
>> >> >>
>> >> >> There is already on irregular device type -- the PCI device. I suppose
>> >> >> you probably need another hook somewhere. And please convert PCI devices
>> >> >> if you can.
>> >> >
>> >> > OK, going through the code I think we need to come to a conclusion if we
>> >> > want an extra callback to handle the irregular device names first
>> >> > because that's likely to affect the code of the framework in previous
>> >> > patch.
>> >>
>> >> Actually creating new callback to handle irregular device name looks
>> >> not so good.
>> >> There is the pattern which all namings should follow. May be it has to
>> >> be documented
>> >
>> > The normal pattern is DEVTYPEs.
>> >
>> >> somewhere. p9 was added recently we can ask the author to review this rename.
>> >
>> > Once it is released we can't change it, of course unless we deem it
>> > unstable. I'm two minded here. P9 was released in 4.9, which was only a
>> > few months old.
>> >
>>
>> IMHO this the bug I mean wrong naming and it should be fixed.
>>
>> > But we definitely can't change the PCI type. It has been around since
>> > forever. And there is provision in code to deal with that.
>>
>> Agree, I didn't touch PCI.
>>
>> >> From other side this rename touches only internals changes: no changes
>> >> in config file
>> >> or CLI interface.
>> >>
>> >
>> > As said, the framework need to be ready to deal with PCI anyway.
>> >
>> > What sort of issues do you foresee here?
>>
>> Do you mean in case to rework PCI to use the device framework?
>>
>
> No, I mean adding the new hook. You said "handle irregular device name
> looks not so good"
As for me when only internal name of structure or fields are changed
then it should not break anyone configs, setup etc.
Creating hooks in this case makes code hard to read and maintain.
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-08-02 11:37 ` Oleksandr Grytsov
@ 2017-08-04 11:53 ` Wei Liu
2017-08-08 12:39 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-08-04 11:53 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, Wei Liu, Ian Jackson, Oleksandr Grytsov
On Wed, Aug 02, 2017 at 02:37:10PM +0300, Oleksandr Grytsov wrote:
[...]
> >> >> From other side this rename touches only internals changes: no changes
> >> >> in config file
> >> >> or CLI interface.
> >> >>
> >> >
> >> > As said, the framework need to be ready to deal with PCI anyway.
> >> >
> >> > What sort of issues do you foresee here?
> >>
> >> Do you mean in case to rework PCI to use the device framework?
> >>
> >
> > No, I mean adding the new hook. You said "handle irregular device name
> > looks not so good"
>
> As for me when only internal name of structure or fields are changed
> then it should not break anyone configs, setup etc.
> Creating hooks in this case makes code hard to read and maintain.
>
I think you missed my points:
1. libxl types generated from libxl_types.idl aren't just used by xl.
Some applications will use libxl types directly. Not breaking xl config
doesn't mean not breaking other toolstacks like libvirt. In this
particular case, I think we might be able to change p9 to p9s because it
is only released a few months back because the only other known
toolstack that uses libxl can't possibly use that field at the moment.
But Ian might disagree.
2. There is another type, pci dev, that has been there since forever. We
need a hook to deal with it, we can't rename it.
1 and 2 are orthogonal. 2 is a hard requirement.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-08-04 11:53 ` Wei Liu
@ 2017-08-08 12:39 ` Oleksandr Grytsov
2017-08-08 15:05 ` Wei Liu
0 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-08-08 12:39 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Fri, Aug 4, 2017 at 2:53 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Wed, Aug 02, 2017 at 02:37:10PM +0300, Oleksandr Grytsov wrote:
> [...]
>> >> >> From other side this rename touches only internals changes: no changes
>> >> >> in config file
>> >> >> or CLI interface.
>> >> >>
>> >> >
>> >> > As said, the framework need to be ready to deal with PCI anyway.
>> >> >
>> >> > What sort of issues do you foresee here?
>> >>
>> >> Do you mean in case to rework PCI to use the device framework?
>> >>
>> >
>> > No, I mean adding the new hook. You said "handle irregular device name
>> > looks not so good"
>>
>> As for me when only internal name of structure or fields are changed
>> then it should not break anyone configs, setup etc.
>> Creating hooks in this case makes code hard to read and maintain.
>>
>
> I think you missed my points:
>
> 1. libxl types generated from libxl_types.idl aren't just used by xl.
> Some applications will use libxl types directly. Not breaking xl config
> doesn't mean not breaking other toolstacks like libvirt. In this
> particular case, I think we might be able to change p9 to p9s because it
> is only released a few months back because the only other known
> toolstack that uses libxl can't possibly use that field at the moment.
> But Ian might disagree.
I got it. I think that we have to change p9 to p9s ASAP to avoid future hooks.
> 2. There is another type, pci dev, that has been there since forever. We
> need a hook to deal with it, we can't rename it.
>
For PCI all hooks are already there (DEFINE_DEVICE_TYPE_STRUCT_X
to handle pcidev and pci). Also I didn't change PCI fields, names etc.
In libxl_domain_config it is already pcidevs. So, we are safe with PCI.
Sorry I don't understand your concern about PCI. Or I miss something?
> 1 and 2 are orthogonal. 2 is a hard requirement.
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-08-08 12:39 ` Oleksandr Grytsov
@ 2017-08-08 15:05 ` Wei Liu
0 siblings, 0 replies; 56+ messages in thread
From: Wei Liu @ 2017-08-08 15:05 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, Wei Liu, Ian Jackson, Oleksandr Grytsov
On Tue, Aug 08, 2017 at 03:39:23PM +0300, Oleksandr Grytsov wrote:
> On Fri, Aug 4, 2017 at 2:53 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Wed, Aug 02, 2017 at 02:37:10PM +0300, Oleksandr Grytsov wrote:
> > [...]
> >> >> >> From other side this rename touches only internals changes: no changes
> >> >> >> in config file
> >> >> >> or CLI interface.
> >> >> >>
> >> >> >
> >> >> > As said, the framework need to be ready to deal with PCI anyway.
> >> >> >
> >> >> > What sort of issues do you foresee here?
> >> >>
> >> >> Do you mean in case to rework PCI to use the device framework?
> >> >>
> >> >
> >> > No, I mean adding the new hook. You said "handle irregular device name
> >> > looks not so good"
> >>
> >> As for me when only internal name of structure or fields are changed
> >> then it should not break anyone configs, setup etc.
> >> Creating hooks in this case makes code hard to read and maintain.
> >>
> >
> > I think you missed my points:
> >
> > 1. libxl types generated from libxl_types.idl aren't just used by xl.
> > Some applications will use libxl types directly. Not breaking xl config
> > doesn't mean not breaking other toolstacks like libvirt. In this
> > particular case, I think we might be able to change p9 to p9s because it
> > is only released a few months back because the only other known
> > toolstack that uses libxl can't possibly use that field at the moment.
> > But Ian might disagree.
>
> I got it. I think that we have to change p9 to p9s ASAP to avoid future hooks.
>
> > 2. There is another type, pci dev, that has been there since forever. We
> > need a hook to deal with it, we can't rename it.
> >
>
> For PCI all hooks are already there (DEFINE_DEVICE_TYPE_STRUCT_X
> to handle pcidev and pci). Also I didn't change PCI fields, names etc.
> In libxl_domain_config it is already pcidevs. So, we are safe with PCI.
> Sorry I don't understand your concern about PCI. Or I miss something?
Yes I think we're covered there. That macro only covers the case
where new characters are appended.
I was thinking maybe we should have something that deal with new names
weather they are longer or shorter than expected.
But I see now it is probably better to rename the p9 device. I will send
out an email asking for opinions.
>
> > 1 and 2 are orthogonal. 2 is a hard requirement.
>
>
> --
> Best Regards,
> Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v4 06/13] libxl: change p9 to use generec add function
2017-07-18 14:25 ` [PATCH v4 06/13] libxl: change p9 to use generec add function Oleksandr Grytsov
2017-07-28 14:11 ` Wei Liu
@ 2017-09-05 12:53 ` Wei Liu
1 sibling, 0 replies; 56+ messages in thread
From: Wei Liu @ 2017-09-05 12:53 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, wei.liu2, ian.jackson, Oleksandr Grytsov
On Tue, Jul 18, 2017 at 05:25:23PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
This needs rebasing.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v4 07/13] libxl: change vkb to use generec add function
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (5 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 06/13] libxl: change p9 to use generec add function Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 12:54 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 08/13] libxl: change vfb " Oleksandr Grytsov
` (7 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl_console.c | 79 ++++++++------------------------------------
tools/libxl/libxl_create.c | 5 +--
tools/libxl/libxl_dm.c | 3 +-
tools/libxl/libxl_internal.h | 6 +---
4 files changed, 19 insertions(+), 74 deletions(-)
diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index 446e766..48fccec 100644
--- a/tools/libxl/libxl_console.c
+++ b/tools/libxl/libxl_console.c
@@ -583,11 +583,10 @@ int libxl_device_channel_getinfo(libxl_ctx *ctx, uint32_t domid,
return rc;
}
-int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb)
+static int libxl__device_vkb_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_vkb *vkb, bool hotplug)
{
- int rc;
- rc = libxl__resolve_domid(gc, vkb->backend_domname, &vkb->backend_domid);
- return rc;
+ return libxl__resolve_domid(gc, vkb->backend_domname, &vkb->backend_domid);
}
static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
@@ -604,68 +603,6 @@ static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
return 0;
}
-int libxl_device_vkb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vkb *vkb,
- const libxl_asyncop_how *ao_how)
-{
- AO_CREATE(ctx, domid, ao_how);
- int rc;
-
- rc = libxl__device_vkb_add(gc, domid, vkb);
- if (rc) {
- LOGD(ERROR, domid, "Unable to add vkb device");
- goto out;
- }
-
-out:
- libxl__ao_complete(egc, ao, rc);
- return AO_INPROGRESS;
-}
-
-int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
- libxl_device_vkb *vkb)
-{
- flexarray_t *front;
- flexarray_t *back;
- libxl__device device;
- int rc;
-
- rc = libxl__device_vkb_setdefault(gc, vkb);
- if (rc) goto out;
-
- front = flexarray_make(gc, 16, 1);
- back = flexarray_make(gc, 16, 1);
-
- if (vkb->devid == -1) {
- if ((vkb->devid = libxl__device_nextid(gc, domid, "vkb")) < 0) {
- rc = ERROR_FAIL;
- goto out;
- }
- }
-
- rc = libxl__device_from_vkb(gc, domid, vkb, &device);
- if (rc != 0) goto out;
-
- 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(front, "backend-id");
- flexarray_append(front, GCSPRINTF("%d", vkb->backend_domid));
- flexarray_append(front, "state");
- flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising));
-
- libxl__device_generic_add(gc, XBT_NULL, &device,
- libxl__xs_kvs_of_flexarray(gc, back),
- libxl__xs_kvs_of_flexarray(gc, front),
- NULL);
- rc = 0;
-out:
- return rc;
-}
-
int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
{
int rc;
@@ -789,7 +726,17 @@ out:
* 2. dynamically add/remove qemu chardevs via qmp messages. */
/* vkb */
+
+#define libxl__add_vkbs NULL
+#define libxl_device_vkb_list NULL
+#define libxl_device_vkb_compare NULL
+
LIBXL_DEFINE_DEVICE_REMOVE(vkb)
+static LIBXL_DEFINE_UPDATE_DEVID(vkb, "vkbd")
+
+DEFINE_DEVICE_TYPE_STRUCT(vkb,
+ .skip_attach = 1
+);
/* vfb */
LIBXL_DEFINE_DEVICE_REMOVE(vfb)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f483475..9634811 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1346,7 +1346,7 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
}
libxl_device_vkb_init(&vkb);
- libxl__device_vkb_add(gc, domid, &vkb);
+ libxl__device_add(gc, domid, &libxl__vkb_devtype, &vkb);
libxl_device_vkb_dispose(&vkb);
dcs->sdss.dm.guest_domid = domid;
@@ -1372,7 +1372,8 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
for (i = 0; i < d_config->num_vfbs; i++) {
libxl__device_vfb_add(gc, domid, &d_config->vfbs[i]);
- libxl__device_vkb_add(gc, domid, &d_config->vkbs[i]);
+ libxl__device_add(gc, domid, &libxl__vkb_devtype,
+ &d_config->vkbs[i]);
}
init_console_info(gc, &console, 0);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 44ebd70..b37f47e 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1978,7 +1978,8 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
ret = libxl__device_vfb_add(gc, dm_domid, &dm_config->vfbs[0]);
if (ret)
goto out;
- ret = libxl__device_vkb_add(gc, dm_domid, &dm_config->vkbs[0]);
+ ret = libxl__device_add(gc, dm_domid, &libxl__vkb_devtype,
+ &dm_config->vkbs[0]);
if (ret)
goto out;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c53bbd1..49440d7 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1248,7 +1248,6 @@ _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
uint32_t domid, bool hotplug);
_hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
-_hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
_hidden void libxl__rdm_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info);
@@ -2658,10 +2657,6 @@ struct libxl__multidev {
* it's a valid state.
*/
-/* Internal function to connect a vkb device */
-_hidden int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid,
- libxl_device_vkb *vkb);
-
/* Internal function to connect a vfb device */
_hidden int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid,
libxl_device_vfb *vfb);
@@ -3552,6 +3547,7 @@ static inline int *libxl__device_type_get_num(
return (int *)((void *)d_config + dt->num_offset);
}
+extern const struct libxl_device_type libxl__vkb_devtype;
extern const struct libxl_device_type libxl__disk_devtype;
extern const struct libxl_device_type libxl__nic_devtype;
extern const struct libxl_device_type libxl__vtpm_devtype;
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v4 08/13] libxl: change vfb to use generec add function
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (6 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 07/13] libxl: change vkb " Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 12:55 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 09/13] libxl: change disk to use generic getting list functions Oleksandr Grytsov
` (6 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl_console.c | 74 ++++++++++++--------------------------------
tools/libxl/libxl_create.c | 3 +-
tools/libxl/libxl_dm.c | 3 +-
tools/libxl/libxl_internal.h | 6 +---
4 files changed, 25 insertions(+), 61 deletions(-)
diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index 48fccec..4aae82c 100644
--- a/tools/libxl/libxl_console.c
+++ b/tools/libxl/libxl_console.c
@@ -603,7 +603,8 @@ static int libxl__device_from_vkb(libxl__gc *gc, uint32_t domid,
return 0;
}
-int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb)
+static int libxl__device_vfb_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_vfb *vfb, bool hotplug)
{
int rc;
@@ -639,49 +640,11 @@ static int libxl__device_from_vfb(libxl__gc *gc, uint32_t domid,
return 0;
}
-int libxl_device_vfb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_vfb *vfb,
- const libxl_asyncop_how *ao_how)
+static int libxl__set_xenstore_vfb(libxl__gc *gc, uint32_t domid,
+ libxl_device_vfb *vfb,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front)
{
- AO_CREATE(ctx, domid, ao_how);
- int rc;
-
- rc = libxl__device_vfb_add(gc, domid, vfb);
- if (rc) {
- LOGD(ERROR, domid, "Unable to add vfb device");
- goto out;
- }
-
-out:
- libxl__ao_complete(egc, ao, rc);
- return AO_INPROGRESS;
-}
-
-int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
-{
- flexarray_t *front;
- flexarray_t *back;
- libxl__device device;
- int rc;
-
- rc = libxl__device_vfb_setdefault(gc, vfb);
- if (rc) goto out;
-
- front = flexarray_make(gc, 16, 1);
- back = flexarray_make(gc, 16, 1);
-
- if (vfb->devid == -1) {
- if ((vfb->devid = libxl__device_nextid(gc, domid, "vfb")) < 0) {
- rc = ERROR_FAIL;
- goto out;
- }
- }
-
- rc = libxl__device_from_vfb(gc, domid, vfb, &device);
- if (rc != 0) goto out;
-
- flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid));
- flexarray_append_pair(back, "online", "1");
- flexarray_append_pair(back, "state", GCSPRINTF("%d", XenbusStateInitialising));
flexarray_append_pair(back, "vnc",
libxl_defbool_val(vfb->vnc.enable) ? "1" : "0");
flexarray_append_pair(back, "vnclisten", vfb->vnc.listen);
@@ -701,17 +664,7 @@ int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid, libxl_device_vfb *vfb)
flexarray_append_pair(back, "display", vfb->sdl.display);
}
- flexarray_append_pair(front, "backend-id",
- GCSPRINTF("%d", vfb->backend_domid));
- flexarray_append_pair(front, "state", GCSPRINTF("%d", XenbusStateInitialising));
-
- libxl__device_generic_add(gc, XBT_NULL, &device,
- libxl__xs_kvs_of_flexarray(gc, back),
- libxl__xs_kvs_of_flexarray(gc, front),
- NULL);
- rc = 0;
-out:
- return rc;
+ return 0;
}
/* The following functions are defined:
@@ -738,8 +691,21 @@ DEFINE_DEVICE_TYPE_STRUCT(vkb,
.skip_attach = 1
);
+#define libxl__add_vfbs NULL
+#define libxl_device_vfb_list NULL
+#define libxl_device_vfb_compare NULL
+
/* vfb */
LIBXL_DEFINE_DEVICE_REMOVE(vfb)
+static LIBXL_DEFINE_UPDATE_DEVID(vfb, "vfb")
+
+DEFINE_DEVICE_TYPE_STRUCT(vfb,
+ .skip_attach = 1,
+ .set_xenstore_config = (int (*)(libxl__gc *, uint32_t, void *,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front))
+ libxl__set_xenstore_vfb
+);
libxl_xen_console_reader *
libxl_xen_console_read_start(libxl_ctx *ctx, int clear)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 9634811..8089713 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1371,7 +1371,8 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
libxl__device device;
for (i = 0; i < d_config->num_vfbs; i++) {
- libxl__device_vfb_add(gc, domid, &d_config->vfbs[i]);
+ libxl__device_add(gc, domid, &libxl__vfb_devtype,
+ &d_config->vfbs[i]);
libxl__device_add(gc, domid, &libxl__vkb_devtype,
&d_config->vkbs[i]);
}
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b37f47e..0a4f811 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1975,7 +1975,8 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
if (ret)
goto out;
}
- ret = libxl__device_vfb_add(gc, dm_domid, &dm_config->vfbs[0]);
+ ret = libxl__device_add(gc, dm_domid, &libxl__vfb_devtype,
+ &dm_config->vfbs[0]);
if (ret)
goto out;
ret = libxl__device_add(gc, dm_domid, &libxl__vkb_devtype,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 49440d7..36b4d1e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1247,7 +1247,6 @@ _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
uint32_t domid);
_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
uint32_t domid, bool hotplug);
-_hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
_hidden void libxl__rdm_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info);
@@ -2657,10 +2656,6 @@ struct libxl__multidev {
* it's a valid state.
*/
-/* Internal function to connect a vfb device */
-_hidden int libxl__device_vfb_add(libxl__gc *gc, uint32_t domid,
- libxl_device_vfb *vfb);
-
/* Waits for the passed device to reach state XenbusStateInitWait.
* This is not really useful by itself, but is important when executing
* hotplug scripts, since we need to be sure the device is in the correct
@@ -3547,6 +3542,7 @@ static inline int *libxl__device_type_get_num(
return (int *)((void *)d_config + dt->num_offset);
}
+extern const struct libxl_device_type libxl__vfb_devtype;
extern const struct libxl_device_type libxl__vkb_devtype;
extern const struct libxl_device_type libxl__disk_devtype;
extern const struct libxl_device_type libxl__nic_devtype;
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v4 09/13] libxl: change disk to use generic getting list functions
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (7 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 08/13] libxl: change vfb " Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 12:58 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 10/13] libxl: change nic to use generec add function Oleksandr Grytsov
` (5 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl.h | 9 +++-
tools/libxl/libxl_checkpoint_device.c | 7 ++-
tools/libxl/libxl_create.c | 3 +-
tools/libxl/libxl_disk.c | 94 ++++++++++-------------------------
tools/libxl/libxl_internal.h | 7 ---
tools/ocaml/libs/xl/xenlight_stubs.c | 3 +-
tools/xl/xl_block.c | 3 +-
7 files changed, 40 insertions(+), 86 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 4c0d612..991b947 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1723,9 +1723,14 @@ int libxl_device_disk_destroy(libxl_ctx *ctx, uint32_t domid,
const libxl_asyncop_how *ao_how)
LIBXL_EXTERNAL_CALLERS_ONLY;
-libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num);
+libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx,
+ uint32_t domid, int *num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+void libxl_device_disk_list_free(libxl_device_disk* list, int num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
- libxl_device_disk *disk, libxl_diskinfo *diskinfo);
+ libxl_device_disk *disk, libxl_diskinfo *diskinfo)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/*
* Insert a CD-ROM device. A device corresponding to disk must already
diff --git a/tools/libxl/libxl_checkpoint_device.c b/tools/libxl/libxl_checkpoint_device.c
index 01e74b5..7bd832b 100644
--- a/tools/libxl/libxl_checkpoint_device.c
+++ b/tools/libxl/libxl_checkpoint_device.c
@@ -66,7 +66,8 @@ void libxl__checkpoint_devices_setup(libxl__egc *egc,
cds->nics = libxl_device_nic_list(CTX, cds->domid, &cds->num_nics);
if (cds->device_kind_flags & (1 << LIBXL__DEVICE_KIND_VBD))
- cds->disks = libxl_device_disk_list(CTX, cds->domid, &cds->num_disks);
+ cds->disks = libxl__device_list(gc, &libxl__disk_devtype, cds->domid,
+ "disk", &cds->num_disks);
if (cds->num_nics == 0 && cds->num_disks == 0)
goto out;
@@ -221,9 +222,7 @@ static void devices_teardown_cb(libxl__egc *egc,
cds->num_nics = 0;
/* clean disk */
- for (i = 0; i < cds->num_disks; i++)
- libxl_device_disk_dispose(&cds->disks[i]);
- free(cds->disks);
+ libxl__device_list_free(&libxl__vdispl_devtype, cds->disks, cds->num_disks);
cds->disks = NULL;
cds->num_disks = 0;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 8089713..569ff25 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -936,7 +936,8 @@ static void initiate_domain_create(libxl__egc *egc,
store_libxl_entry(gc, domid, &d_config->b_info);
for (i = 0; i < d_config->num_disks; i++) {
- ret = libxl__device_disk_setdefault(gc, &d_config->disks[i], domid);
+ ret = libxl__disk_devtype.set_default(gc, domid, &d_config->disks[i],
+ false);
if (ret) {
LOGD(ERROR, domid, "Unable to set disk defaults for disk %d", i);
goto error_out;
diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index f2f3635..f4f10cb 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -152,8 +152,8 @@ void libxl_evdisable_disk_eject(libxl_ctx *ctx, libxl_evgen_disk_eject *evg) {
GC_FREE;
}
-int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk,
- uint32_t domid)
+static int libxl__device_disk_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_disk *disk, bool hotplug)
{
int rc;
@@ -181,7 +181,7 @@ int libxl__device_disk_setdefault(libxl__gc *gc, libxl_device_disk *disk,
return rc;
}
-int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
+static int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
const libxl_device_disk *disk,
libxl__device *device)
{
@@ -296,7 +296,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
}
}
- rc = libxl__device_disk_setdefault(gc, disk, domid);
+ rc = libxl__device_disk_setdefault(gc, domid, disk, false);
if (rc) goto out;
front = flexarray_make(gc, 16, 1);
@@ -472,17 +472,15 @@ static void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
device_disk_add(egc, domid, disk, aodev, NULL, NULL);
}
-static int libxl__device_disk_from_xenstore(libxl__gc *gc,
- const char *libxl_path,
- libxl_device_disk *disk)
+static int libxl__disk_from_xenstore(libxl__gc *gc, const char *libxl_path,
+ libxl_devid devid,
+ libxl_device_disk *disk)
{
libxl_ctx *ctx = libxl__gc_owner(gc);
unsigned int len;
char *tmp;
int rc;
- libxl_device_disk_init(disk);
-
const char *backend_path;
rc = libxl__xs_read_checked(gc, XBT_NULL,
GCSPRINTF("%s/backend", libxl_path),
@@ -617,69 +615,28 @@ int libxl_vdev_to_device_disk(libxl_ctx *ctx, uint32_t domid,
}
libxl_path = GCSPRINTF("%s/device/vbd/%d", dom_xl_path, devid);
- rc = libxl__device_disk_from_xenstore(gc, libxl_path, disk);
+ rc = libxl__disk_from_xenstore(gc, libxl_path, devid, disk);
out:
GC_FREE;
return rc;
}
-static int libxl__append_disk_list(libxl__gc *gc,
- uint32_t domid,
- libxl_device_disk **disks,
- int *ndisks)
-{
- char *libxl_dir_path = NULL;
- char **dir = NULL;
- unsigned int n = 0;
- libxl_device_disk *pdisk = NULL, *pdisk_end = NULL;
- int rc=0;
- int initial_disks = *ndisks;
-
- libxl_dir_path = GCSPRINTF("%s/device/vbd",
- libxl__xs_libxl_path(gc, domid));
- dir = libxl__xs_directory(gc, XBT_NULL, libxl_dir_path, &n);
- if (dir && n) {
- libxl_device_disk *tmp;
- tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
- if (tmp == NULL)
- return ERROR_NOMEM;
- *disks = tmp;
- pdisk = *disks + initial_disks;
- pdisk_end = *disks + initial_disks + n;
- for (; pdisk < pdisk_end; pdisk++, dir++) {
- const char *p;
- p = GCSPRINTF("%s/%s", libxl_dir_path, *dir);
- if ((rc=libxl__device_disk_from_xenstore(gc, p, pdisk)))
- goto out;
- *ndisks += 1;
- }
- }
-out:
- return rc;
-}
-
libxl_device_disk *libxl_device_disk_list(libxl_ctx *ctx, uint32_t domid, int *num)
{
- GC_INIT(ctx);
- libxl_device_disk *disks = NULL;
- int rc;
+ libxl_device_disk *r;
- *num = 0;
+ GC_INIT(ctx);
- rc = libxl__append_disk_list(gc, domid, &disks, num);
- if (rc) goto out_err;
+ r = libxl__device_list(gc, &libxl__disk_devtype, domid, "disk", num);
GC_FREE;
- return disks;
-out_err:
- LOG(ERROR, "Unable to list disks");
- while (disks && *num) {
- (*num)--;
- libxl_device_disk_dispose(&disks[*num]);
- }
- free(disks);
- return NULL;
+ return r;
+}
+
+void libxl_device_disk_list_free(libxl_device_disk* list, int num)
+{
+ libxl__device_list_free(&libxl__disk_devtype, list, num);
}
int libxl_device_disk_getinfo(libxl_ctx *ctx, uint32_t domid,
@@ -751,7 +708,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
disk_empty.vdev = libxl__strdup(NOGC, disk->vdev);
disk_empty.pdev_path = libxl__strdup(NOGC, "");
disk_empty.is_cdrom = 1;
- libxl__device_disk_setdefault(gc, &disk_empty, domid);
+ libxl__device_disk_setdefault(gc, domid, &disk_empty, false);
libxl_domain_type type = libxl__domain_type(gc, domid);
if (type == LIBXL_DOMAIN_TYPE_INVALID) {
@@ -783,7 +740,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
goto out;
}
- disks = libxl_device_disk_list(ctx, domid, &num);
+ disks = libxl__device_list(gc, &libxl__disk_devtype, domid, "disk", &num);
for (i = 0; i < num; i++) {
if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
{
@@ -798,7 +755,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
goto out;
}
- rc = libxl__device_disk_setdefault(gc, disk, domid);
+ rc = libxl__device_disk_setdefault(gc, domid, disk, false);
if (rc) goto out;
if (!disk->pdev_path) {
@@ -921,9 +878,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
out:
libxl__xs_transaction_abort(gc, &t);
- for (i = 0; i < num; i++)
- libxl_device_disk_dispose(&disks[i]);
- free(disks);
+ libxl__device_list_free(&libxl__disk_devtype, disks, num);
libxl_device_disk_dispose(&disk_empty);
libxl_device_disk_dispose(&disk_saved);
libxl_domain_config_dispose(&d_config);
@@ -1073,7 +1028,8 @@ void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
disk->script = libxl__strdup(gc, in_disk->script);
disk->vdev = NULL;
- rc = libxl__device_disk_setdefault(gc, disk, LIBXL_TOOLSTACK_DOMID);
+ rc = libxl__device_disk_setdefault(gc, LIBXL_TOOLSTACK_DOMID,
+ disk, false);
if (rc) goto out;
libxl__prepare_ao_device(ao, &dls->aodev);
@@ -1249,7 +1205,9 @@ static int libxl_device_disk_dm_needed(void *e, unsigned domid)
DEFINE_DEVICE_TYPE_STRUCT(disk,
.merge = libxl_device_disk_merge,
.dm_needed = libxl_device_disk_dm_needed,
- .skip_attach = 1
+ .from_xenstore = (int (*)(libxl__gc *, const char *, libxl_devid, void *))
+ libxl__disk_from_xenstore,
+ .skip_attach = 1
);
/*
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 36b4d1e..9f8b56a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1242,9 +1242,6 @@ _hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl_domain_create_info *c_info);
_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info);
-_hidden int libxl__device_disk_setdefault(libxl__gc *gc,
- libxl_device_disk *disk,
- uint32_t domid);
_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
uint32_t domid, bool hotplug);
_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
@@ -1755,10 +1752,6 @@ _hidden char *libxl__blktap_devpath(libxl__gc *gc,
*/
_hidden int libxl__device_destroy_tapdisk(libxl__gc *gc, const char *params);
-_hidden int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
- const libxl_device_disk *disk,
- libxl__device *device);
-
/* Calls poll() again - useful to check whether a signaled condition
* is still true. Cannot fail. Returns currently-true revents. */
_hidden short libxl__fd_poll_recheck(libxl__egc *egc, int fd, short events);
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index 98b52b9..55f09d7 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -763,9 +763,8 @@ value stub_xl_device_disk_list(value ctx, value domid)
Field(list, 1) = temp;
temp = list;
Store_field(list, 0, Val_device_disk(&c_list[i]));
- libxl_device_disk_dispose(&c_list[i]);
}
- free(c_list);
+ libxl_device_disk_list_free(c_list, nb);
CAMLreturn(list);
}
diff --git a/tools/xl/xl_block.c b/tools/xl/xl_block.c
index da337ef..acaf9b9 100644
--- a/tools/xl/xl_block.c
+++ b/tools/xl/xl_block.c
@@ -88,9 +88,8 @@ int main_blocklist(int argc, char **argv)
diskinfo.state, diskinfo.evtch, diskinfo.rref, diskinfo.backend);
libxl_diskinfo_dispose(&diskinfo);
}
- libxl_device_disk_dispose(&disks[i]);
}
- free(disks);
+ libxl_device_disk_list_free(disks, nb);
}
return 0;
}
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v4 09/13] libxl: change disk to use generic getting list functions
2017-07-18 14:25 ` [PATCH v4 09/13] libxl: change disk to use generic getting list functions Oleksandr Grytsov
@ 2017-09-05 12:58 ` Wei Liu
0 siblings, 0 replies; 56+ messages in thread
From: Wei Liu @ 2017-09-05 12:58 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, wei.liu2, ian.jackson, Oleksandr Grytsov
On Tue, Jul 18, 2017 at 05:25:26PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
[...]
> /*
> * Insert a CD-ROM device. A device corresponding to disk must already
> diff --git a/tools/libxl/libxl_checkpoint_device.c b/tools/libxl/libxl_checkpoint_device.c
> index 01e74b5..7bd832b 100644
> --- a/tools/libxl/libxl_checkpoint_device.c
> +++ b/tools/libxl/libxl_checkpoint_device.c
> @@ -66,7 +66,8 @@ void libxl__checkpoint_devices_setup(libxl__egc *egc,
> cds->nics = libxl_device_nic_list(CTX, cds->domid, &cds->num_nics);
>
> if (cds->device_kind_flags & (1 << LIBXL__DEVICE_KIND_VBD))
> - cds->disks = libxl_device_disk_list(CTX, cds->domid, &cds->num_disks);
> + cds->disks = libxl__device_list(gc, &libxl__disk_devtype, cds->domid,
> + "disk", &cds->num_disks);
>
> if (cds->num_nics == 0 && cds->num_disks == 0)
> goto out;
> @@ -221,9 +222,7 @@ static void devices_teardown_cb(libxl__egc *egc,
> cds->num_nics = 0;
>
> /* clean disk */
> - for (i = 0; i < cds->num_disks; i++)
> - libxl_device_disk_dispose(&cds->disks[i]);
> - free(cds->disks);
> + libxl__device_list_free(&libxl__vdispl_devtype, cds->disks, cds->num_disks);
Wrong type.
> cds->disks = NULL;
> cds->num_disks = 0;
>
[...]
> @@ -1249,7 +1205,9 @@ static int libxl_device_disk_dm_needed(void *e, unsigned domid)
> DEFINE_DEVICE_TYPE_STRUCT(disk,
> .merge = libxl_device_disk_merge,
> .dm_needed = libxl_device_disk_dm_needed,
> - .skip_attach = 1
> + .from_xenstore = (int (*)(libxl__gc *, const char *, libxl_devid, void *))
> + libxl__disk_from_xenstore,
> + .skip_attach = 1
Unrelated change.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v4 10/13] libxl: change nic to use generec add function
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (8 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 09/13] libxl: change disk to use generic getting list functions Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 13:03 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 11/13] libxl: change vtpm " Oleksandr Grytsov
` (4 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl.h | 9 +-
tools/libxl/libxl_checkpoint_device.c | 9 +-
tools/libxl/libxl_colo_save.c | 4 +-
tools/libxl/libxl_dm.c | 4 +-
tools/libxl/libxl_internal.h | 2 -
tools/libxl/libxl_nic.c | 212 +++++++---------------------------
tools/ocaml/libs/xl/xenlight_stubs.c | 3 +-
tools/xl/xl_nic.c | 3 +-
8 files changed, 59 insertions(+), 187 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 991b947..4a29b43 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1824,9 +1824,14 @@ int libxl_device_nic_destroy(libxl_ctx *ctx, uint32_t domid,
const libxl_asyncop_how *ao_how)
LIBXL_EXTERNAL_CALLERS_ONLY;
-libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num);
+libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx,
+ uint32_t domid, int *num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+void libxl_device_nic_list_free(libxl_device_nic* list, int num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
- libxl_device_nic *nic, libxl_nicinfo *nicinfo);
+ libxl_device_nic *nic, libxl_nicinfo *nicinfo)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/*
* Virtual Channels
diff --git a/tools/libxl/libxl_checkpoint_device.c b/tools/libxl/libxl_checkpoint_device.c
index 7bd832b..eb9c604 100644
--- a/tools/libxl/libxl_checkpoint_device.c
+++ b/tools/libxl/libxl_checkpoint_device.c
@@ -63,7 +63,8 @@ void libxl__checkpoint_devices_setup(libxl__egc *egc,
cds->num_disks = 0;
if (cds->device_kind_flags & (1 << LIBXL__DEVICE_KIND_VIF))
- cds->nics = libxl_device_nic_list(CTX, cds->domid, &cds->num_nics);
+ cds->nics = libxl__device_list(gc, &libxl__nic_devtype, cds->domid,
+ "vif", &cds->num_nics);
if (cds->device_kind_flags & (1 << LIBXL__DEVICE_KIND_VBD))
cds->disks = libxl__device_list(gc, &libxl__disk_devtype, cds->domid,
@@ -206,8 +207,6 @@ static void devices_teardown_cb(libxl__egc *egc,
libxl__multidev *multidev,
int rc)
{
- int i;
-
STATE_AO_GC(multidev->ao);
/* Convenience aliases */
@@ -215,9 +214,7 @@ static void devices_teardown_cb(libxl__egc *egc,
CONTAINER_OF(multidev, *cds, multidev);
/* clean nic */
- for (i = 0; i < cds->num_nics; i++)
- libxl_device_nic_dispose(&cds->nics[i]);
- free(cds->nics);
+ libxl__device_list_free(&libxl__nic_devtype, cds->nics, cds->num_nics);
cds->nics = NULL;
cds->num_nics = 0;
diff --git a/tools/libxl/libxl_colo_save.c b/tools/libxl/libxl_colo_save.c
index f687d5a..8a9d37a 100644
--- a/tools/libxl/libxl_colo_save.c
+++ b/tools/libxl/libxl_colo_save.c
@@ -87,6 +87,7 @@ void libxl__colo_save_setup(libxl__egc *egc, libxl__colo_save_state *css)
libxl__srm_save_autogen_callbacks *const callbacks =
&dss->sws.shs.callbacks.save.a;
libxl_device_nic *nics;
+ int nb;
STATE_AO_GC(dss->ao);
@@ -122,9 +123,10 @@ void libxl__colo_save_setup(libxl__egc *egc, libxl__colo_save_state *css)
cds->device_kind_flags = (1 << LIBXL__DEVICE_KIND_VBD);
/* Use this args we can connect to qemu colo-compare */
- nics = libxl_device_nic_list(CTX, cds->domid, &cds->num_nics);
+ nics = libxl_device_nic_list(CTX, cds->domid, &nb);
css->cps.checkpoint_host = nics->colo_checkpoint_host;
css->cps.checkpoint_port = nics->colo_checkpoint_port;
+ libxl_device_nic_list_free(nics, nb);
} else {
cds->device_kind_flags = (1 << LIBXL__DEVICE_KIND_VIF) |
(1 << LIBXL__DEVICE_KIND_VBD);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 0a4f811..eee4c98 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1970,8 +1970,8 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
* called libxl_device_nic_add at this point, but qemu needs
* the nic information to be complete.
*/
- ret = libxl__device_nic_setdefault(gc, &dm_config->nics[i], dm_domid,
- false);
+ ret = libxl__nic_devtype.set_default(gc, dm_domid, &dm_config->nics[i],
+ false);
if (ret)
goto out;
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9f8b56a..4b1c5ab 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1242,8 +1242,6 @@ _hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl_domain_create_info *c_info);
_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info);
-_hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
- uint32_t domid, bool hotplug);
_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
_hidden void libxl__rdm_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info);
diff --git a/tools/libxl/libxl_nic.c b/tools/libxl/libxl_nic.c
index dd07a6c..16a6c8c 100644
--- a/tools/libxl/libxl_nic.c
+++ b/tools/libxl/libxl_nic.c
@@ -20,15 +20,18 @@
int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
const char *mac, libxl_device_nic *nic)
{
+ GC_INIT(ctx);
libxl_device_nic *nics;
int nb, rc, i;
libxl_mac mac_n;
+ libxl_device_nic_init(nic);
+
rc = libxl__parse_mac(mac, mac_n);
if (rc)
return rc;
- nics = libxl_device_nic_list(ctx, domid, &nb);
+ nics = libxl__device_list(gc, &libxl__nic_devtype, domid, "vif", &nb);
if (!nics)
return ERROR_FAIL;
@@ -37,23 +40,18 @@ int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
rc = ERROR_INVAL;
for (i = 0; i < nb; ++i) {
if (!libxl__compare_macs(&mac_n, &nics[i].mac)) {
- *nic = nics[i];
+ libxl_device_nic_copy(ctx, nic, &nics[i]);
rc = 0;
- i++; /* Do not dispose this NIC on exit path */
break;
}
- libxl_device_nic_dispose(&nics[i]);
}
-
- for (; i<nb; i++)
- libxl_device_nic_dispose(&nics[i]);
-
- free(nics);
+ libxl__device_list_free(&libxl__nic_devtype, nics, nb);
+ GC_FREE;
return rc;
}
-int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
- uint32_t domid, bool hotplug)
+static int libxl__device_nic_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_nic *nic, bool hotplug)
{
int rc;
@@ -138,49 +136,13 @@ static void libxl__update_config_nic(libxl__gc *gc, libxl_device_nic *dst,
libxl_mac_copy(CTX, &dst->mac, &src->mac);
}
-static void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
- libxl_device_nic *nic,
- libxl__ao_device *aodev)
+static int libxl__set_xenstore_nic(libxl__gc *gc, uint32_t domid,
+ libxl_device_nic *nic,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front)
{
- STATE_AO_GC(aodev->ao);
- flexarray_t *front;
- flexarray_t *back;
- libxl__device *device;
- int rc;
- xs_transaction_t t = XBT_NULL;
- libxl_domain_config d_config;
- libxl_device_nic nic_saved;
- libxl__domain_userdata_lock *lock = NULL;
-
- libxl_domain_config_init(&d_config);
- libxl_device_nic_init(&nic_saved);
- libxl_device_nic_copy(CTX, &nic_saved, nic);
-
- rc = libxl__device_nic_setdefault(gc, nic, domid, aodev->update_json);
- if (rc) goto out;
+ flexarray_grow(back, 2);
- front = flexarray_make(gc, 16, 1);
- back = flexarray_make(gc, 18, 1);
-
- if (nic->devid == -1) {
- if ((nic->devid = libxl__device_nextid(gc, domid, "vif")) < 0) {
- rc = ERROR_FAIL;
- goto out;
- }
- }
-
- libxl__update_config_nic(gc, &nic_saved, nic);
-
- GCNEW(device);
- rc = libxl__device_from_nic(gc, domid, nic, device);
- if ( rc != 0 ) goto out;
-
- 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));
if (nic->script)
flexarray_append_pair(back, "script",
libxl__abs_path(gc, nic->script,
@@ -281,78 +243,24 @@ static void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
flexarray_append(back, libxl__strdup(gc,
libxl_nic_type_to_string(nic->nictype)));
- flexarray_append(front, "backend-id");
- flexarray_append(front, GCSPRINTF("%d", nic->backend_domid));
- flexarray_append(front, "state");
- flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising));
flexarray_append(front, "handle");
flexarray_append(front, GCSPRINTF("%d", nic->devid));
flexarray_append(front, "mac");
flexarray_append(front, GCSPRINTF(
LIBXL_MAC_FMT, LIBXL_MAC_BYTES(nic->mac)));
- if (aodev->update_json) {
- lock = libxl__lock_domain_userdata(gc, domid);
- if (!lock) {
- rc = ERROR_LOCK_FAIL;
- goto out;
- }
-
- rc = libxl__get_domain_configuration(gc, domid, &d_config);
- if (rc) goto out;
-
- DEVICE_ADD(nic, nics, domid, &nic_saved, COMPARE_DEVID, &d_config);
-
- rc = libxl__dm_check_start(gc, &d_config, domid);
- if (rc) goto out;
- }
-
- for (;;) {
- rc = libxl__xs_transaction_start(gc, &t);
- if (rc) goto out;
-
- rc = libxl__device_exists(gc, t, device);
- if (rc < 0) goto out;
- if (rc == 1) { /* 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 = libxl__set_domain_configuration(gc, domid, &d_config);
- if (rc) goto out;
- }
-
- libxl__device_generic_add(gc, t, device,
- libxl__xs_kvs_of_flexarray(gc, back),
- libxl__xs_kvs_of_flexarray(gc, front),
- NULL);
-
- rc = libxl__xs_transaction_commit(gc, &t);
- if (!rc) break;
- if (rc < 0) goto out;
- }
-
- aodev->dev = device;
- aodev->action = LIBXL__DEVICE_ACTION_ADD;
- libxl__wait_device_connection(egc, aodev);
+ return 0;
+}
- rc = 0;
-out:
- libxl__xs_transaction_abort(gc, &t);
- if (lock) libxl__unlock_domain_userdata(lock);
- libxl_device_nic_dispose(&nic_saved);
- libxl_domain_config_dispose(&d_config);
- aodev->rc = rc;
- if (rc) aodev->callback(egc, aodev);
- return;
+static void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_nic *nic,
+ libxl__ao_device *aodev)
+{
+ libxl__device_add_async(egc, domid, &libxl__nic_devtype, nic, aodev);
}
-static int libxl__device_nic_from_xenstore(libxl__gc *gc,
- const char *libxl_path,
- libxl_device_nic *nic)
+static int libxl__nic_from_xenstore(libxl__gc *gc, const char *libxl_path,
+ libxl_devid devid, libxl_device_nic *nic)
{
const char *tmp;
int rc;
@@ -498,7 +406,7 @@ int libxl_devid_to_device_nic(libxl_ctx *ctx, uint32_t domid,
libxl_path = GCSPRINTF("%s/device/vif/%d", libxl_dom_path, devid);
- rc = libxl__device_nic_from_xenstore(gc, libxl_path, nic);
+ rc = libxl__nic_from_xenstore(gc, libxl_path, devid, nic);
if (rc) goto out;
rc = 0;
@@ -507,64 +415,22 @@ out:
return rc;
}
-static int libxl__append_nic_list(libxl__gc *gc,
- uint32_t domid,
- libxl_device_nic **nics,
- int *nnics)
-{
- char *libxl_dir_path = NULL;
- char **dir = NULL;
- unsigned int n = 0;
- libxl_device_nic *pnic = NULL, *pnic_end = NULL;
- int rc;
-
- libxl_dir_path = GCSPRINTF("%s/device/vif",
- libxl__xs_libxl_path(gc, domid));
- dir = libxl__xs_directory(gc, XBT_NULL, libxl_dir_path, &n);
- if (dir && n) {
- libxl_device_nic *tmp;
- tmp = realloc(*nics, sizeof (libxl_device_nic) * (*nnics + n));
- if (tmp == NULL)
- return ERROR_NOMEM;
- *nics = tmp;
- pnic = *nics + *nnics;
- pnic_end = *nics + *nnics + n;
- for (; pnic < pnic_end; pnic++, dir++) {
- const char *p;
- p = GCSPRINTF("%s/%s", libxl_dir_path, *dir);
- rc = libxl__device_nic_from_xenstore(gc, p, pnic);
- if (rc) goto out;
- }
- *nnics += n;
- }
- return 0;
-
- out:
- return rc;
-}
-
libxl_device_nic *libxl_device_nic_list(libxl_ctx *ctx, uint32_t domid, int *num)
{
- GC_INIT(ctx);
- libxl_device_nic *nics = NULL;
- int rc;
+ libxl_device_nic *r;
- *num = 0;
+ GC_INIT(ctx);
- rc = libxl__append_nic_list(gc, domid, &nics, num);
- if (rc) goto out_err;
+ r = libxl__device_list(gc, &libxl__nic_devtype, domid, "vif", num);
GC_FREE;
- return nics;
-out_err:
- LOGD(ERROR, domid, "Unable to list nics");
- while (*num) {
- (*num)--;
- libxl_device_nic_dispose(&nics[*num]);
- }
- free(nics);
- return NULL;
+ return r;
+}
+
+void libxl_device_nic_list_free(libxl_device_nic* list, int num)
+{
+ libxl__device_list_free(&libxl__nic_devtype, list, num);
}
int libxl_device_nic_getinfo(libxl_ctx *ctx, uint32_t domid,
@@ -646,7 +512,7 @@ int libxl__device_nic_set_devids(libxl__gc *gc, libxl_domain_config *d_config,
* called libxl_device_nic_add when domcreate_launch_dm gets called,
* but qemu needs the nic information to be complete.
*/
- ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid,
+ ret = libxl__device_nic_setdefault(gc, domid, &d_config->nics[i],
false);
if (ret) {
LOGD(ERROR, domid, "Unable to set nic defaults for nic %d", i);
@@ -669,10 +535,16 @@ LIBXL_DEFINE_DEVICE_ADD(nic)
LIBXL_DEFINE_DEVICES_ADD(nic)
LIBXL_DEFINE_DEVICE_REMOVE(nic)
-#define libxl__device_nic_update_devid NULL
+static LIBXL_DEFINE_UPDATE_DEVID(nic, "vif")
DEFINE_DEVICE_TYPE_STRUCT(nic,
- .update_config = libxl_device_nic_update_config
+ .update_config = libxl_device_nic_update_config,
+ .from_xenstore = (int (*)(libxl__gc *, const char *, libxl_devid, void *))
+ libxl__nic_from_xenstore,
+ .set_xenstore_config = (int (*)(libxl__gc *, uint32_t, void *,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front))
+ libxl__set_xenstore_nic
);
/*
diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c
index 55f09d7..badf9c9 100644
--- a/tools/ocaml/libs/xl/xenlight_stubs.c
+++ b/tools/ocaml/libs/xl/xenlight_stubs.c
@@ -734,9 +734,8 @@ value stub_xl_device_nic_list(value ctx, value domid)
Field(list, 1) = temp;
temp = list;
Store_field(list, 0, Val_device_nic(&c_list[i]));
- libxl_device_nic_dispose(&c_list[i]);
}
- free(c_list);
+ libxl_device_nic_list_free(c_list, nb)
CAMLreturn(list);
}
diff --git a/tools/xl/xl_nic.c b/tools/xl/xl_nic.c
index a78d944..2315dcd 100644
--- a/tools/xl/xl_nic.c
+++ b/tools/xl/xl_nic.c
@@ -124,9 +124,8 @@ int main_networklist(int argc, char **argv)
nicinfo.rref_tx, nicinfo.rref_rx, nicinfo.backend);
libxl_nicinfo_dispose(&nicinfo);
}
- libxl_device_nic_dispose(&nics[i]);
}
- free(nics);
+ libxl_device_nic_list_free(nics, nb);
}
return 0;
}
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v4 10/13] libxl: change nic to use generec add function
2017-07-18 14:25 ` [PATCH v4 10/13] libxl: change nic to use generec add function Oleksandr Grytsov
@ 2017-09-05 13:03 ` Wei Liu
2017-09-06 15:39 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-09-05 13:03 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, ian.jackson, wei.liu2, Oleksandr Grytsov
On Tue, Jul 18, 2017 at 05:25:27PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> diff --git a/tools/libxl/libxl_nic.c b/tools/libxl/libxl_nic.c
> index dd07a6c..16a6c8c 100644
> --- a/tools/libxl/libxl_nic.c
> +++ b/tools/libxl/libxl_nic.c
> @@ -20,15 +20,18 @@
> int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
> const char *mac, libxl_device_nic *nic)
> {
> + GC_INIT(ctx);
> libxl_device_nic *nics;
> int nb, rc, i;
> libxl_mac mac_n;
>
> + libxl_device_nic_init(nic);
> +
Why is this change introduced?
This is changing the behaviour of the API.
To be clear I don't think its original behaviour is desirable. But if
you are to change it, please make a separate patch.
> rc = libxl__parse_mac(mac, mac_n);
> if (rc)
> return rc;
>
> - nics = libxl_device_nic_list(ctx, domid, &nb);
> + nics = libxl__device_list(gc, &libxl__nic_devtype, domid, "vif", &nb);
> if (!nics)
> return ERROR_FAIL;
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 10/13] libxl: change nic to use generec add function
2017-09-05 13:03 ` Wei Liu
@ 2017-09-06 15:39 ` Oleksandr Grytsov
0 siblings, 0 replies; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-09-06 15:39 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Tue, Sep 5, 2017 at 4:03 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jul 18, 2017 at 05:25:27PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> diff --git a/tools/libxl/libxl_nic.c b/tools/libxl/libxl_nic.c
>> index dd07a6c..16a6c8c 100644
>> --- a/tools/libxl/libxl_nic.c
>> +++ b/tools/libxl/libxl_nic.c
>> @@ -20,15 +20,18 @@
>> int libxl_mac_to_device_nic(libxl_ctx *ctx, uint32_t domid,
>> const char *mac, libxl_device_nic *nic)
>> {
>> + GC_INIT(ctx);
>> libxl_device_nic *nics;
>> int nb, rc, i;
>> libxl_mac mac_n;
>>
>> + libxl_device_nic_init(nic);
>> +
>
> Why is this change introduced?
>
> This is changing the behaviour of the API.
>
> To be clear I don't think its original behaviour is desirable. But if
> you are to change it, please make a separate patch.
Yes, the behavior is changed. I will revert these changes.
>> rc = libxl__parse_mac(mac, mac_n);
>> if (rc)
>> return rc;
>>
>> - nics = libxl_device_nic_list(ctx, domid, &nb);
>> + nics = libxl__device_list(gc, &libxl__nic_devtype, domid, "vif", &nb);
>> if (!nics)
>> return ERROR_FAIL;
>>
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* [PATCH v4 11/13] libxl: change vtpm to use generec add function
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (9 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 10/13] libxl: change nic to use generec add function Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 13:05 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 12/13] libxl: remove unneeded DEVICE_ADD macro Oleksandr Grytsov
` (3 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl.h | 12 ++-
tools/libxl/libxl_vtpm.c | 231 +++++++++++++----------------------------------
tools/xl/xl_vtpm.c | 3 +-
3 files changed, 73 insertions(+), 173 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 4a29b43..c169eff 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1580,9 +1580,6 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
int *nb_vcpu, int *nr_cpus_out);
void libxl_vcpuinfo_list_free(libxl_vcpuinfo *, int nr_vcpus);
-void libxl_device_vtpm_list_free(libxl_device_vtpm*, int nr_vtpms);
-void libxl_vtpminfo_list_free(libxl_vtpminfo *, int nr_vtpms);
-
/*
* Devices
* =======
@@ -1857,9 +1854,14 @@ int libxl_device_vtpm_destroy(libxl_ctx *ctx, uint32_t domid,
const libxl_asyncop_how *ao_how)
LIBXL_EXTERNAL_CALLERS_ONLY;
-libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *num);
+libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx,
+ uint32_t domid, int *num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+void libxl_device_vtpm_list_free(libxl_device_vtpm*, int num)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
int libxl_device_vtpm_getinfo(libxl_ctx *ctx, uint32_t domid,
- libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo);
+ libxl_device_vtpm *vtpm, libxl_vtpminfo *vtpminfo)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
/* Virtual displays */
int libxl_device_vdispl_add(libxl_ctx *ctx, uint32_t domid,
diff --git a/tools/libxl/libxl_vtpm.c b/tools/libxl/libxl_vtpm.c
index cbd5461..0661a30 100644
--- a/tools/libxl/libxl_vtpm.c
+++ b/tools/libxl/libxl_vtpm.c
@@ -17,7 +17,9 @@
#include "libxl_internal.h"
-static int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm)
+static int libxl__device_vtpm_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_vtpm *vtpm,
+ bool hotplug)
{
int rc;
if (libxl_uuid_is_nil(&vtpm->uuid)) {
@@ -48,169 +50,73 @@ static void libxl__update_config_vtpm(libxl__gc *gc, libxl_device_vtpm *dst,
libxl_uuid_copy(CTX, &dst->uuid, &src->uuid);
}
-static void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
- libxl_device_vtpm *vtpm,
- libxl__ao_device *aodev)
+static int libxl__set_xenstore_vtpm(libxl__gc *gc, uint32_t domid,
+ libxl_device_vtpm *vtpm,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front)
{
- STATE_AO_GC(aodev->ao);
- flexarray_t *front;
- flexarray_t *back;
- libxl__device *device;
- int rc;
- xs_transaction_t t = XBT_NULL;
- libxl_domain_config d_config;
- libxl_device_vtpm vtpm_saved;
- libxl__domain_userdata_lock *lock = NULL;
-
- libxl_domain_config_init(&d_config);
- libxl_device_vtpm_init(&vtpm_saved);
- libxl_device_vtpm_copy(CTX, &vtpm_saved, vtpm);
+ flexarray_append_pair(back, "handle", GCSPRINTF("%d", vtpm->devid));
+ flexarray_append_pair(back, "uuid",
+ GCSPRINTF(LIBXL_UUID_FMT,
+ LIBXL_UUID_BYTES(vtpm->uuid)));
+ flexarray_append_pair(back, "resume", "False");
- rc = libxl__device_vtpm_setdefault(gc, vtpm);
- if (rc) goto out;
+ flexarray_append_pair(front, "handle", GCSPRINTF("%d", vtpm->devid));
- front = flexarray_make(gc, 16, 1);
- back = flexarray_make(gc, 16, 1);
+ return 0;
+}
- if (vtpm->devid == -1) {
- if ((vtpm->devid = libxl__device_nextid(gc, domid, "vtpm")) < 0) {
- rc = ERROR_FAIL;
- goto out;
- }
- }
- libxl__update_config_vtpm(gc, &vtpm_saved, vtpm);
-
- GCNEW(device);
- rc = libxl__device_from_vtpm(gc, domid, vtpm, device);
- if ( rc != 0 ) goto out;
-
- 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", vtpm->devid));
-
- flexarray_append(back, "uuid");
- flexarray_append(back, GCSPRINTF(LIBXL_UUID_FMT, LIBXL_UUID_BYTES(vtpm->uuid)));
- flexarray_append(back, "resume");
- flexarray_append(back, "False");
-
- flexarray_append(front, "backend-id");
- flexarray_append(front, GCSPRINTF("%d", vtpm->backend_domid));
- flexarray_append(front, "state");
- flexarray_append(front, GCSPRINTF("%d", XenbusStateInitialising));
- flexarray_append(front, "handle");
- flexarray_append(front, GCSPRINTF("%d", vtpm->devid));
-
- if (aodev->update_json) {
- lock = libxl__lock_domain_userdata(gc, domid);
- if (!lock) {
- rc = ERROR_LOCK_FAIL;
- goto out;
- }
+static void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_vtpm *vtpm,
+ libxl__ao_device *aodev)
+{
+ libxl__device_add_async(egc, domid, &libxl__vtpm_devtype, vtpm, aodev);
+}
- rc = libxl__get_domain_configuration(gc, domid, &d_config);
- if (rc) goto out;
+static int libxl__vtpm_from_xenstore(libxl__gc *gc, const char *libxl_path,
+ libxl_devid devid,
+ libxl_device_vtpm *vtpm)
+{
+ int rc;
+ char *be_path;
+ char *uuid;
- DEVICE_ADD(vtpm, vtpms, domid, &vtpm_saved, COMPARE_DEVID, &d_config);
+ vtpm->devid = devid;
- rc = libxl__dm_check_start(gc, &d_config, domid);
- if (rc) goto out;
- }
+ be_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/backend", libxl_path));
- for (;;) {
- rc = libxl__xs_transaction_start(gc, &t);
- if (rc) goto out;
-
- rc = libxl__device_exists(gc, t, device);
- if (rc < 0) goto out;
- if (rc == 1) { /* 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;
- }
+ rc = libxl__backendpath_parse_domid(gc, be_path, &vtpm->backend_domid);
+ if (rc) return rc;
- if (aodev->update_json) {
- rc = libxl__set_domain_configuration(gc, domid, &d_config);
- if (rc) goto out;
+ uuid = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/uuid", be_path));
+ if (uuid) {
+ if(libxl_uuid_from_string(&(vtpm->uuid), uuid)) {
+ LOGD(ERROR, vtpm->backend_domid, "%s/uuid is a malformed uuid?? "
+ "(%s) Probably a bug!!\n", be_path, uuid);
+ return ERROR_FAIL;
}
-
- libxl__device_generic_add(gc, t, device,
- libxl__xs_kvs_of_flexarray(gc, back),
- libxl__xs_kvs_of_flexarray(gc, front),
- NULL);
-
- rc = libxl__xs_transaction_commit(gc, &t);
- if (!rc) break;
- if (rc < 0) goto out;
}
- aodev->dev = device;
- aodev->action = LIBXL__DEVICE_ACTION_ADD;
- libxl__wait_device_connection(egc, aodev);
-
- rc = 0;
-out:
- libxl__xs_transaction_abort(gc, &t);
- if (lock) libxl__unlock_domain_userdata(lock);
- libxl_device_vtpm_dispose(&vtpm_saved);
- libxl_domain_config_dispose(&d_config);
- aodev->rc = rc;
- if(rc) aodev->callback(egc, aodev);
- return;
+ return 0;
}
libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx, uint32_t domid, int *num)
{
+ libxl_device_vtpm *r;
+
GC_INIT(ctx);
- libxl_device_vtpm* vtpms = NULL;
- char *libxl_path;
- char** dir = NULL;
- unsigned int ndirs = 0;
- int rc;
-
- *num = 0;
-
- libxl_path = GCSPRINTF("%s/device/vtpm", libxl__xs_libxl_path(gc, domid));
- dir = libxl__xs_directory(gc, XBT_NULL, libxl_path, &ndirs);
- if (dir && ndirs) {
- vtpms = malloc(sizeof(*vtpms) * ndirs);
- libxl_device_vtpm* vtpm;
- libxl_device_vtpm* end = vtpms + ndirs;
- for(vtpm = vtpms; vtpm < end; ++vtpm, ++dir) {
- char* tmp;
- const char* be_path = libxl__xs_read(gc, XBT_NULL,
- GCSPRINTF("%s/%s/backend",
- libxl_path, *dir));
-
- libxl_device_vtpm_init(vtpm);
-
- vtpm->devid = atoi(*dir);
-
- rc = libxl__backendpath_parse_domid(gc, be_path,
- &vtpm->backend_domid);
- if (rc) return NULL;
-
- tmp = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/uuid", libxl_path));
- if (tmp) {
- if(libxl_uuid_from_string(&(vtpm->uuid), tmp)) {
- LOGD(ERROR, domid, "%s/uuid is a malformed uuid?? (%s) Probably a bug!!\n", be_path, tmp);
- free(vtpms);
- return NULL;
- }
- }
- }
- }
- *num = ndirs;
+ r = libxl__device_list(gc, &libxl__vtpm_devtype, domid, "vtpm", num);
GC_FREE;
- return vtpms;
+
+ return r;
+}
+
+void libxl_device_vtpm_list_free(libxl_device_vtpm* list, int num)
+{
+ libxl__device_list_free(&libxl__vtpm_devtype, list, num);
}
int libxl_device_vtpm_getinfo(libxl_ctx *ctx,
@@ -282,11 +188,12 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx,
int devid,
libxl_device_vtpm *vtpm)
{
+ GC_INIT(ctx);
libxl_device_vtpm *vtpms;
int nb, i;
int rc;
- vtpms = libxl_device_vtpm_list(ctx, domid, &nb);
+ vtpms = libxl__device_list(gc, &libxl__vtpm_devtype, domid, "vtpm", &nb);
if (!vtpms)
return ERROR_FAIL;
@@ -302,7 +209,8 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx,
}
}
- libxl_device_vtpm_list_free(vtpms, nb);
+ libxl__device_list_free(&libxl__vtpm_devtype, vtpms, nb);
+ GC_FREE;
return rc;
}
@@ -315,11 +223,12 @@ static int libxl_device_vtpm_compare(libxl_device_vtpm *d1,
int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
libxl_uuid* uuid, libxl_device_vtpm *vtpm)
{
+ GC_INIT(ctx);
libxl_device_vtpm *vtpms;
int nb, i;
int rc;
- vtpms = libxl_device_vtpm_list(ctx, domid, &nb);
+ vtpms = libxl__device_list(gc, &libxl__vtpm_devtype, domid, "vtpm", &nb);
if (!vtpms)
return ERROR_FAIL;
@@ -335,26 +244,11 @@ int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
}
}
- libxl_device_vtpm_list_free(vtpms, nb);
+ libxl__device_list_free(&libxl__vtpm_devtype, vtpms, nb);
+ GC_FREE;
return rc;
}
-void libxl_vtpminfo_list_free(libxl_vtpminfo* list, int nr)
-{
- int i;
- for (i = 0; i < nr; i++)
- libxl_vtpminfo_dispose(&list[i]);
- free(list);
-}
-
-void libxl_device_vtpm_list_free(libxl_device_vtpm* list, int nr)
-{
- int i;
- for (i = 0; i < nr; i++)
- libxl_device_vtpm_dispose(&list[i]);
- free(list);
-}
-
static void libxl_device_vtpm_update_config(libxl__gc *gc, void *d, void *s)
{
libxl__update_config_vtpm(gc, d, s);
@@ -363,11 +257,16 @@ static void libxl_device_vtpm_update_config(libxl__gc *gc, void *d, void *s)
LIBXL_DEFINE_DEVICE_ADD(vtpm)
static LIBXL_DEFINE_DEVICES_ADD(vtpm)
LIBXL_DEFINE_DEVICE_REMOVE(vtpm)
-
-#define libxl__device_vtpm_update_devid NULL
+static LIBXL_DEFINE_UPDATE_DEVID(vtpm, "vtpm")
DEFINE_DEVICE_TYPE_STRUCT(vtpm,
- .update_config = libxl_device_vtpm_update_config
+ .update_config = libxl_device_vtpm_update_config,
+ .from_xenstore = (int (*)(libxl__gc *, const char *, libxl_devid, void *))
+ libxl__vtpm_from_xenstore,
+ .set_xenstore_config = (int (*)(libxl__gc *, uint32_t, void *,
+ flexarray_t *back, flexarray_t *front,
+ flexarray_t *ro_front))
+ libxl__set_xenstore_vtpm
);
/*
diff --git a/tools/xl/xl_vtpm.c b/tools/xl/xl_vtpm.c
index 6f56be0..0ba5041 100644
--- a/tools/xl/xl_vtpm.c
+++ b/tools/xl/xl_vtpm.c
@@ -105,9 +105,8 @@ int main_vtpmlist(int argc, char **argv)
libxl_vtpminfo_dispose(&vtpminfo);
}
- libxl_device_vtpm_dispose(&vtpms[i]);
}
- free(vtpms);
+ libxl_device_vtpm_list_free(vtpms, nb);
}
return 0;
}
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v4 12/13] libxl: remove unneeded DEVICE_ADD macro
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (10 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 11/13] libxl: change vtpm " Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 13:05 ` Wei Liu
2017-07-18 14:25 ` [PATCH v4 13/13] libxl: make pci and usb setdefault function generic Oleksandr Grytsov
` (2 subsequent siblings)
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl_device.c | 6 ++---
tools/libxl/libxl_disk.c | 5 +++--
tools/libxl/libxl_internal.h | 52 +++-----------------------------------------
tools/libxl/libxl_pci.c | 3 ++-
tools/libxl/libxl_usb.c | 8 +++----
5 files changed, 14 insertions(+), 60 deletions(-)
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index f1d4848..ca7b165 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -1793,10 +1793,8 @@ out:
return AO_CREATE_FAIL(rc);
}
-static void device_add_domain_config(libxl__gc *gc,
- libxl_domain_config *d_config,
- const struct libxl_device_type *dt,
- void *type)
+void device_add_domain_config(libxl__gc *gc, libxl_domain_config *d_config,
+ const struct libxl_device_type *dt, void *type)
{
int *num_dev;
int i;
diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index f4f10cb..c41c7b5 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -277,7 +277,8 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
rc = libxl__get_domain_configuration(gc, domid, &d_config);
if (rc) goto out;
- DEVICE_ADD(disk, disks, domid, &disk_saved, COMPARE_DISK, &d_config);
+ device_add_domain_config(gc, &d_config, &libxl__disk_devtype,
+ &disk_saved);
rc = libxl__dm_check_start(gc, &d_config, domid);
if (rc) goto out;
@@ -832,7 +833,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
rc = libxl__get_domain_configuration(gc, domid, &d_config);
if (rc) goto out;
- DEVICE_ADD(disk, disks, domid, &disk_saved, COMPARE_DISK, &d_config);
+ device_add_domain_config(gc, &d_config, &libxl__disk_devtype, &disk_saved);
rc = libxl__dm_check_start(gc, &d_config, domid);
if (rc) goto out;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4b1c5ab..5fd0356 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4282,55 +4282,6 @@ void libxl__xcinfo2xlinfo(libxl_ctx *ctx,
(a)->port == (b)->port)
#define COMPARE_USBCTRL(a, b) ((a)->devid == (b)->devid)
-/* DEVICE_ADD
- *
- * Add a device in libxl_domain_config structure
- *
- * It takes 6 parameters:
- * type: the type of the device, say nic, vtpm, disk, pci etc
- * ptr: pointer to the start of the array, the array must be
- * of type libxl_device_#type
- * domid: domain id of target domain
- * dev: the device that is to be added / removed / updated
- * compare: the COMPARE_* macro used to compare @dev's identifier to
- * those in the array pointed to by @ptr
- * d_config: pointer to template domain config
- *
- * For most device types (nic, vtpm), the array pointer @ptr can be
- * derived from @type, pci device being the exception, hence we need
- * to have @ptr.
- *
- * If there is already a device with the same identifier in d_config,
- * that entry is updated.
- */
-#define DEVICE_ADD(type, ptr, domid, dev, compare, d_config) \
- ({ \
- int DA_x; \
- libxl_device_##type *DA_p = NULL; \
- \
- /* Check for existing device */ \
- for (DA_x = 0; DA_x < (d_config)->num_##ptr; DA_x++) { \
- if (compare(&(d_config)->ptr[DA_x], (dev))) { \
- DA_p = &(d_config)->ptr[DA_x]; \
- break; \
- } \
- } \
- \
- if (!DA_p) { \
- (d_config)->ptr = \
- libxl__realloc(NOGC, (d_config)->ptr, \
- ((d_config)->num_##ptr + 1) * \
- sizeof(libxl_device_##type)); \
- DA_p = &(d_config)->ptr[(d_config)->num_##ptr]; \
- (d_config)->num_##ptr++; \
- } else { \
- libxl_device_##type##_dispose(DA_p); \
- } \
- \
- libxl_device_##type##_init(DA_p); \
- libxl_device_##type##_copy(CTX, DA_p, (dev)); \
- })
-
/* This function copies X bytes from source to destination bitmap,
* where X is the smaller of the two sizes.
*
@@ -4360,6 +4311,9 @@ static inline bool libxl__acpi_defbool_val(const libxl_domain_build_info *b_info
libxl_defbool_val(b_info->u.hvm.acpi);
}
+void device_add_domain_config(libxl__gc *gc, libxl_domain_config *d_config,
+ const struct libxl_device_type *dt, void *type);
+
void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
const struct libxl_device_type *dt, void *type,
libxl__ao_device *aodev);
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index c3f1e5c..159d046 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -160,7 +160,8 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_d
rc = libxl__get_domain_configuration(gc, domid, &d_config);
if (rc) goto out;
- DEVICE_ADD(pci, pcidevs, domid, &pcidev_saved, COMPARE_PCI, &d_config);
+ device_add_domain_config(gc, &d_config, &libxl__pcidev_devtype,
+ &pcidev_saved);
rc = libxl__dm_check_start(gc, &d_config, domid);
if (rc) goto out;
diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index 07fb202..e526c08 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -245,8 +245,8 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
rc = libxl__get_domain_configuration(gc, domid, &d_config);
if (rc) goto out;
- DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved,
- COMPARE_USBCTRL, &d_config);
+ device_add_domain_config(gc, &d_config, &libxl__usbctrl_devtype,
+ &usbctrl_saved);
rc = libxl__dm_check_start(gc, &d_config, domid);
if (rc) goto out;
@@ -1199,8 +1199,8 @@ static int libxl__device_usbdev_add_xenstore(libxl__gc *gc, uint32_t domid,
rc = libxl__get_domain_configuration(gc, domid, &d_config);
if (rc) goto out;
- DEVICE_ADD(usbdev, usbdevs, domid, &usbdev_saved,
- COMPARE_USB, &d_config);
+ device_add_domain_config(gc, &d_config, &libxl__usbdev_devtype,
+ &usbdev_saved);
rc = libxl__dm_check_start(gc, &d_config, domid);
if (rc) goto out;
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* [PATCH v4 13/13] libxl: make pci and usb setdefault function generic
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (11 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 12/13] libxl: remove unneeded DEVICE_ADD macro Oleksandr Grytsov
@ 2017-07-18 14:25 ` Oleksandr Grytsov
2017-09-05 13:06 ` Wei Liu
2017-07-27 11:30 ` [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Andrushchenko
2017-07-28 14:13 ` Wei Liu
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-07-18 14:25 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, wei.liu2, Oleksandr Grytsov
From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Due to changes in device framework setdefault function
should have same format. Otherwise calling devicetype
set_default causes segfault.
Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
---
tools/libxl/libxl_internal.h | 1 -
tools/libxl/libxl_pci.c | 5 +++--
tools/libxl/libxl_usb.c | 7 ++++---
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 5fd0356..e4799eb 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1242,7 +1242,6 @@ _hidden int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl_domain_create_info *c_info);
_hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info);
-_hidden int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci);
_hidden void libxl__rdm_setdefault(libxl__gc *gc,
libxl_domain_build_info *b_info);
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 159d046..fa86bcf 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1158,7 +1158,8 @@ static int libxl__device_pci_reset(libxl__gc *gc, unsigned int domain, unsigned
return -1;
}
-int libxl__device_pci_setdefault(libxl__gc *gc, libxl_device_pci *pci)
+static int libxl__device_pci_setdefault(libxl__gc *gc, uint32_t domid,
+ libxl_device_pci *pci, bool hotplug)
{
/* We'd like to force reserve rdm specific to a device by default.*/
if (pci->rdm_policy == LIBXL_RDM_RESERVE_POLICY_INVALID)
@@ -1214,7 +1215,7 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcide
}
}
- rc = libxl__device_pci_setdefault(gc, pcidev);
+ rc = libxl__device_pci_setdefault(gc, domid, pcidev, false);
if (rc) goto out;
if (pcidev->seize && !pciback_dev_is_assigned(gc, pcidev)) {
diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
index e526c08..9fdb284 100644
--- a/tools/libxl/libxl_usb.c
+++ b/tools/libxl/libxl_usb.c
@@ -39,7 +39,8 @@ static int usbback_is_loaded(libxl__gc *gc)
}
static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid,
- libxl_device_usbctrl *usbctrl)
+ libxl_device_usbctrl *usbctrl,
+ bool update_json)
{
int rc;
libxl_domain_type domtype = libxl__domain_type(gc, domid);
@@ -449,7 +450,7 @@ static void libxl__device_usbctrl_add(libxl__egc *egc, uint32_t domid,
libxl__device *device;
int rc;
- rc = libxl__device_usbctrl_setdefault(gc, domid, usbctrl);
+ rc = libxl__device_usbctrl_setdefault(gc, domid, usbctrl, false);
if (rc < 0) goto out;
if (usbctrl->devid == -1) {
@@ -1079,7 +1080,7 @@ static int libxl__device_usbdev_setdefault(libxl__gc *gc,
GCNEW(usbctrl);
libxl_device_usbctrl_init(usbctrl);
- rc = libxl__device_usbctrl_setdefault(gc, domid, usbctrl);
+ rc = libxl__device_usbctrl_setdefault(gc, domid, usbctrl, update_json);
if (rc < 0) goto out;
if (usbctrl->devid == -1) {
--
2.7.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 56+ messages in thread* Re: [PATCH v4 13/13] libxl: make pci and usb setdefault function generic
2017-07-18 14:25 ` [PATCH v4 13/13] libxl: make pci and usb setdefault function generic Oleksandr Grytsov
@ 2017-09-05 13:06 ` Wei Liu
2017-09-06 15:53 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-09-05 13:06 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, wei.liu2, ian.jackson, Oleksandr Grytsov
On Tue, Jul 18, 2017 at 05:25:30PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Due to changes in device framework setdefault function
> should have same format. Otherwise calling devicetype
> set_default causes segfault.
>
> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Shouldn't this patch be placed before the introduction of the new
framework?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v4 13/13] libxl: make pci and usb setdefault function generic
2017-09-05 13:06 ` Wei Liu
@ 2017-09-06 15:53 ` Oleksandr Grytsov
2017-09-07 9:05 ` Wei Liu
0 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-09-06 15:53 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Tue, Sep 5, 2017 at 4:06 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jul 18, 2017 at 05:25:30PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Due to changes in device framework setdefault function
>> should have same format. Otherwise calling devicetype
>> set_default causes segfault.
>>
>> Signed-off-by: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Shouldn't this patch be placed before the introduction of the new
> framework?
Wrong function parameters will cause crash if devtype framework
will be used. For example if someone call pci set_default:
libxl__nic_devtype.set_default(...)
So I guess the right place for these changes will be
first patch where changes to devtype are introduced.
I will fix setdefault function parameters for all devtypes.
Does it sounds good?
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v4 00/13] libxl: add PV display device driver interface
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (12 preceding siblings ...)
2017-07-18 14:25 ` [PATCH v4 13/13] libxl: make pci and usb setdefault function generic Oleksandr Grytsov
@ 2017-07-27 11:30 ` Oleksandr Andrushchenko
2017-07-27 14:49 ` Wei Liu
2017-07-28 14:13 ` Wei Liu
14 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Andrushchenko @ 2017-07-27 11:30 UTC (permalink / raw)
To: Oleksandr Grytsov, xen-devel; +Cc: wei.liu2, ian.jackson, Oleksandr Grytsov
ping
On 07/18/2017 05:25 PM, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Changes since V3:
> * 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 required to unreasonable extend
> libxl__device_add_async and its parameters;
> * disk device list changed to use libxl__device_list;
> * previous comments are applied.
>
> Patches on github [1].
>
> [1] https://github.com/al1img/xen/tree/xl-vdispl-v4
>
> Oleksandr Grytsov (13):
> libxl: add generic function to add device
> libxl: add generic functions to get and free device list
> libxl: add vdispl device
> xl: add PV display device commands
> docs: add PV display driver information
> libxl: change p9 to use generec add function
> libxl: change vkb to use generec add function
> libxl: change vfb to use generec add function
> libxl: change disk to use generic getting list functions
> libxl: change nic to use generec add function
> libxl: change vtpm to use generec add function
> libxl: remove unneeded DEVICE_ADD macro
> libxl: make pci and usb setdefault function generic
>
> docs/man/xl.cfg.pod.5.in | 49 ++++++
> docs/man/xl.pod.1.in | 42 +++++
> tools/libxl/Makefile | 2 +-
> tools/libxl/libxl.h | 54 +++++--
> tools/libxl/libxl_9pfs.c | 67 +++-----
> tools/libxl/libxl_checkpoint_device.c | 16 +-
> tools/libxl/libxl_colo_save.c | 4 +-
> tools/libxl/libxl_console.c | 153 ++++--------------
> tools/libxl/libxl_create.c | 17 +-
> tools/libxl/libxl_device.c | 262 ++++++++++++++++++++++++++++++
> tools/libxl/libxl_disk.c | 101 ++++--------
> tools/libxl/libxl_dm.c | 10 +-
> tools/libxl/libxl_internal.h | 126 ++++++---------
> tools/libxl/libxl_nic.c | 212 +++++--------------------
> tools/libxl/libxl_pci.c | 10 +-
> tools/libxl/libxl_types.idl | 40 ++++-
> tools/libxl/libxl_types_internal.idl | 1 +
> tools/libxl/libxl_usb.c | 21 ++-
> tools/libxl/libxl_utils.h | 4 +
> tools/libxl/libxl_vdispl.c | 289 ++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_vtpm.c | 229 ++++++++-------------------
> tools/ocaml/libs/xl/xenlight_stubs.c | 6 +-
> tools/xl/Makefile | 1 +
> tools/xl/xl.h | 3 +
> tools/xl/xl_block.c | 3 +-
> tools/xl/xl_cmdtable.c | 19 +++
> tools/xl/xl_nic.c | 3 +-
> tools/xl/xl_parse.c | 79 +++++++++-
> tools/xl/xl_parse.h | 2 +-
> tools/xl/xl_vdispl.c | 163 +++++++++++++++++++
> tools/xl/xl_vtpm.c | 3 +-
> 31 files changed, 1293 insertions(+), 698 deletions(-)
> create mode 100644 tools/libxl/libxl_vdispl.c
> create mode 100644 tools/xl/xl_vdispl.c
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 00/13] libxl: add PV display device driver interface
2017-07-18 14:25 [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Grytsov
` (13 preceding siblings ...)
2017-07-27 11:30 ` [PATCH v4 00/13] libxl: add PV display device driver interface Oleksandr Andrushchenko
@ 2017-07-28 14:13 ` Wei Liu
2017-08-17 10:13 ` Oleksandr Grytsov
14 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-07-28 14:13 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, wei.liu2, ian.jackson, Oleksandr Grytsov
On Tue, Jul 18, 2017 at 05:25:17PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>
> Changes since V3:
> * 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 required to unreasonable extend
> libxl__device_add_async and its parameters;
> * disk device list changed to use libxl__device_list;
> * previous comments are applied.
>
> Patches on github [1].
>
> [1] https://github.com/al1img/xen/tree/xl-vdispl-v4
So I just went through this series and pointed out issues I can
immediately find. I will need to take a closer look at the framework
itself next week.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 00/13] libxl: add PV display device driver interface
2017-07-28 14:13 ` Wei Liu
@ 2017-08-17 10:13 ` Oleksandr Grytsov
2017-08-17 11:11 ` Wei Liu
0 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-08-17 10:13 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, Oleksandr Grytsov
On Fri, Jul 28, 2017 at 5:13 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Tue, Jul 18, 2017 at 05:25:17PM +0300, Oleksandr Grytsov wrote:
>> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>>
>> Changes since V3:
>> * 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 required to unreasonable extend
>> libxl__device_add_async and its parameters;
>> * disk device list changed to use libxl__device_list;
>> * previous comments are applied.
>>
>> Patches on github [1].
>>
>> [1] https://github.com/al1img/xen/tree/xl-vdispl-v4
>
> So I just went through this series and pointed out issues I can
> immediately find. I will need to take a closer look at the framework
> itself next week.
ping
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v4 00/13] libxl: add PV display device driver interface
2017-08-17 10:13 ` Oleksandr Grytsov
@ 2017-08-17 11:11 ` Wei Liu
2017-08-30 15:49 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Wei Liu @ 2017-08-17 11:11 UTC (permalink / raw)
To: Oleksandr Grytsov, g; +Cc: xen-devel, Wei Liu, Ian Jackson, Oleksandr Grytsov
On Thu, Aug 17, 2017 at 01:13:39PM +0300, Oleksandr Grytsov wrote:
> On Fri, Jul 28, 2017 at 5:13 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > On Tue, Jul 18, 2017 at 05:25:17PM +0300, Oleksandr Grytsov wrote:
> >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
> >>
> >> Changes since V3:
> >> * 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 required to unreasonable extend
> >> libxl__device_add_async and its parameters;
> >> * disk device list changed to use libxl__device_list;
> >> * previous comments are applied.
> >>
> >> Patches on github [1].
> >>
> >> [1] https://github.com/al1img/xen/tree/xl-vdispl-v4
> >
> > So I just went through this series and pointed out issues I can
> > immediately find. I will need to take a closer look at the framework
> > itself next week.
>
> ping
I'm still waiting for the outcome from the other thread in which I
proposed to change p9 to p9s. Ian is out of office at the moment.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v4 00/13] libxl: add PV display device driver interface
2017-08-17 11:11 ` Wei Liu
@ 2017-08-30 15:49 ` Oleksandr Grytsov
2017-08-30 15:52 ` Ian Jackson
0 siblings, 1 reply; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-08-30 15:49 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson, g, Oleksandr Grytsov
On Thu, Aug 17, 2017 at 2:11 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> On Thu, Aug 17, 2017 at 01:13:39PM +0300, Oleksandr Grytsov wrote:
>> On Fri, Jul 28, 2017 at 5:13 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> > On Tue, Jul 18, 2017 at 05:25:17PM +0300, Oleksandr Grytsov wrote:
>> >> From: Oleksandr Grytsov <oleksandr_grytsov@epam.com>
>> >>
>> >> Changes since V3:
>> >> * 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 required to unreasonable extend
>> >> libxl__device_add_async and its parameters;
>> >> * disk device list changed to use libxl__device_list;
>> >> * previous comments are applied.
>> >>
>> >> Patches on github [1].
>> >>
>> >> [1] https://github.com/al1img/xen/tree/xl-vdispl-v4
>> >
>> > So I just went through this series and pointed out issues I can
>> > immediately find. I will need to take a closer look at the framework
>> > itself next week.
>>
>> ping
>
> I'm still waiting for the outcome from the other thread in which I
> proposed to change p9 to p9s. Ian is out of office at the moment.
ping
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread
* Re: [PATCH v4 00/13] libxl: add PV display device driver interface
2017-08-30 15:49 ` Oleksandr Grytsov
@ 2017-08-30 15:52 ` Ian Jackson
2017-08-31 9:01 ` Oleksandr Grytsov
0 siblings, 1 reply; 56+ messages in thread
From: Ian Jackson @ 2017-08-30 15:52 UTC (permalink / raw)
To: Oleksandr Grytsov; +Cc: xen-devel, Wei Liu, g, Oleksandr Grytsov
Oleksandr Grytsov writes ("Re: [PATCH v4 00/13] libxl: add PV display device driver interface"):
> On Thu, Aug 17, 2017 at 2:11 PM, Wei Liu <wei.liu2@citrix.com> wrote:
> > I'm still waiting for the outcome from the other thread in which I
> > proposed to change p9 to p9s. Ian is out of office at the moment.
>
> ping
Sorry, I thought the previous mails were clear. I think changing the
name is fine now, despite it being an API break, because we think
there are very few out-of-tree callers.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread* Re: [PATCH v4 00/13] libxl: add PV display device driver interface
2017-08-30 15:52 ` Ian Jackson
@ 2017-08-31 9:01 ` Oleksandr Grytsov
0 siblings, 0 replies; 56+ messages in thread
From: Oleksandr Grytsov @ 2017-08-31 9:01 UTC (permalink / raw)
To: Ian Jackson; +Cc: xen-devel, Wei Liu, g, Oleksandr Grytsov
On Wed, Aug 30, 2017 at 6:52 PM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:
> Oleksandr Grytsov writes ("Re: [PATCH v4 00/13] libxl: add PV display device driver interface"):
>> On Thu, Aug 17, 2017 at 2:11 PM, Wei Liu <wei.liu2@citrix.com> wrote:
>> > I'm still waiting for the outcome from the other thread in which I
>> > proposed to change p9 to p9s. Ian is out of office at the moment.
>>
>> ping
>
> Sorry, I thought the previous mails were clear. I think changing the
> name is fine now, despite it being an API break, because we think
> there are very few out-of-tree callers.
>
> Ian.
Yes, it is clear about changing the name.
By pinging I mean review and feedback for all other patches in this patch set.
--
Best Regards,
Oleksandr Grytsov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 56+ messages in thread