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: Opps, thanks for reporting this problem - surprised it slipped past our testing for so long ! 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'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. Regards, Daniel Index: src/libvirt.c =================================================================== RCS file: /data/cvs/libvirt/src/libvirt.c,v retrieving revision 1.214 diff -u -p -u -r1.214 libvirt.c --- src/libvirt.c 12 Jun 2009 13:20:13 -0000 1.214 +++ src/libvirt.c 18 Jun 2009 11:14:44 -0000 @@ -4386,11 +4386,11 @@ error: * @domain: a domain object * @seclabel: pointer to a virSecurityLabel structure * - * Extract security label of an active domain. + * Extract security label of an active domain. The 'label' field + * in the @seclabel argument will be initialized to the empty 2A+ * string if the domain is not running under a security model. * - * Returns 0 in case of success, -1 in case of failure, and -2 - * if the operation is not supported (caller decides if that's - * an error). + * Returns 0 in case of success, -1 in case of failure */ int virDomainGetSecurityLabel(virDomainPtr domain, virSecurityLabelPtr seclabel) @@ -4421,10 +4421,11 @@ virDomainGetSecurityLabel(virDomainPtr d * @conn: a connection object * @secmodel: pointer to a virSecurityModel structure * - * Extract the security model of a hypervisor. + * Extract the security model of a hypervisor. The 'model' field + * in the @secmodel argument may be initialized to the empty + * string if the driver has not activated a security model. * - * Returns 0 in case of success, -1 in case of failure, and -2 if the - * operation is not supported (caller decides if that's an error). + * Returns 0 in case of success, -1 in case of failure */ int virNodeGetSecurityModel(virConnectPtr conn, virSecurityModelPtr secmodel) Index: src/qemu_driver.c =================================================================== RCS file: /data/cvs/libvirt/src/qemu_driver.c,v retrieving revision 1.255 diff -u -p -u -r1.255 qemu_driver.c --- src/qemu_driver.c 16 Jun 2009 15:42:46 -0000 1.255 +++ src/qemu_driver.c 18 Jun 2009 11:14:44 -0000 @@ -3064,6 +3064,8 @@ static int qemudDomainGetSecurityLabel(v qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + memset(seclabel, 0, sizeof(*seclabel)); + if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); @@ -3121,7 +3123,7 @@ static int qemudNodeGetSecurityModel(vir qemuDriverLock(driver); if (!driver->securityDriver) { - ret = -2; + memset(secmodel, 0, sizeof (*secmodel)); goto cleanup; } Index: src/remote_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/remote_internal.c,v retrieving revision 1.157 diff -u -p -u -r1.157 remote_internal.c --- src/remote_internal.c 12 Jun 2009 13:20:13 -0000 1.157 +++ src/remote_internal.c 18 Jun 2009 11:14:44 -0000 @@ -2320,6 +2320,8 @@ remoteDomainGetSecurityLabel (virDomainP make_nonnull_domain (&args.dom, domain); memset (&ret, 0, sizeof ret); + memset (seclabel, 0, sizeof (*seclabel)); + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_SECURITY_LABEL, (xdrproc_t) xdr_remote_domain_get_security_label_args, (char *)&args, (xdrproc_t) xdr_remote_domain_get_security_label_ret, (char *)&ret) == -1) { @@ -2353,6 +2355,8 @@ remoteNodeGetSecurityModel (virConnectPt remoteDriverLock(priv); memset (&ret, 0, sizeof ret); + memset (secmodel, 0, sizeof (*secmodel)); + if (call (conn, priv, 0, REMOTE_PROC_NODE_GET_SECURITY_MODEL, (xdrproc_t) xdr_void, NULL, (xdrproc_t) xdr_remote_node_get_security_model_ret, (char *)&ret) == -1) { 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') { -- |: 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 :|