[libvirt] [PATCH 1/3] qemu: migration: Improve p2p error if we can't open conn

Eric Blake eblake at redhat.com
Tue May 28 21:04:33 UTC 2013


On 05/28/2013 02:44 PM, Cole Robinson wrote:
> By actually showing the Open() error to the user
> ---
>  src/qemu/qemu_migration.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Does qemuDomainObjExitRemote(vm) risk clobbering the last error message?
 Since it potentially unrefs vm on the last reference, I'm wondering if
we have to cache the last error at the right point in time instead of
relying on current behavior.  But obviously it worked in your testing as
written.

> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 19b1236..9ac9be4 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -3472,7 +3472,8 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
>      qemuDomainObjExitRemote(vm);
>      if (dconn == NULL) {
>          virReportError(VIR_ERR_OPERATION_FAILED,
> -                       _("Failed to connect to remote libvirt URI %s"), dconnuri);
> +                       _("Failed to connect to remote libvirt URI %s: %s"),
> +                       dconnuri, virGetLastErrorMessage());
>          virObjectUnref(cfg);
>          return -1;

Another alternative is to just return -1 directly instead of doing a
virReportError() that overwrites the original error, although I'm not
sure if that loses some context that would help the user.  Can you
include a sample error message in your commit log that shows the
improvement?  If so, I'm inclined to ACK this.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130528/d71f5fc3/attachment-0001.sig>


More information about the libvir-list mailing list