2011/9/15 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.
Well, it's not a bugfix, because NetBSD was not supported by libxl, so
I think it's just part of the patch to add support for NetBSD to
libxl. I will split the line, and the patch if it's necessary, but one
doesn't make sense without the other, and I don't really know what
would be in each patch.
>>
>> 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?
The block NetBSD script contains some echos, so I feel it would be
interesting to add this one too.
>> + 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.
If Linux also uses this, I will have to modify the Linux block script,
so that it sets hotplug-status to "disconnected" also. This will be
nice, I don't like code with #ifdefs as it tends to get messy and it's
not easy to understand.
Regards, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home