[libvirt] [[PATCH libvirt-java]] Implement Connect.listAllDomains
From: Cédric Bosdonnat <cedric.bosdonnat@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); + } + /** * 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 + * @throws LibvirtException + */ + public Domain[] listAllDomains(int flags) throws LibvirtException { + 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; + } + + /** * Provides the list of names of interfaces on this host * * @return an Array of Strings that contains the names of the interfaces on diff --git a/src/main/java/org/libvirt/jna/Libvirt.java b/src/main/java/org/libvirt/jna/Libvirt.java index 0e4c9fc..1331f80 100644 --- a/src/main/java/org/libvirt/jna/Libvirt.java +++ b/src/main/java/org/libvirt/jna/Libvirt.java @@ -8,6 +8,7 @@ import com.sun.jna.Platform; import com.sun.jna.Pointer; import com.sun.jna.ptr.IntByReference; import com.sun.jna.ptr.LongByReference; +import com.sun.jna.ptr.PointerByReference; /** * The libvirt interface which is exposed via JNA. The complete API is @@ -135,6 +136,7 @@ public interface Libvirt extends Library { int virConnectListDefinedStoragePools(ConnectionPointer virConnectPtr, Pointer[] names, int maxnames); int virConnectListDefinedInterfaces(ConnectionPointer virConnectPtr, Pointer[] name, int maxNames); int virConnectListDomains(ConnectionPointer virConnectPtr, int[] ids, int maxnames); + int virConnectListAllDomains(ConnectionPointer virConnectPtr, PointerByReference domains, int flags); int virConnectListInterfaces(ConnectionPointer virConnectPtr, Pointer[] name, int maxNames); int virConnectListNetworks(ConnectionPointer virConnectPtr, Pointer[] name, int maxnames); int virConnectListNWFilters(ConnectionPointer virConnectPtr, Pointer[] name, int maxnames); diff --git a/src/test/java/org/libvirt/TestJavaBindings.java b/src/test/java/org/libvirt/TestJavaBindings.java index 0123e6a..2703aa3 100644 --- a/src/test/java/org/libvirt/TestJavaBindings.java +++ b/src/test/java/org/libvirt/TestJavaBindings.java @@ -137,6 +137,7 @@ public final class TestJavaBindings extends TestCase { assertEquals("Number of listed domains", 2, conn.listDomains().length); assertEquals("Number of defined domains", 1, conn.numOfDefinedDomains()); assertEquals("Number of listed defined domains", 1, conn.listDefinedDomains().length); + assertEquals("Number of all listed domains", 3, conn.listAllDomains(0).length); assertTrue("Domain1 should be persistent", dom1.isPersistent() == 1); assertTrue("Domain1 should not be active", dom1.isActive() == 0); assertTrue("Domain2 should be active", dom2.isActive() == 1); -- 1.8.4.5
Hi. At Sat, 25 Oct 2014 16:25:48 -0700, Cédric Bosdonnat wrote:
From: Cédric Bosdonnat <cedric.bosdonnat@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 + * @throws LibvirtException + */ + public Domain[] listAllDomains(int flags) throws LibvirtException { + 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. -- Claudio --
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@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=aa0d1948f82ad0e385ea80... --
Hello Claudio, On Fri, 2014-10-31 at 23:40 +0100, Claudio Bley wrote:
At Sat, 25 Oct 2014 16:25:48 -0700, Cédric Bosdonnat wrote: I'd prefer an enum instead of these (ugly) constants.
As a side node, these constants are useless since the ListAllDomainsFlags is not public.
Ok, I mimicked similar code in the Domain class... Just changed it to use an enum.
/** * 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 + * @throws LibvirtException + */ + public Domain[] listAllDomains(int flags) throws LibvirtException { + 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.
Arf, with all those Java things, I forgot about the hidden C pointers behind. I guess a Library.free(domainsRef.getValue()); inside a finally block would do the trick. In your other mail you mention freeing the domains pointers, but the only method that could throw an exception is virConnectListAllDomains... unless we need to catch also any other exception like those thrown by a new. -- Cedric
Sorry for answering so late. But, nowadays, I only get to read emails every other day, if at all. At Mon, 03 Nov 2014 13:26:44 +0100, Cedric Bosdonnat wrote:
Hello Claudio,
On Fri, 2014-10-31 at 23:40 +0100, Claudio Bley wrote:
At Sat, 25 Oct 2014 16:25:48 -0700, Cédric Bosdonnat wrote: I'd prefer an enum instead of these (ugly) constants.
As a side node, these constants are useless since the ListAllDomainsFlags is not public.
Ok, I mimicked similar code in the Domain class... Just changed it to use an enum.
Good.
You leak the memory of the array here.
Arf, with all those Java things, I forgot about the hidden C pointers behind. I guess a
Library.free(domainsRef.getValue());
inside a finally block would do the trick.
In your other mail you mention freeing the domains pointers, but the only method that could throw an exception is virConnectListAllDomains... unless we need to catch also any other exception like those thrown by a new.
Yup, catching on "RuntimeException" and all other checked exceptions (if any are thrown) should be good enough. Claudio
participants (3)
-
Cedric Bosdonnat -
Claudio Bley -
Cédric Bosdonnat