
Hi Dan, Daniel P. Berrange wrote:
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.
Ok, I see.
Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.210 diff -u -p -u -r1.210 virsh.c --- src/virsh.c 3 Jun 2009 12:13:52 -0000 1.210 +++ src/virsh.c 18 Jun 2009 11:14:44 -0000 @@ -1643,8 +1643,10 @@ cmdDominfo(vshControl *ctl, const vshCmd /* Security model and label information */ memset(&secmodel, 0, sizeof secmodel); if (virNodeGetSecurityModel(ctl->conn,&secmodel) == -1) { - virDomainFree(dom); - return FALSE; + if (last_error->code != VIR_ERR_NO_SUPPORT) { + virDomainFree(dom); + return FALSE; + } } else { /* Only print something if a security model is active */ if (secmodel.model[0] != '\0') {
Don't check last_error->code of virDomainGetSecurityLabel()? should check the same as virNodeGetSecurityModel(). Thanks Tatsuro Enokura