
On Thu, Jun 18, 2009 at 12:20:16PM +0100, Daniel P. Berrange wrote:
On Thu, Jun 18, 2009 at 06:08:36PM +0900, Tatsuro Enokura wrote:
Hi all
I try virsh dominfo in upstream libvirt on xen machine, the commands returns -1 as follows:
Opps, thanks for reporting this problem - surprised it slipped past our testing for so long !
[root@vmi20 ~]# virsh dominfo rhel53rc2_pv_sdb3 Id: 1 Name: rhel53rc2_pv_sdb3 UUID: 05ba9be8-f4e9-e208-11c7-fc936655cd8e OS Type: linux State: idle CPU(s): 2 CPU time: 8.8s Max memory: 1048576 kB Used memory: 716800 kB Autostart: disable error: this function is not supported by the hypervisor: virNodeGetSecurityModel
[root@vmi20 ~]# echo $? 1
The explanation of virNodeGetSecurityModel() and virNodeGetSecurityModel() in libvirt.c is return -2 when hypervisor drivers don't support these operations. But these functions return -1 in this case, and so cmdDominfo() in virsh.c returns FALSE.
This API description about returning -1 vs -2 is totally bogus. With the remote driver we only have a boolean success vs fail status, so there is no way to return 2 different error codes. In addition already have a way to report methods which are not supported, by giving back a VIR_ERR_NO_SUPPORT code, so there is no need for a special '-2' value in any case.
I make a patch. - virNodeGetSecurityModel() and virNodeGetSecurityModel() return -2 when drivers don't supprted these operations. - In CmdDominfo(), it is no operation when virNodeGetSecurityModel() and virNodeGetSecurityModel() return -2.
I'm attaching a alternate patch which just checks for the VIR_ERR_NO_SUPPORT code and simply ignores that error. This should deal with the error scenario you saw with Xen.
I'm also fixing the API description to match reality and adding in several missing 'memset()' calls, because the drivers should not assume the caller has zero'd these structs.
Looks fine, ACK, 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/