Re: [Xen-devel] [PATCH 14 of 23] libxl: use libxl__device in libxl_devices_destroy and libxl__device_pci_remove_xenstore

[Roger Pau Monnà <roger.pau@xxxxxxxxxxxxx>]

2011/9/30 Ian Campbell <ian.campbell@xxxxxxxxxx>:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1317389248 -3600
> # Node ID ec28ee6dace513c3d009d498341f9537a19b2d98
> # Parent Âe5a70a3b61a1acfa7d73db548a591486024d01b7
> libxl: use libxl__device in libxl_devices_destroy and 
> libxl__device_pci_remove_xenstore
>
> Doing this allows us to use the common functions for removing devices.
>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>
> diff -r e5a70a3b61a1 -r ec28ee6dace5 tools/libxl/libxl_device.c
> --- a/tools/libxl/libxl_device.c    ÂFri Sep 30 14:27:28 2011 +0100
> +++ b/tools/libxl/libxl_device.c    ÂFri Sep 30 14:27:28 2011 +0100
> @@ -49,6 +49,25 @@ char *libxl__device_backend_path(libxl__
> Â Â Â Â Â Â Â Â Â Â Â Â Â device->domid, device->devid);
> Â}
>
> +int libxl__parse_backend_path(libxl__gc *gc,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âconst char *path,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âlibxl__device *dev)
> +{
> + Â Â/* /local/domain/<domid>/backend/<kind>/<domid>/<devid> */
> + Â Âchar strkind[16]; /* Longest is actually "console" */
> + Â Âuint32_t domain;
> + Â Âint rc = sscanf(path, "/local/domain/%d/backend/%15[^/]/%d/%d",
> + Â Â Â Â Â Â Â Â Â Â&dev->backend_domid,
> + Â Â Â Â Â Â Â Â Â Âstrkind,
> + Â Â Â Â Â Â Â Â Â Â&domain,
> + Â Â Â Â Â Â Â Â Â Â&dev->backend_devid);
> +
> + Â Âif (rc != 4)
> + Â Â Â Âreturn ERROR_FAIL;
> +
> + Â Âreturn libxl__device_kind_from_string(strkind, &dev->backend_kind);
> +}
> +
> Âint libxl__device_generic_add(libxl__gc *gc, libxl__device *device,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âchar **bents, char **fents)
> Â{
> @@ -348,10 +367,11 @@ int libxl__device_disk_dev_number(const
> Â Â return -1;
> Â}
>
> -int libxl__device_remove(libxl__gc *gc, char *be_path)
> +int libxl__device_remove(libxl__gc *gc, libxl__device *dev)
> Â{
> Â Â libxl_ctx *ctx = libxl__gc_owner(gc);
> Â Â xs_transaction_t t;
> + Â Âchar *be_path = libxl__device_backend_path(gc, dev);
> Â Â char *state_path = libxl__sprintf(gc, "%s/state", be_path);
> Â Â char *state = libxl__xs_read(gc, XBT_NULL, state_path);
> Â Â int rc = 0;
> @@ -429,10 +449,12 @@ static int wait_for_dev_destroy(libxl__g
> Âint libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force)
> Â{
> Â Â libxl_ctx *ctx = libxl__gc_owner(gc);
> - Â Âchar *path, *be_path, *fe_path;
> + Â Âchar *path;
> Â Â unsigned int num1, num2;
> Â Â char **l1 = NULL, **l2 = NULL;
> Â Â int i, j, n_watches = 0;
> + Â Âlibxl__device dev;
> + Â Âlibxl__device_kind kind;
>
> Â Â path = libxl__sprintf(gc, "/local/domain/%d/device", domid);
> Â Â l1 = libxl__xs_directory(gc, XBT_NULL, path, &num1);
> @@ -445,22 +467,27 @@ int libxl__devices_destroy(libxl__gc *gc
> Â Â Â Â num1 = 0;
> Â Â }
> Â Â for (i = 0; i < num1; i++) {
> - Â Â Â Âif (!strcmp("vfs", l1[i]))
> + Â Â Â Âif (libxl__device_kind_from_string(l1[i], &kind))
> + Â Â Â Â Â Âcontinue;
> + Â Â Â Âif (kind == LIBXL__DEVICE_KIND_VBD)
> Â Â Â Â Â Â continue;

This should not be there, or hotplug scripts for VBD device types are
not called. I know Linux doesn't need to call hotplug scripts for VBD,
but NetBSD does. Should we add some kind of helper function or ifdef,
or is it safe to remove this and handle VBD devices normally?

> Â Â Â Â path = libxl__sprintf(gc, "/local/domain/%d/device/%s", domid, l1[i]);
> Â Â Â Â l2 = libxl__xs_directory(gc, XBT_NULL, path, &num2);
> Â Â Â Â if (!l2)
> Â Â Â Â Â Â continue;
> Â Â Â Â for (j = 0; j < num2; j++) {
> - Â Â Â Â Â Âfe_path = libxl__sprintf(gc, "/local/domain/%d/device/%s/%s", 
> domid, l1[i], l2[j]);
> - Â Â Â Â Â Âbe_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, 
> "%s/backend", fe_path));
> - Â Â Â Â Â Âif (be_path != NULL) {
> + Â Â Â Â Â Âpath = libxl__sprintf(gc, 
> "/local/domain/%d/device/%s/%s/backend",
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âdomid, l1[i], l2[j]);
> + Â Â Â Â Â Âpath = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, path));
> + Â Â Â Â Â Âif (path && libxl__parse_backend_path(gc, path, &dev) == 0) {
> + Â Â Â Â Â Â Â Âdev.domid = domid;
> + Â Â Â Â Â Â Â Âdev.kind = kind;
> + Â Â Â Â Â Â Â Âdev.devid = atoi(l2[j]);
> +
> Â Â Â Â Â Â Â Â if (force) {
> - Â Â Â Â Â Â Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, be_path);
> - Â Â Â Â Â Â Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, fe_path);
> - Â Â Â Â Â Â Â Â Â Âlibxl__device_destroy_tapdisk(gc, be_path);
> + Â Â Â Â Â Â Â Â Â Âlibxl__device_force_remove(gc, &dev);
> Â Â Â Â Â Â Â Â } else {
> - Â Â Â Â Â Â Â Â Â Âif (libxl__device_remove(gc, be_path) > 0)
> + Â Â Â Â Â Â Â Â Â Âif (libxl__device_remove(gc, &dev) > 0)
> Â Â Â Â Â Â Â Â Â Â Â Â n_watches++;
> Â Â Â Â Â Â Â Â }
> Â Â Â Â Â Â }
> @@ -468,14 +495,18 @@ int libxl__devices_destroy(libxl__gc *gc
> Â Â }
>
> Â Â /* 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, "")) {
> + Â Âpath = libxl__sprintf(gc, "/local/domain/%d/console/backend", domid);
> + Â Âpath = libxl__xs_read(gc, XBT_NULL, path);
> + Â Âif (path && strcmp(path, "") &&
> + Â Â Â Âlibxl__parse_backend_path(gc, path, &dev) == 0) {
> + Â Â Â Âdev.domid = domid;
> + Â Â Â Âdev.kind = LIBXL__DEVICE_KIND_CONSOLE;
> + Â Â Â Âdev.devid = 0;
> +
> Â Â Â Â if (force) {
> - Â Â Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, be_path);
> - Â Â Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, fe_path);
> + Â Â Â Â Â Âlibxl__device_force_remove(gc, &dev);
> Â Â Â Â } else {
> - Â Â Â Â Â Âif (libxl__device_remove(gc, be_path) > 0)
> + Â Â Â Â Â Âif (libxl__device_remove(gc, &dev) > 0)
> Â Â Â Â Â Â Â Â n_watches++;
> Â Â Â Â }
> Â Â }
> @@ -505,12 +536,9 @@ int libxl__device_del(libxl__gc *gc, lib
> Â{
> Â Â libxl_ctx *ctx = libxl__gc_owner(gc);
> Â Â struct timeval tv;
> - Â Âchar *backend_path;
> Â Â int rc;
>
> - Â Âbackend_path = libxl__device_backend_path(gc, dev);
> -
> - Â Ârc = libxl__device_remove(gc, backend_path);
> + Â Ârc = libxl__device_remove(gc, dev);
> Â Â if (rc == -1) {
> Â Â Â Â rc = ERROR_FAIL;
> Â Â Â Â goto out;
> diff -r e5a70a3b61a1 -r ec28ee6dace5 tools/libxl/libxl_internal.h
> --- a/tools/libxl/libxl_internal.h   ÂFri Sep 30 14:27:28 2011 +0100
> +++ b/tools/libxl/libxl_internal.h   ÂFri Sep 30 14:27:28 2011 +0100
> @@ -240,8 +240,10 @@ _hidden int libxl__device_generic_add(li
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âchar **bents, char **fents);
> Â_hidden char *libxl__device_backend_path(libxl__gc *gc, libxl__device 
> *device);
> Â_hidden char *libxl__device_frontend_path(libxl__gc *gc, libxl__device 
> *device);
> +_hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âlibxl__device *dev);
> Â_hidden int libxl__device_del(libxl__gc *gc, libxl__device *dev);
> -_hidden int libxl__device_remove(libxl__gc *gc, char *be_path);
> +_hidden int libxl__device_remove(libxl__gc *gc, libxl__device *dev);
> Â_hidden int libxl__device_force_remove(libxl__gc *gc, libxl__device *dev);
> Â_hidden int libxl__devices_destroy(libxl__gc *gc, uint32_t domid, int force);
> Â_hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char 
> *state);
> diff -r e5a70a3b61a1 -r ec28ee6dace5 tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c  Fri Sep 30 14:27:28 2011 +0100
> +++ b/tools/libxl/libxl_pci.c  Fri Sep 30 14:27:28 2011 +0100
> @@ -410,9 +410,15 @@ retry_transaction2:
> Â Â Â Â Â Â goto retry_transaction2;
>
> Â Â if (num == 1) {
> - Â Â Â Âchar *fe_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, 
> "%s/frontend", be_path));
> - Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, be_path);
> - Â Â Â Âxs_rm(ctx->xsh, XBT_NULL, fe_path);
> + Â Â Â Âlibxl__device dev;
> + Â Â Â Âif (libxl__parse_backend_path(gc, be_path, &dev) != 0)
> + Â Â Â Â Â Âreturn ERROR_FAIL;
> +
> + Â Â Â Âdev.domid = domid;
> + Â Â Â Âdev.kind = LIBXL__DEVICE_KIND_PCI;
> + Â Â Â Âdev.devid = 0;
> +
> + Â Â Â Âlibxl__device_force_remove(gc, &dev);
> Â Â Â Â return 0;
> Â Â }
>
>
> _______________________________________________
> 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