Re: [libvirt] [PATCHv2 0/6] remaining cleanups to libvirt.c
On 01/14/2014 04:43 PM, Eric Blake wrote:
v2 of my patch series started here, after applying all patches
already reviewed in that thread:
https://www.redhat.com/archives/libvir-list/2013-December/msg01284.html
Patches 1-3 can be considered bug fixes (particularly patch 3),
so I'd like them in 1.2.1 if they get a favorable review. Patches
4-6 are less urgent, but might as well finish the work all within
one release.
Patch 1 comes from 5/24 in v1
Patch 2 is new
Patch 3 and 4 come from 23/24 in v1
Patch 5 is new
Patch 6 comes from 24/24 in v1
Eric Blake (6):
maint: don't leave garbage on early API exit
maint: avoid nested use of virConnect{Ref,Close}
maint: don't lose error on canceled migration
maint: clean up error reporting in migration
maint: simplify driver registration at startup
maint: replace remaining virLib*Error with better names
cfg.mk | 10 --
src/libvirt.c | 397 ++++++++++++++++++++-----------------------
src/lxc/lxc_process.c | 11 +-
src/qemu/qemu_driver.c | 12 +-
src/qemu/qemu_migration.c | 14 +-
src/qemu/qemu_process.c | 10 +-
src/storage/storage_driver.c | 5 +-
src/uml/uml_driver.c | 3 +-
8 files changed, 209 insertions(+), 253 deletions(-)
Patch 3 seems to be the one where there would be a couple of migration
fixes that could use a backport, especially the change in
virDomainMigrateVersion3Full() where the code now actually honors the
condition where the uri was not set rather than just playing dumb. I
also keep looking the changed "single" (!ret) to a "multi-state"
(!ret
&& !xxx) condition and keep asking myself could we run into a situation
where (!ret and xxx) is true where previously we'd fail on the !ret, but
now wouldn't because xxx was true (where xxx is retcode or cancelled).
Another "step" to consider for patch 5 would be to make common the
repetitive code that follows the virDriverCheckTabMaxReturn(), e.g.
VIR_DEBUG('string')
table[count] = driver
return count++
I don't see a reason other than proximity to release date to preclude
inclusion in 1.2.1
ACK series in general though.
John