[Libvir] Re: [PATCH] Python bindings now generate exceptions for libvirt errors (third version)

Richard W.M. Jones rjones at redhat.com
Wed Mar 28 10:06:01 UTC 2007


Daniel Veillard wrote:
>> So this patch disables exceptions in those two functions only.
> 
>   If you are 100% sure you can't pass a wrong pointer though the 
> python binding then okay.
> 

OK so the problem here is that virDomainGetID can return -1 to mean one 
of two things:  Either that the domain is inactive.  Or that an error 
occurred.

The first case happens because xm_internal.c marks inactive domains by 
setting ->id = -1:

     virDomainPtr xenXMDomainLookupByName(virConnectPtr conn, const char 
*domname)
     //...

     /* Ensure its marked inactive, because may be cached
        handle to a previously active domain */
     ret->id = -1;

(and other places).

virt-manager contains code which relies on this, specifically:

     def current_memory_pretty(self):
         if self.get_id() == -1:
             return "0.00 MB"
         return self.get_memory_pretty()

The second case (error) is when domain is NULL or corrupt:

     unsigned int
     virDomainGetID(virDomainPtr domain)
     {
         if (!VIR_IS_DOMAIN(domain)) {
             virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, 
__FUNCTION__);
             return ((unsigned int) -1);
         }
         return (domain->id);
     }

Anyway, adding code to throw an exception when get_id returns -1 (as in 
patch versions 1 & 2) wasn't clever because it breaks the first case. 
So the third version of the patch doesn't add the exception code for 
just this function and virDomainGetName which is similar.

We're not making things worse by not adding exception code to this 
function (after all - it didn't throw an exception before, and it 
doesn't throw one now: so no change!).  But in general there is a real 
problem here, because we ought to throw an exception when there's an error.

 >> Note that the documentation for virDomainGetID is wrong.
 >
 >   I don't see the error

The virDomainGetID documentation is wrong when it says:

    Returns the domain ID number or (unsigned int) -1 in case of error

It should say something like:

    Returns the domain ID number.  Returns (unsigned int) -1 either for
    error or in the Xen case if the domain is inactive.

Rich.

-- 
Emerging Technologies, Red Hat  http://et.redhat.com/~rjones/
64 Baker Street, London, W1U 7DF     Mobile: +44 7866 314 421
  "[Negative numbers] darken the very whole doctrines of the equations
  and make dark of the things which are in their nature excessively
  obvious and simple" (Francis Maseres FRS, mathematician, 1759)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3237 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20070328/1d9b6de5/attachment-0001.bin>


More information about the libvir-list mailing list