The patch is good but I think it would be probably cleaner to add a
function like:
libxl__get_fe_path(libxl__device_kinds kind, int devid)
that would be called by libxl__device_generic_add to figure out the fe
path, rather than adding yet another parameter to the function,
parameter that in most cases is NULL anyway.
On Mon, 4 Oct 2010, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1286207114 -3600
> # Node ID 9e0d9b5460dc1c3cad0f5576157ac10b4045b4de
> # Parent a2c40fa3bc6c8471e1988e445f449975cf6f3822
> libxl: remove console 0 backend directory from xenstore on devices destroy
>
> The first PV console device has an unusual frontend path for
> historical reasons (/local/domain/<domid>/console rather than
> /local/domain/<domid>/device/console/0).
>
> Therefore we need to check this additional frontend path as well as
> those under /local/domain/<domid>/device when tearing down all
> backends or else we miss the backend for the console.
>
> As part of this we need to ensure that the frontend directory has a
> valid link to the backend, currently this link does not exist.
>
> Fix this by allowing libxl__device_generic_add to take an optional
> frontend path suffix relative to the dompath allowing us to use the
> function even for setting up the special case frontend.
>
> This also fixes the link from backend to frontend which until now gave
> a dangling link to the normal device path instead of the exceptional
> one.
>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl.c Mon Oct 04 16:45:14 2010 +0100
> @@ -1897,7 +1897,7 @@ int libxl_device_disk_add(libxl_ctx *ctx
>
> libxl__device_generic_add(ctx, &device,
> libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> - libxl__xs_kvs_of_flexarray(&gc, front,
> foffset));
> + libxl__xs_kvs_of_flexarray(&gc, front,
> foffset), NULL);
>
> rc = 0;
>
> @@ -2047,7 +2047,7 @@ int libxl_device_nic_add(libxl_ctx *ctx,
>
> libxl__device_generic_add(ctx, &device,
> libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> - libxl__xs_kvs_of_flexarray(&gc, front,
> foffset));
> + libxl__xs_kvs_of_flexarray(&gc, front,
> foffset), NULL);
>
> /* FIXME: wait for plug */
> rc = 0;
> @@ -2232,7 +2232,7 @@ int libxl_device_net2_add(libxl_ctx *ctx
>
> libxl__device_generic_add(ctx, &device,
> libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> - libxl__xs_kvs_of_flexarray(&gc, front,
> foffset));
> + libxl__xs_kvs_of_flexarray(&gc, front,
> foffset), NULL);
>
> /* FIXME: wait for plug */
> rc = 0;
> @@ -2324,36 +2324,13 @@ int libxl_device_console_add(libxl_ctx *
> int libxl_device_console_add(libxl_ctx *ctx, uint32_t domid,
> libxl_device_console *console)
> {
> libxl__gc gc = LIBXL_INIT_GC(ctx);
> + char *frontend_path = NULL;
> flexarray_t *front;
> flexarray_t *back;
> unsigned int boffset = 0;
> unsigned int foffset = 0;
> libxl__device device;
> int rc;
> -
> - if (console->build_state) {
> - xs_transaction_t t;
> - char **ents = (char **) libxl__calloc(&gc, 11, sizeof(char *));
> - ents[0] = "console/port";
> - ents[1] = libxl__sprintf(&gc, "%"PRIu32,
> console->build_state->console_port);
> - ents[2] = "console/ring-ref";
> - ents[3] = libxl__sprintf(&gc, "%lu",
> console->build_state->console_mfn);
> - ents[4] = "console/limit";
> - ents[5] = libxl__sprintf(&gc, "%d", LIBXL_XENCONSOLE_LIMIT);
> - ents[6] = "console/type";
> - if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> - ents[7] = "xenconsoled";
> - else
> - ents[7] = "ioemu";
> - ents[8] = "console/output";
> - ents[9] = console->output;
> -retry_transaction:
> - t = xs_transaction_start(ctx->xsh);
> - libxl__xs_writev(&gc, t, libxl__xs_get_dompath(&gc, console->domid),
> ents);
> - if (!xs_transaction_end(ctx->xsh, t, 0))
> - if (errno == EAGAIN)
> - goto retry_transaction;
> - }
>
> front = flexarray_make(16, 1);
> if (!front) {
> @@ -2384,29 +2361,40 @@ retry_transaction:
> flexarray_set(back, boffset++, "protocol");
> flexarray_set(back, boffset++, LIBXL_XENCONSOLE_PROTOCOL);
>
> - /* if devid == 0 do not add the frontend to device/console/ because
> - * it has already been added to console/ */
> - if (device.devid > 0) {
> - flexarray_set(front, foffset++, "backend-id");
> - flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
> console->backend_domid));
> + flexarray_set(front, foffset++, "backend-id");
> + flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
> console->backend_domid));
> + flexarray_set(front, foffset++, "limit");
> + flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
> LIBXL_XENCONSOLE_LIMIT));
> + flexarray_set(front, foffset++, "type");
> + if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> + flexarray_set(front, foffset++, "xenconsoled");
> + else
> + flexarray_set(front, foffset++, "ioemu");
> + flexarray_set(front, foffset++, "output");
> + flexarray_set(front, foffset++, console->output);
> +
> + if (device.devid == 0) {
> + if (console->build_state == NULL) {
> + rc = ERROR_INVAL;
> + goto out_free;
> + }
> + frontend_path = "console";
> +
> + flexarray_set(front, foffset++, "port");
> + flexarray_set(front, foffset++, libxl__sprintf(&gc, "%"PRIu32,
> console->build_state->console_port));
> + flexarray_set(front, foffset++, "ring-ref");
> + flexarray_set(front, foffset++, libxl__sprintf(&gc, "%lu",
> console->build_state->console_mfn));
> + } else {
> flexarray_set(front, foffset++, "state");
> flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d", 1));
> - flexarray_set(front, foffset++, "limit");
> - flexarray_set(front, foffset++, libxl__sprintf(&gc, "%d",
> LIBXL_XENCONSOLE_LIMIT));
> flexarray_set(front, foffset++, "protocol");
> flexarray_set(front, foffset++, LIBXL_XENCONSOLE_PROTOCOL);
> - flexarray_set(front, foffset++, "type");
> - if (console->consback == LIBXL_CONSBACK_XENCONSOLED)
> - flexarray_set(front, foffset++, "xenconsoled");
> - else
> - flexarray_set(front, foffset++, "ioemu");
> - flexarray_set(front, foffset++, "output");
> - flexarray_set(front, foffset++, console->output);
> }
>
> libxl__device_generic_add(ctx, &device,
> libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> - libxl__xs_kvs_of_flexarray(&gc, front,
> foffset));
> + libxl__xs_kvs_of_flexarray(&gc, front, foffset),
> + frontend_path);
> rc = 0;
> out_free:
> flexarray_free(back);
> @@ -2461,7 +2449,7 @@ int libxl_device_vkb_add(libxl_ctx *ctx,
>
> libxl__device_generic_add(ctx, &device,
> libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> - libxl__xs_kvs_of_flexarray(&gc, front,
> foffset));
> + libxl__xs_kvs_of_flexarray(&gc, front,
> foffset), NULL);
> rc = 0;
> out_free:
> flexarray_free(back);
> @@ -2722,7 +2710,7 @@ int libxl_device_vfb_add(libxl_ctx *ctx,
>
> libxl__device_generic_add(ctx, &device,
> libxl__xs_kvs_of_flexarray(&gc, back, boffset),
> - libxl__xs_kvs_of_flexarray(&gc, front,
> foffset));
> + libxl__xs_kvs_of_flexarray(&gc, front,
> foffset), NULL);
> rc = 0;
> out_free:
> flexarray_free(front);
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl_device.c Mon Oct 04 16:45:14 2010 +0100
> @@ -40,10 +40,10 @@ static const char *string_of_kinds[] = {
> };
>
> int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
> - char **bents, char **fents)
> + char **bents, char **fents, char
> *frontend_path)
> {
> libxl__gc gc = LIBXL_INIT_GC(ctx);
> - char *dom_path_backend, *dom_path, *frontend_path, *backend_path;
> + char *dom_path_backend, *dom_path, *backend_path;
> xs_transaction_t t;
> struct xs_permissions frontend_perms[2];
> struct xs_permissions backend_perms[2];
> @@ -57,8 +57,12 @@ int libxl__device_generic_add(libxl_ctx
> dom_path_backend = libxl__xs_get_dompath(&gc, device->backend_domid);
> dom_path = libxl__xs_get_dompath(&gc, device->domid);
>
> - frontend_path = libxl__sprintf(&gc, "%s/device/%s/%d",
> - dom_path, string_of_kinds[device->kind],
> device->devid);
> + if (frontend_path)
> + frontend_path = libxl__sprintf(&gc, "%s/%s",
> + dom_path, frontend_path);
> + else
> + frontend_path = libxl__sprintf(&gc, "%s/device/%s/%d",
> + dom_path,
> string_of_kinds[device->kind], device->devid);
> backend_path = libxl__sprintf(&gc, "%s/backend/%s/%u/%d",
> dom_path_backend,
> string_of_kinds[device->backend_kind], device->domid, device->devid);
>
> @@ -326,6 +330,16 @@ int libxl__devices_destroy(libxl_ctx *ct
> }
> }
> }
> +
> + /* console 0 frontend directory is not under
> /local/domain/<domid>/device */
> + fe_path = libxl__sprintf(&gc, "/local/domain/%d/console", domid);
> + be_path = libxl__xs_read(&gc, XBT_NULL, libxl__sprintf(&gc,
> "%s/backend", fe_path));
> + if (be_path && strcmp(be_path, "")) {
> + if (libxl__device_destroy(ctx, be_path, force) > 0)
> + n_watches++;
> + flexarray_set(toremove, n++, libxl__dirname(&gc, be_path));
> + }
> +
> if (!force) {
> /* Linux-ism. Most implementations leave the timeout
> * untouched after select. Linux, however, will chip
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl_internal.h Mon Oct 04 16:45:14 2010 +0100
> @@ -171,7 +171,7 @@ _hidden int libxl__device_disk_dev_numbe
> _hidden int libxl__device_disk_dev_number(char *virtpath);
>
> _hidden int libxl__device_generic_add(libxl_ctx *ctx, libxl__device *device,
> - char **bents, char **fents);
> + char **bents, char **fents, char
> *frontend_path);
> _hidden int libxl__device_del(libxl_ctx *ctx, libxl__device *dev, int wait);
> _hidden int libxl__device_destroy(libxl_ctx *ctx, char *be_path, int force);
> _hidden int libxl__devices_destroy(libxl_ctx *ctx, uint32_t domid, int
> force);
> diff -r a2c40fa3bc6c -r 9e0d9b5460dc tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c Mon Oct 04 15:39:30 2010 +0100
> +++ b/tools/libxl/libxl_pci.c Mon Oct 04 16:45:14 2010 +0100
> @@ -280,7 +280,7 @@ static int libxl_create_pci_backend(libx
>
> libxl__device_generic_add(ctx, &device,
> libxl__xs_kvs_of_flexarray(gc, back, boffset),
> - libxl__xs_kvs_of_flexarray(gc, front, foffset));
> + libxl__xs_kvs_of_flexarray(gc, front, foffset),
> NULL);
>
> flexarray_free(back);
> flexarray_free(front);
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|