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

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.