RE: [Xen-devel] [PATCH] qemu-xen: fix cpu hotplug

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

Ian Jackson wrote:
> Stefano Stabellini writes ("[Xen-devel] [PATCH] qemu-xen: fix cpu
> hotplug"): 
>> qemu-xen: fix cpu hotplug
>> 
>> The current xenstore watch path for a vcpu-set event is wrong and is
>> also wrong the code to parse it.  This patch fixes both of them:
> 
> Thanks.  So it seems you're saying it's completely broken in
> xen-unstable.

Stabellini, I read your attached patch, it's OK. 
In fact, we firstly implemented xenstore watch by same scheme of your patch, 
watching each cpu node status:
/local/domain/xx/cpu/yy/availability=offline (online)

However, we finally didn't use this scheme. We watch 'common' node instead:
/local/domain/xx/cpu
in this way, only 1 watch point need. 
Considering vcpu number may become more and more in the future (say, more than 
128), it's more simple and reasonable.
(Watches can be set at points in the hierarchy and an individual watch will be 
triggered when anything at or below that point in the hierachy changes)


> 
> I have CC'd a bunch of Intel folks who were doing some other
> work on cpu hotplug.  They were dealing with a race when multiple CPUs
> were added at once.
> 
> So I think there must be some confusion.  Perhaps xl and xend have
> different ideas about what the xenstore syntax is for these
> operations ? 
> 
> Jinsong Liu et al: would you care to comment ?  Particularly about
> this: 
> 
>> A xenstore vcpu hotplug command is of the following form:
>> path: /local/domain/DOMID/cpu/VCPU_NUMBER/availability
>> values: "online" or "offline"

Yes, I think there must be some confusion.
Currently 'xm vcpu-set' command works fine with both PV and HVM vcpu hotplug.

Stabellini/Jackson, would your please tell me what xl recently happened for 
vcpu hotplug?
is there any different ideas xl and xend about xenstore syntax?
(Each time 'xm vcpu-set' executed, xend will write all xenstore cpu node status)

> 
> That doesn't seem to match up with what's in xend, which seems to
> write "cpu_avail" in the vm tree.
> 
> Thanks,
> Ian.

--- Begin Message ---

To:	"xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject:	[Xen-devel] [PATCH] qemu-xen: fix cpu hotplug
From:	Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date:	Thu, 2 Sep 2010 21:17:20 +0800
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>
Sender:	"xen-devel-bounces@xxxxxxxxxxxxxxxxxxx" <xen-devel-bounces@xxxxxxxxxxxxxxxxxxx>
Thread-index:	ActKoUm9oPN9mduXT1GStxYH0p6y0A==
Thread-topic:	[Xen-devel] [PATCH] qemu-xen: fix cpu hotplug
User-agent:	Alpine 2.00 (DEB 1167 2008-08-23)

qemu-xen: fix cpu hotplug

The current xenstore watch path for a vcpu-set event is wrong and is
also wrong the code to parse it.
This patch fixes both of them: a xenstore vcpu hotplug command is of the
following form:

path: /local/domain/DOMID/cpu/VCPU_NUMBER/availability
values: "online" or "offline"

Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

diff --git a/xenstore.c b/xenstore.c
index 6d24613..2c325ad 100644
--- a/xenstore.c
+++ b/xenstore.c
@@ -680,9 +680,12 @@ void xenstore_parse_domain_config(int hvm_domid)
     }
 
     /* Set a watch for vcpu-set */
-    if (pasprintf(&buf, "/local/domain/%u/cpu", domid) != -1) {
-        xs_watch(xsh, buf, "vcpu-set");
-        fprintf(logfile, "Watching %s\n", buf);
+    for (i = 0; i < vcpus; i++) {
+        if (pasprintf(&buf, "/local/domain/%u/cpu/%u/availability",
+                    domid, i) != -1) {
+            xs_watch(xsh, buf, "vcpu-set");
+            fprintf(logfile, "Watching %s\n", buf);
+        }
     }
 
     /* no need for ifdef CONFIG_STUBDOM, since in the qemu case
@@ -970,15 +973,14 @@ void xenstore_record_dm_state(const char *state)
 static void xenstore_process_vcpu_set_event(char **vec)
 {
     char *act = NULL;
-    char *vcpustr, *node = vec[XS_WATCH_PATH];
-    unsigned int vcpu, len;
+    char *node = vec[XS_WATCH_PATH];
+    unsigned int vcpu, len, domid;
 
-    vcpustr = strstr(node, "cpu/");
-    if (!vcpustr) {
-        fprintf(stderr, "vcpu-set: watch node error.\n");
+    if (sscanf(node, "/local/domain/%u/cpu/%u/availability",
+                &domid, &vcpu) <= 0) {
+        fprintf(stderr, "vcpu-set: watch node error, path=%s\n", node);
         return;
     }
-    sscanf(vcpustr, "cpu/%u", &vcpu);
 
     act = xs_read(xsh, XBT_NULL, node, &len);
     if (!act) {

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

--- End Message ---

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