[libvirt] PATCH: Improve error reporting for operations on inactive domains

[Daniel Veillard]

On Fri, Apr 17, 2009 at 12:39:42PM +0100, Daniel P. Berrange wrote:
> If you try todo an operation on an inactive QEMU guest which is not 
> applicable, eg ask to pause an inactive guest, then you currently get
> a useless message
> 
>   # virsh suspend demo
>   error: Failed to suspend domain demo
>   error: invalid domain pointer in no domain with matching id -1
> 
> There are two issues here
> 
>  - The QEMU driver is mistakenly doing a lookup-by-id when locating the 
>    guest in question. It should in fact do lookup-by-uuid for all APIs
>    since that's the best unique identifier. 
>  - It was using VIR_ERR_INVALID_DOMAIN code instead of VIR_ERR_NO_DOMAIN
>    code, hence the 'invalid domain pointer' bogus message.
> 
> This patch changes all QEMU APIs to lookup based on UUID, and use the
> correct VIR_ERR_NO_DOMAIN code when reporting failures.

   sounds good

> You now get to see the real useful error message later in the API where
> it validates whether the guest is running or not
> 
>   # virsh suspend demo
>   error: Failed to suspend domain demo
>   error: operation failed: domain is not running
> 
> One thing I'm wondering, is whether we should introduce an explicit error
> code for operations that are not applicable.

  yes that would make sense.

> eg, instead of giving VIR_ERR_OPERATION_FAILED, we could give back a code
> like VIR_ERR_OPERATION_INVALID.  This would let callers distinguish 
> real failure of the operation, vs the fact that it simply isn't applicable
> for inactive guests.

  from an application building perspective yes we should really do that,
an user may be able to infer that the domain wasn't started and need
this as a preliminary step. I hope applications will be able to gather
the 2 errors to recover properly from the failure in an automated
fashion.

Patch looks fine, I also checked the virDomainIsActive() check was
present too because going from Id->UUID lookup means we now succeed
in the lookup on inactive domains.

 ACK, looks fine to me

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/