Re: [libvirt] [PATCH] fix virsh dominfo returns error when virNodeGetSecurityModel() is not supported.

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.