Third version of this patch, which should allow virt-manager to start up correctly*. virt-manager relies on virDomainGetID returning -1 in the case when a domain is inactive. On further investigation it turns out that virDomainGetID and virDomainGetName can never fail**, since all they do is read fields from the domain pointer that you pass. It was also claimed that virDomainGetUUID was also error-free, but that's actually not the case. So this patch disables exceptions in those two functions only. Note that the documentation for virDomainGetID is wrong. Thanks to Hugh Brock and Dan Berrange for finding and diagnosing the problem. Rich. * Not tested: for me virt-manager fails for another reason, with or without this patch. ** Well, they can fail in the case where you've corrupted memory and your virDomainPtr isn't really a virDomainPtr, but at that point all bets are off anyway ... -- 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)

Daniel Veillard wrote: 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. 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)

Daniel Veillard wrote: Agreed. Version 3 of the patch has been committed. 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)

On Wed, Mar 28, 2007 at 07:07:32AM -0400, Daniel Veillard wrote: We already have a state flag for it VIR_DOMAIN_SHUTOFF which is set in virDomainInfoPtr, we can fix virt-manager not to call get_id if the domain is shutdown. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

On Wed, Mar 28, 2007 at 10:45:34AM -0400, Daniel Veillard wrote: The virDomainGetInfo is a fast-path call - its either a direct hypercall, or a hypercall via the proxy - we never make an expensive XenD call unless they've broken the proxy somehow. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

Daniel Veillard wrote: Just what I was going to say :-) Rich. -- Emerging Technologies, Red Hat http://et.redhat.com/~rjones/ 64 Baker Street, London, W1U 7DF Mobile: +44 7866 314 421 Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in UK and Wales under Company Registration No. 3798903 Directors: Michael Cunningham (USA), Charlie Peters (USA) and David Owens (Ireland)

On Wed, Mar 28, 2007 at 04:20:18PM +0100, Daniel P. Berrange wrote: Seems to me domain->flags bit DOMAIN_IS_DEFINED is the right place. That could be refreshed each time virConnectListDefinedDomains() virDomainUndefine() virNodeGetInfo() virDomainCreate() is called. The exact same issue could be pointed out for DefinedNetworks, except I would not expect the cost to really be an issue I would guess it's called far less frequently, but I'm just guessing. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/