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

[Daniel Veillard]

On Thu, May 07, 2009 at 07:07:17PM +0900, Tatsuro Enokura wrote:
> Hi Dan & Daniel,

  Hi Tatsuro,

> Daniel Veillard wrote:
> >>> The form of cpumap argument in op_pincpu method on xend change
> >>> by following a patch.
> >>>    http://xenbits.xensource.com/xen-unstable.hg?rev/a63d20d7a941
> >>>
> >>> xenDaemonDomainPinCpu() sends the string for cpumap argument
> >>> like python's list style now.
> >>> This patch is following the change of cpumap argument.
> >>>    "[0,1,2]"    --->    "0,1,2"
> >> If we switch to the new format, does that still work with older
> >> XenD ?
> >
> >    Yup I had the same question when reading the description, if not
> > we need to make that conditional on xen version.
> 
> The older xend's behavior is not affected change the cpumap
> format, because the older xend works incorrectly by the xend's
> cpumap format bug.
> The new xend(no cpumap format bug) and the current libvirt
> cpumap format occurred error.
> The new xend and the new libvirt cpumap format works fine.
> 
> Therefor, I think that we don't need to make that conditional
> on xen version.
> 

  Thanks a lot for going through all those test, and bringing the
results !

> 
> The test result of verifying the xend and the cpumap format is
> described as follows.
> 
> * 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

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/