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

[Cole Robinson]

On 05/28/2013 05:04 PM, Eric Blake wrote:
> 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.
> 

Yeah seems to work, and doesn't look like anything in virobject.c touches the
error APIs, so it think it's okay. Error objects are thread local, not tied to
any VM reference.

>>
>> 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.
> 

I think the current way is preferred, with migration it could be ambiguous
what connection the error is coming from. With the original code you see:

error: operation failed: Failed to connect to remote libvirt URI
qemu+ssh://root@192.168.123.142/system

If we just raised the connection error you get:

Cannot recv data: Host key verification failed.: Connection reset by peer

With my fix you get:

error: operation failed: Failed to connect to remote libvirt URI
qemu+ssh://root@192.168.123.142/system: Cannot recv data: Host key
verification failed.: Connection reset by peer

Unfortunately I just pushed before adding this to the commit message, sorry.
Thanks for the review.

- Cole