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)
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature