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] xend, sparse: move vcpu-hotplug to xenbus/store

To: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] xend, sparse: move vcpu-hotplug to xenbus/store
From: Ryan Harper <ryanh@xxxxxxxxxx>
Date: Mon, 8 Aug 2005 09:14:21 -0500
Cc: Ryan Harper <ryanh@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 08 Aug 2005 14:12:51 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1123317594.17152.23.camel@xxxxxxxxxxxxxxxxxxxxx>
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/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <20050805192156.GJ7538@xxxxxxxxxx> <1123317594.17152.23.camel@xxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.6+20040907i
* Rusty Russell <rusty@xxxxxxxxxxxxxxx> [2005-08-08 08:58]:
> On Fri, 2005-08-05 at 14:21 -0500, Ryan Harper wrote:
> > +/* xenbus watch struct */
> > +static struct xenbus_watch cpus_watch = {
> > +   .node = "cpus",
> > +   .callback = handle_cpus_watch,
> > +};
> Everywhere else we've used singular
> names: /domain, /domain/<uuid>/device, /tool, /domain/<uuid>/backend.
> So I'd suggest either using "cpu" or consider putting this under
> "device" and actually using the xenbus system, although I'm not sure
> that's all that useful for you.  Perhaps revisit after Christian has
> done latest merge.


> > +static void handle_cpus_watch(struct xenbus_watch *watch, const char *node)
> Just a preference, I prefer the watch callback to be something like
> "handle_XXX_event", which seems clearer to me.


> > +   /* get a pointer to start of cpus/cpu string */
> > +   if ((cpustr = strstr(node, "cpus/cpu")) != NULL) {
> > +
> > +           /* find which cpu state changed, note vcpu for handler */
> > +           sscanf(cpustr, "cpus/cpu%d", &cpu);
> Please don't repeat "cpu", just use "cpu/0", "cpu/1" which matches what
> block interfaces are doing.
> The guidelines I've been working on with store layout are as follows (no
> doubt we'll all come up with more as we use this thing):
> 1) Redundant information in the store is bad.  Don't put information in
> two places then insist they be the same.
> 2) Multiple entries should be in directory names, unless the hierarchy
> is getting completely out of control.
> 3) Directory names have to be unique, obviously.  If there is a useful
> handle to use, do so, otherwise simply use numbers starting from 0,
> which makes it fairly clear to the reader the tags are arbitrary.
> I'm still not sure about binary flags.  For the block devices, Christian
> and I chose "read-only": if it exists, the device is read only, if not,
> it's read-write.  That's preferable to a "writability" flag which
> contains either "read-only" or "read-write", since the default is
> fairly-clear.

Good to know.

> In the case of online vs. offline cpus, you could either have an
> "offline" entry (not existing meaning "online", which is probably the
> clearer default), or a "availability" which contains the string "online"
> or "offline".  Please do not have a node called "online" which contains
> 0 to mean "NOT"!

In this case, I was just following (with the exception of cpus) the
sysfs example which already exists:


If we don't care that the xenbus entries _do_ the exact same thing, but
aren't similarly named, I'm fine with that.

Just to be clear, it sounds like you want:

/domain/uuid/cpu/0/online  # cpu 0 is online
/domain/uuid/cpu/1/offline # cpu 1 is offline
/domain/uuid/cpu/2/online  # etc.
/domain/uuid/cpu/3/online  # etc.

I don't like that.  What about this?


and the values can be "online|offline"

I rather like having a single target whose value changes rather than
two entries one of which I must remove when the state changes.  In this
case, I can keep a single entry but toss the online=0 stuff you

Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
(512) 838-9253   T/L: 678-9253

Xen-devel mailing list