[libvirt] [PATCH 02/48] Destroy virdomainlist.[ch]

[Eric Blake]

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 at 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: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120813/3c60b7a1/attachment-0001.sig>