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@xxxxxxx>
> >
> > ---
> > 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
--
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list