This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH] Clear device-model information when destroying a

To: "Jun Zhu (Intern)" <Jun.Zhu@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] Clear device-model information when destroying an HVM domain with stubdom
From: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Date: Wed, 8 Sep 2010 14:51:18 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 08 Sep 2010 06:52:18 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <433DDF91DFB08148BAD3FDB6FDDA314C9F35F3BB58@xxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Newsgroups: chiark.mail.xen.devel
References: <433DDF91DFB08148BAD3FDB6FDDA314C9F35F3BB58@xxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Jun Zhu (Intern) writes ("[Xen-devel] [PATCH] Clear device-model information 
when destroying an HVM domain with stubdom"):
> This patch clears the device-model information from xenstore when
> destroying an HVM domain with stubdom.

I don't think this patch is correct.

> +     xs_rm(ctx->xsh, XBT_NULL, libxl_sprintf(ctx, 
> "/local/domain/0/device-model/%d", domid));
> +

This removes the information from xenstore before the process is
killed.  These things should be done in the opposite order, so that we
are never (even briefly) in a situation with a dm (whether stubdom or
dom0 process) which is not mentioned in xenstore.

The code already has this bug in the dom0 process case.

>      pid = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, 
> "/local/domain/%d/image/device-model-pid", domid));
>      if (!pid) {
>          int stubdomid = libxl_get_stubdom_id(ctx, domid);
> @@ -844,7 +846,6 @@
>          XL_LOG(ctx, XL_LOG_ERROR, "Device model is a stubdom, domid=%d\n", 
> stubdomid);
>          return libxl_domain_destroy(ctx, stubdomid, 0);
>      }
> -    xs_rm(ctx->xsh, XBT_NULL, libxl_sprintf(ctx, 
> "/local/domain/0/device-model/%d", domid));
>      ret = kill(atoi(pid), SIGHUP);
>      if (ret < 0 && errno == ESRCH) {

The right answer is probably to move the xs_rm until _after_ the kill
is dealt with, and to put the kill section in an else branch of the
if, and remove the return in the stubdom case and replace it with an
ordinary error check.


Xen-devel mailing list