|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 01/13] libxl: add generic function to add device
On Tue, Jul 18, 2017 at 05:25:18PM +0300, Oleksandr Grytsov wrote:
> From: Oleksandr Grytsov <oleksandr_grytsov@xxxxxxxx>
>
> 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@xxxxxxxx>
> ---
> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |