[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Fix the style of argument("cpumap") in op_pincpu()



Hi Daniel,

Daniel Veillard wrote:
>> >> * xen-unstable
>> >> Set cpu affinity used by xenDaemonDomainPinCpu():
>> >> # set xenHypervisorPinVcpu() always return -1 for test
>> >>           | libvirt             | libvirt
>> >>           | current format      | new format
>> >>           +----------+----------+----------+----------
>> >>           | old xend | new xend | old xend | new xend
>> >> ---------+----------+----------+----------+----------
>> >> inactive |    (1)   |   (3)    |   (5)    |    (7)
>> >>    domain |    NG1   |   NG2    |   NG1    |    OK
>> >> ---------+----------+----------+----------+----------
>> >>    active |    (2)   |   (4)    |   (6)    |    (8)
>> >>    domain |    NG1   |   NG2    |   NG1    |    OK
>> >>
>> >> old xend: before xen-unstable changeset 19579
>> >> new xend: after  xen-unstable changeset 19580
>> >>
>> >> OK : virsh command end normaly and
>> >>       set cpu affinity.
>> >> NG1: virsh command end normaly, but
>> >>       can't set cpu affinity.
>> >> NG2: virsh command end with show error msg.
>> >>
>> >> Result (1),(2) is the same as result (5),(6).
> >
> >    Okay, but I'm still worrying.
> > The fact that we fail to detect the error NG1 is a bug, and that
> > bug should be fixed. Seems to me the change may just replace one error
> > by another one but in the end we should instead aim at fixing the NG1
> > problem with the old format, not substituing it with something else.
> >
> >    I would prefer if the patch did some checking about the current xend
> > version running, but unfortunately priv->xendConfigVersion won't be
> > precise enough.
> >
> >    Sorry I don't know how to handle this more correctly right now

I see.
priv->xendConfigVersion isn't appropriate for tracing the xend's
version. We should request to the xen community for new interface
of to get the xend's version.

On other hand,
the new xend and libvirt without the cpumap patch occur error(NG2).
Behavior of the old xend and libvirt with the cpumap patch is the same
behavior of libvirt without the cpumap patch.
Moreover, there is no work for libvirt of this issue any further at present.

I make the patch that added the foregoing content as TODO comment.

Signed-off-by: Tatsuro Enokura <fj2026af aa jp fujitsu com>

Thanks,
Tatsuro Enokura



Index: src/xend_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xend_internal.c,v
retrieving revision 1.261
diff -u -r1.261 xend_internal.c
--- src/xend_internal.c	7 May 2009 07:27:49 -0000	1.261
+++ src/xend_internal.c	8 May 2009 09:41:42 -0000
@@ -3765,6 +3765,13 @@
  * @maplen: length of cpumap in bytes
  *
  * Dynamically change the real CPUs which can be allocated to a virtual CPU.
+ * TODO: When we use libvirt,
+ *       the XenD cpu affinity works on after cset 19579.
+ *       But we cannot detect the XenD version precisely.
+ *       This is because XenD does not have precise version info I/F.
+ *       We need to request precise XenD version detection I/F
+ *       to the Xen community.
+ *       After that, we need to discriminate the XenD version.
  *
  * Returns 0 for success; -1 (with errno) on error
  */
@@ -3772,7 +3779,7 @@
 xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu,
                      unsigned char *cpumap, int maplen)
 {
-    char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64] = "[";
+    char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64] = "";
     int i, j;
 
     if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)
@@ -3788,7 +3795,7 @@
         snprintf(buf, sizeof(buf), "%d,", (8 * i) + j);
         strcat(mapstr, buf);
     }
-    mapstr[strlen(mapstr) - 1] = ']';
+    mapstr[strlen(mapstr) - 1] = '\0';
     snprintf(buf, sizeof(buf), "%d", vcpu);
     return(xend_op(domain->conn, domain->name, "op", "pincpu", "vcpu", buf,
                   "cpumap", mapstr, NULL));

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]