On 08/03/2012 09:48 AM, Osier Yang wrote: > As the consensus in: > https://www.redhat.com/archives/libvir-list/2012-July/msg01692.html, > this patch is to destroy conf/virdomainlist.[ch], foldering the s/foldering/folding/ > helpers into conf/domain_conf.[ch]. > * src/Makefile.am: > - Various indentions fixes incidentally s/indentions/indentations/ The fact that you used the word 'incidentally' (or any other synonym of 'also') should be a red flag that says "am I doing enough extra stuff to warrant splitting it into a separate patch?" If the cleanups are limited to the hunks already touched, and the bulk of the changes are tied to the new code rather than cleanup, then a single patch is okay. But in this particular case, you ended up touching more lines of Makefile.am for indentation than you did for actual code changes, including hunks that had no other change, so it should probably be split into two patches. However, I'd like to see this go in sooner, rather than later, as I have a pending patch based on top of this one that further splits domain_conf.c into snapshot_conf.c now that I'm adding even more snapshot functionality. So while splitting would be nice, I'll let this one slide if its easier for you to push as-is. However, I did hit a compile failure: CC libvirt_conf_la-domain_conf.lo conf/domain_conf.c: In function 'virDomainList': conf/domain_conf.c:15533:17: error: implicit declaration of function 'virUnrefDomain' [-Werror=implicit-function-declaration] conf/domain_conf.c:15533:17: error: nested extern declaration of 'virUnrefDomain' [-Werror=nested-externs] conf/domain_conf.c: In function 'virDomainListSnapshots': conf/domain_conf.c:15579:17: error: implicit declaration of function 'virUnrefDomainSnapshot' [-Werror=implicit-function-declaration] conf/domain_conf.c:15579:17: error: nested extern declaration of 'virUnrefDomainSnapshot' [-Werror=nested-externs] which means you need to rebase it on top of Dan's atomic patches before you can push. ACK with this squashed in: diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index 688da54..71f94e8 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -15530,7 +15530,7 @@ cleanup: int count = virHashSize(domobjs); for (i = 0; i < count; i++) { if (data.domains[i]) - virUnrefDomain(data.domains[i]); + virObjectUnref(data.domains[i]); } } @@ -15576,7 +15576,7 @@ cleanup: if (ret < 0 && list) { for (i = 0; i < count; i++) if (list[i]) - virUnrefDomainSnapshot(list[i]); + virObjectUnref(list[i]); VIR_FREE(list); } return ret; -- Eric Blake eblake@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list