[libvirt] [PATCH] fix virsh dominfo returns error when virNodeGetSecurityModel() is not supported.
Tatsuro Enokura
fj2026af at aa.jp.fujitsu.com
Fri Jun 19 01:58:18 UTC 2009
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
More information about the libvir-list
mailing list