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 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list