On Tue, 2010-10-05 at 14:39 +0100, Stefano Stabellini wrote:
> 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.
Yes, that's a good idea.
>
>
>
> 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
|