On Mon, 26 Jul 2010, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1280140562 -3600
> # Node ID 1e0b63948031587b958ea307410d19c7b2be9614
> # Parent f6300d42a667cf6a1a02fc065ecd9eaea0e10ecc
> libxl: signal caller if domain already destroyed on domain death event
>
> Currently libxl_event_get_domain_death_info returns 0 if the event was
> not a domain death event and 1 if it was but does not infom the user
> if someone else has already cleaned up the domain, which means the
> caller must replicate some of the logic from within libxl.
>
> Instead have the libxl_event_get_XXX_info functions required that the
> event is of the right type (the caller must have recently switched on
> event->type anyway).
>
> This allows the return codes to be used in an event specific way and
> we take advantage of this by returning an error from
> libxl_event_get_domain_death_info if the domain is not dying.
>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>
> diff -r f6300d42a667 -r 1e0b63948031 tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c Mon Jul 26 11:36:02 2010 +0100
> +++ b/tools/libxl/libxl.c Mon Jul 26 11:36:02 2010 +0100
> @@ -721,54 +721,54 @@ int libxl_free_waiter(libxl_waiter *wait
> return 0;
> }
>
> +/*
> + * Returns:
> + * - 0 if the domain is dead and there is no cleanup to be done. e.g
> because someone else has already done it.
> + * - 1 if the domain is dead and there is cleanup to be done.
> + *
> + * Can return error if the domain exists and is still running
> + */
> int libxl_event_get_domain_death_info(struct libxl_ctx *ctx, uint32_t domid,
> libxl_event *event, struct libxl_dominfo *info)
> {
> - int rc = 0, ret;
> - if (event && event->type == LIBXL_EVENT_DOMAIN_DEATH) {
> - ret = libxl_domain_info(ctx, info, domid);
> + if (libxl_domain_info(ctx, info, domid) < 0)
> + return 0;
>
> - if (ret == 0 && info->domid == domid) {
> - if (info->running || (!info->shutdown && !info->dying &&
> !info->crashed))
> - goto out;
> - rc = 1;
> - goto out;
> - }
> - memset(info, 0, sizeof(*info));
> - rc = 1;
> - goto out;
> - }
> -out:
> - return rc;
> + if (info->running || (!info->shutdown && !info->dying && !info->crashed))
> + return ERROR_INVAL;
> +
> + return 1;
> }
>
> +/*
> + * Returns true and fills *disk if the caller should eject the disk
> + */
> int libxl_event_get_disk_eject_info(struct libxl_ctx *ctx, uint32_t domid,
> libxl_event *event, libxl_device_disk *disk)
> {
> - if (event && event->type == LIBXL_EVENT_DISK_EJECT) {
> - char *path;
> - char *backend;
> - char *value = libxl_xs_read(ctx, XBT_NULL, event->path);
> + char *path;
> + char *backend;
> + char *value;
>
> - if (!value || strcmp(value, "eject"))
> - return 0;
> + value = libxl_xs_read(ctx, XBT_NULL, event->path);
>
> - path = strdup(event->path);
> - path[strlen(path) - 6] = '\0';
> - backend = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx,
> "%s/backend", path));
> + if (!value || strcmp(value, "eject"))
> + return 0;
>
> - disk->backend_domid = 0;
> - disk->domid = domid;
> - disk->physpath = NULL;
> - disk->phystype = 0;
> - /* this value is returned to the user: do not free right away */
> - disk->virtpath = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx,
> "%s/dev", backend));
> - disk->unpluggable = 1;
> - disk->readwrite = 0;
> - disk->is_cdrom = 1;
> + path = strdup(event->path);
> + path[strlen(path) - 6] = '\0';
> + backend = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/backend",
> path));
>
> - free(path);
> - return 1;
> - }
> - return 0;
> + disk->backend_domid = 0;
> + disk->domid = domid;
> + disk->physpath = NULL;
> + disk->phystype = 0;
> + /* this value is returned to the user: do not free right away */
> + disk->virtpath = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx,
> "%s/dev", backend));
> + disk->unpluggable = 1;
> + disk->readwrite = 0;
> + disk->is_cdrom = 1;
> +
> + free(path);
> + return 1;
> }
>
> static int libxl_destroy_device_model(struct libxl_ctx *ctx, uint32_t domid)
> diff -r f6300d42a667 -r 1e0b63948031 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c Mon Jul 26 11:36:02 2010 +0100
> +++ b/tools/libxl/xl_cmdimpl.c Mon Jul 26 11:36:02 2010 +0100
> @@ -1338,7 +1338,6 @@ start:
> struct libxl_dominfo info;
> libxl_event event;
> libxl_device_disk disk;
> - memset(&info, 0x00, sizeof(xc_domaininfo_t));
>
> FD_ZERO(&rfds);
> FD_SET(fd, &rfds);
> @@ -1349,12 +1348,17 @@ start:
> libxl_get_event(&ctx, &event);
> switch (event.type) {
> case LIBXL_EVENT_DOMAIN_DEATH:
> - if (libxl_event_get_domain_death_info(&ctx, domid, &event,
> &info)) {
> - LOG("Domain %d is dead", domid);
> - if (info.crashed || info.dying || (info.shutdown &&
> info.shutdown_reason != SHUTDOWN_suspend)) {
> + ret = libxl_event_get_domain_death_info(&ctx, domid, &event,
> &info);
> +
> + if (ret < 0) continue;
> +
> + LOG("Domain %d is dead", domid);
> +
> + if (ret) {
> + if (info.shutdown_reason != SHUTDOWN_suspend) {
> LOG("Domain %d needs to be clean: destroying the
> domain", domid);
> libxl_domain_destroy(&ctx, domid, 0);
> - if (info.shutdown && info.shutdown_reason ==
> SHUTDOWN_reboot) {
> + if (info.shutdown_reason == SHUTDOWN_reboot) {
Isn't still possible to get here and have info.shutdown == 0 (and even
info.dying == 0, after reading the fourth patch)?
If so, the previous test is probably clearer.
The rest of the series looks fine.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home