Re: [Xen-devel] [PATCH] libxl: add support for booting PV domains from NetBSD using raw files as disks. Fixed the shutdown race problem by checking "hotplug-status" instead of "state" xenstore variable in NetBSD

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2011-09-15 at 09:03 -0400, Roger Pau Monne wrote:
> # HG changeset patch
> # User Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> # Date 1316091721 -7200
> # Node ID 015617579cd36fc58318aaf350bec5f7cc07ef2f
> # Parent  63e254468d6e8d81baeafaed68f05791dc21eb4e
> libxl: add support for booting PV domains from NetBSD using raw files
> as disks. Fixed the shutdown race problem by checking "hotplug-status"
> instead of "state" xenstore variable in NetBSD.

This was one long line which mercurial will use as a summary, it's a
good idea to make sure that the first line stands somewhat alone as a
summary so the e.g. "hg log" is useful.

Also this sounds on the face of it like two bugfixes, if that's the case
they should be submitted separately.

> 
> Signed-off-by: Roger Pau Monne <roger.pau@xxxxxxxxxxxxx>
> 
> diff -r 63e254468d6e -r 015617579cd3 tools/hotplug/NetBSD/block
> --- a/tools/hotplug/NetBSD/block      Wed Sep 14 14:18:40 2011 +0200
> +++ b/tools/hotplug/NetBSD/block      Thu Sep 15 15:02:01 2011 +0200
> @@ -19,7 +19,7 @@ error() {
>  
>  xpath=$1
>  xstatus=$2
> -xtype=$(xenstore-read "$xpath/type")
> +xtype=$3
>  xparams=$(xenstore-read "$xpath/params")
>  
>  case $xstatus in
> @@ -38,6 +38,8 @@ 6)
>               echo "unknown type $xtype" >&2
>               ;;
>       esac
> +     echo xenstore-write $xpath/hotplug-status disconnected

leftover debugging?

> +     xenstore-write $xpath/hotplug-status disconnected
>       xenstore-rm $xpath
>       exit 0
>       ;;
>      libxl_ctx *ctx = libxl__gc_owner(gc);
>      xs_transaction_t t;
>      char *state_path = libxl__sprintf(gc, "%s/state", be_path);
> +    char *hotplug_path = libxl__sprintf(gc, "%s/hotplug-status", be_path);
>      char *state = libxl__xs_read(gc, XBT_NULL, state_path);
> +    char *hotplug = libxl__xs_read(gc, XBT_NULL, hotplug_path);
>      int rc = 0;
>  
>      if (!state)
>          goto out;
> +
> +#ifdef HAVE_PHY_BACKEND_FILE_SUPPORT
> +    if (!strstr(be_path, "vbd")) {
> +        if (atoi(state) != 4) {
> +            xs_rm(ctx->xsh, XBT_NULL, be_path);
> +            goto out;
> +        }
> +    } else {
> +        if (!hotplug)
> +            goto out;
> +        if (!strcmp(hotplug, "disconnected")) {
> +            xs_rm(ctx->xsh, XBT_NULL, be_path);
> +            goto out;
> +        }
> +    }

Do the other backend types also write this node? It looks like Linux
does, at least in some circumstances (tap and vbd AFAICT), in which case
perhaps this is suitable as the only test here (i.e. drop the #ifdef and
the #else case). That would remove a lot of the conditional code in this
patch.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel