On Fri, Apr 17, 2009 at 04:33:59PM +0200, Daniel Veillard wrote:
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
I've committed this, and will work on a new error code next.. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|