[libvirt] [PATCH] Fix the style of argument("cpumap") in op_pincpu()
Hi all, 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" Signed-off-by: Tatsuro Enokura <fj2026af@aa.jp.fujitsu.com> # my e-mail address has changed. Thanks Tatsuro Enokura
On Thu, Apr 30, 2009 at 04:00:54PM +0900, Tatsuro Enokura wrote:
Hi all,
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 ? Daniel
Index: src/xend_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xend_internal.c,v retrieving revision 1.260 diff -u -r1.260 xend_internal.c --- src/xend_internal.c 24 Apr 2009 12:17:50 -0000 1.260 +++ src/xend_internal.c 30 Apr 2009 01:40:59 -0000 @@ -3760,7 +3760,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) @@ -3776,7 +3776,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));
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Wed, May 06, 2009 at 09:53:46AM +0100, Daniel P. Berrange wrote:
On Thu, Apr 30, 2009 at 04:00:54PM +0900, Tatsuro Enokura wrote:
Hi all,
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. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Hi Dan & Daniel, 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. 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). * F8 Set cpu affinity used by xenDaemonDomainPinCpu(): # set xenHypervisorPinVcpu() always return -1 for test | libvirt | libvirt | current format | new format ---------+-------------------+------------------ inactive | (1) | (3) domain | NG2 | NG2 ---------+-------------------+------------------ active | (2) | (4) domain | NG1 | NG1 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 (3),(4). * RHEL5.3 Set cpu affinity used by xenDaemonDomainPinCpu(): # set xenHypervisorPinVcpu() and XmDomainPinVcpu() # always return -1 for test | libvirt | libvirt | current format | new format ---------+-------------------+------------------ inactive | (1) | (3) domain | NG2 | NG2 ---------+-------------------+------------------ active | (2) | (4) domain | NG1 | NG1 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 (3),(4). # The each command log is attached. Thakns, Tatsuro Enokura
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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
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
On Fri, May 08, 2009 at 06:45:59PM +0900, Tatsuro Enokura wrote:
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>
Okay, here is my suggested patch to minimize impact on old xen setups maybe someone locally fixed the xend side, and we should at least try to not break it on old setups. So I updated the comment And activate the change only for version of xend >= 3 which won't cover the full range but should still protect some of the older setups, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, May 12, 2009 at 10:50:02AM +0200, Daniel Veillard wrote:
On Fri, May 08, 2009 at 06:45:59PM +0900, Tatsuro Enokura wrote:
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>
Okay, here is my suggested patch to minimize impact on old xen setups maybe someone locally fixed the xend side, and we should at least try to not break it on old setups. So I updated the comment And activate the change only for version of xend >= 3 which won't cover the full range but should still protect some of the older setups,
Urgh, I wasn't fully awake, that one does compile :-\ ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Hi Daniel, Daniel Veillard wrote:
Okay, here is my suggested patch to minimize impact on old xen setups maybe someone locally fixed the xend side, and we should at least try to not break it on old setups. So I updated the comment And activate the change only for version of xend>= 3 which won't cover the full range but should still protect some of the older setups,
Urgh, I wasn't fully awake, that one does compile :-\ !
Thank you! this patch works fine for xend-unstable. Thanks, Tatsuro Enokura
On Wed, May 13, 2009 at 11:48:35AM +0900, Tatsuro Enokura wrote:
Hi Daniel,
Daniel Veillard wrote:
Okay, here is my suggested patch to minimize impact on old xen setups maybe someone locally fixed the xend side, and we should at least try to not break it on old setups. So I updated the comment And activate the change only for version of xend>= 3 which won't cover the full range but should still protect some of the older setups,
Urgh, I wasn't fully awake, that one does compile :-\ !
Thank you! this patch works fine for xend-unstable.
Okidoc, commited, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Tatsuro Enokura