[libvirt] [PATCH 23/24] maint: clean up error reporting in migration

[Eric Blake]

On 01/02/2014 06:57 PM, John Ferlan wrote:
> 
> 
> On 12/28/2013 11:11 AM, Eric Blake wrote:
>> While auditing the error reporting, I noticed that migration
>> had some issues.  Some of the static helper functions tried
>> to call virDispatchError(), even though their caller will also
>> report the error.  Also, if a migration is cancelled early
>> because a uri was not set, we did not guarantee that the finish
>> stage would not overwrite the first error message.
>>
>> * src/qemu/qemu_migration.c (doPeer2PeerMigrate2)
>> (doPeer2PeerMigrate3): Preserve first error when cancelling.
>> * src/libvirt.c (virDomainMigrateVersion3Full): Likewise.
>> (virDomainMigrateVersion1, virDomainMigrateVersion2)
>> (virDomainMigrateDirect): Avoid redundant error dispatch.
>> (virDomainMigrateFinish2, virDomainMigrateFinish3)
>> (virDomainMigrateFinish3Params): Don't report error on cleanup
>> path.
>> (virDomainMigratePeer2PeerFull, virDomainMigrateDirect)
>> (virDomainMigrate2): Use correct errors.
>> (virDomainMigrate*): Prefer virReportError over virLibConnError.
>>
>> Signed-off-by: Eric Blake <eblake at redhat.com>
>> ---
>>  src/libvirt.c             | 181 ++++++++++++++++++++++++----------------------
>>  src/qemu/qemu_migration.c |  10 ++-
>>  2 files changed, 104 insertions(+), 87 deletions(-)
>>
>> diff --git a/src/libvirt.c b/src/libvirt.c
>> index a74bfc7..637bfc1 100644
>> --- a/src/libvirt.c
>> +++ b/src/libvirt.c
> 
> <...snip...>
> 
> 
>>
>> @@ -6387,7 +6396,7 @@ virDomainMigrateFinish2(virConnectPtr dconn,
>>                                                    cookie, cookielen,
>>                                                    uri, flags,
>>                                                    retcode);
>> -        if (!ret)
>> +        if (!ret && !retcode)
> 
> This one (and the two below) feel like a bug fix unrelated to error
> reporting/processing...

Indeed; I'll split this into smaller chunks in my series v2 post, and
see if we can git Jirka to look at it as well (since he's familiar with
migration error scenarios).

> 
> ACK in general - your call on whether to split out the extra checks
> pointed out above or leave things as they are.  If in fact those are
> "bugs" that "should" go into a RHEL release, then perhaps they need to
> be separated...

Worth a v2.

-- 
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: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140109/9307d1d8/attachment-0001.sig>