[libvirt] [PATCH] fix virsh dominfo returns error when virNodeGetSecurityModel() is not supported.
Daniel P. Berrange
berrange at redhat.com
Thu Jun 18 11:20:16 UTC 2009
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 at 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 at 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.
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 :|
More information about the libvir-list
mailing list