[libvirt] [[PATCH libvirt-java]] Implement Connect.listAllDomains

[Claudio Bley]

At Fri, 31 Oct 2014 23:40:26 +0100,
Claudio Bley wrote:
> 
> Hi.
> 
> At Sat, 25 Oct 2014 16:25:48 -0700,
> Cédric Bosdonnat wrote:
> > 
> > From: Cédric Bosdonnat <cedric.bosdonnat at free.fr>
> > 
> > ---
> >  src/main/java/org/libvirt/Connect.java          | 46 +++++++++++++++++++++++++
> >  src/main/java/org/libvirt/jna/Libvirt.java      |  2 ++
> >  src/test/java/org/libvirt/TestJavaBindings.java |  1 +
> >  3 files changed, 49 insertions(+)
> > 
> > diff --git a/src/main/java/org/libvirt/Connect.java b/src/main/java/org/libvirt/Connect.java
> > index fedc60e..390ed89 100644
> > --- a/src/main/java/org/libvirt/Connect.java
> > +++ b/src/main/java/org/libvirt/Connect.java
> > @@ -24,6 +24,7 @@ import com.sun.jna.Memory;
> >  import com.sun.jna.NativeLong;
> >  import com.sun.jna.Pointer;
> >  import com.sun.jna.ptr.LongByReference;
> > +import com.sun.jna.ptr.PointerByReference;
> >  
> >  /**
> >   * The Connect object represents a connection to a local or remote
> > @@ -33,6 +34,28 @@ import com.sun.jna.ptr.LongByReference;
> >   */
> >  public class Connect {
> >  
> > +    static final class ListAllDomainsFlags {
> > +        static final int VIR_CONNECT_LIST_DOMAINS_ACTIVE         = (1 << 0);
> > +        static final int VIR_CONNECT_LIST_DOMAINS_INACTIVE       = (1 << 1);
> > +
> > +        static final int VIR_CONNECT_LIST_DOMAINS_PERSISTENT     = (1 << 2);
> > +        static final int VIR_CONNECT_LIST_DOMAINS_TRANSIENT      = (1 << 3);
> > +
> > +        static final int VIR_CONNECT_LIST_DOMAINS_RUNNING        = (1 << 4);
> > +        static final int VIR_CONNECT_LIST_DOMAINS_PAUSED         = (1 << 5);
> > +        static final int VIR_CONNECT_LIST_DOMAINS_SHUTOFF        = (1 << 6);
> > +        static final int VIR_CONNECT_LIST_DOMAINS_OTHER          = (1 << 7);
> > +
> > +        static final int VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE    = (1 << 8);
> > +        static final int VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE = (1 << 9);
> > +
> > +        static final int VIR_CONNECT_LIST_DOMAINS_AUTOSTART      = (1 << 10);
> > +        static final int VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART   = (1 << 11);
> > +
> > +        static final int VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT   = (1 << 12);
> > +        static final int VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT    = (1 << 13);
> > +    }
> 
> I'd prefer an enum instead of these (ugly) constants.
> 
> As a side node, these constants are useless since the
> ListAllDomainsFlags is not public.
> 
> >      /**
> >       * Get the version of a connection.
> >       *
> > @@ -758,6 +781,29 @@ public class Connect {
> >      }
> >  
> >      /**
> > +     * Lists a possibly filtered list of all the domains.
> > +     * 
> > +     * @param flags bitwise-OR of ListAllDomainsFlags
> > +     *
> > +     * @return and array of the IDs of the active domains

I'd reword that. It returns an array of Domains, not necessarily
active domains.

> > +     * @throws LibvirtException
> > +     */
> > +    public Domain[] listAllDomains(int flags) throws LibvirtException {

Change the signature of the method to something like

public Domain[] listAllDomains(ListAllDomainsFlags...) throw LibvirtException {

with ListAllDomainsFlags being an Enum implementing the BitFlags
interface. See commit aa0d1948[1] I've pushed a couple of minutes ago.

> > +        PointerByReference domainsRef = new PointerByReference();
> > +        int ret = libvirt.virConnectListAllDomains(VCP, domainsRef, flags);
> > +        processError(ret);
> > +
> > +        Pointer[] pointers  = domainsRef.getValue().getPointerArray(0);
> > +        Domain[] domains = new Domain[ret];
> > +        for (int i = 0; i < ret; i++) {
> > +            DomainPointer domainPtr = new DomainPointer();
> > +            domainPtr.setPointer(pointers[i]);
> > +            domains[i] = new Domain(this, domainPtr);
> > +        }
> > +        return domains;
> > +    }
> 
> You leak the memory of the array here.

Oh, and you should make this exception-safe, ie. in case of an
exception inside the loop, call virFreeDomain on each element and free
the array before re-throwing the exception.

--
Claudio

[1]: http://libvirt.org/git/?p=libvirt-java.git;a=commit;h=aa0d1948f82ad0e385ea80e2d8efe6d8d3a1f379
--