1:48 p.m.
This allows us to send Qemu Guest Agent commands to running domains.
Signed-off-by: Wido den Hollander <wido(a)widodh.nl>
---
src/main/java/org/libvirt/Domain.java | 36 ++++++++++++++++++++++++++
src/main/java/org/libvirt/Library.java | 3 +++
src/main/java/org/libvirt/jna/LibvirtQemu.java | 16 ++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java
diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
index 83a500c..c24df48 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -8,6 +8,7 @@ import org.libvirt.jna.CString;
import org.libvirt.jna.DomainPointer;
import org.libvirt.jna.DomainSnapshotPointer;
import org.libvirt.jna.Libvirt;
+import org.libvirt.jna.LibvirtQemu;
import org.libvirt.jna.SizeT;
import org.libvirt.jna.virDomainBlockInfo;
import org.libvirt.jna.virDomainBlockStats;
@@ -22,6 +23,7 @@ import org.libvirt.event.LifecycleListener;
import org.libvirt.event.PMWakeupListener;
import org.libvirt.event.PMSuspendListener;
import static org.libvirt.Library.libvirt;
+import static org.libvirt.Library.libvirtqemu;
import static org.libvirt.ErrorHandler.processError;
import static org.libvirt.ErrorHandler.processErrorIfZero;
@@ -141,6 +143,23 @@ public class Domain {
public static final int NO_METADATA = (1 << 4);
}
+ static final class QemuAgentFlags {
+ /**
+ * Do not wait for a result
+ */
+ public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0;
+
+ /**
+ * Use default timeout value
+ */
+ public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1;
+
+ /**
+ * Block forever waiting for a result
+ */
+ public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2;
+ }
+
/**
* the native virDomainPtr.
*/
@@ -1556,4 +1575,21 @@ public class Domain {
return processError(libvirt.virDomainUpdateDeviceFlags(VDP, xml, flags));
}
+ /**
+ * Send a Qemu Guest Agent command to a domain
+ *
+ * @param cmd
+ * The command which has to be send
+ * @param timeout
+ * The timeout for waiting on an answer. See QemuAgentFlags for more
information.
+ * @param flags
+ * Flags to be send
+ * @return String
+ * @throws LibvirtException
+ */
+ public String qemuAgentCommand(String cmd, int timeout, int flags) throws
LibvirtException {
+ CString result = libvirtqemu.virDomainQemuAgentCommand(this.VDP, cmd, timeout,
flags);
+ processError(result);
+ return result.toString();
+ }
}
diff --git a/src/main/java/org/libvirt/Library.java
b/src/main/java/org/libvirt/Library.java
index 8e054c6..30f15be 100644
--- a/src/main/java/org/libvirt/Library.java
+++ b/src/main/java/org/libvirt/Library.java
@@ -2,6 +2,7 @@ package org.libvirt;
import org.libvirt.jna.Libvirt;
import org.libvirt.jna.Libvirt.VirEventTimeoutCallback;
+import org.libvirt.jna.LibvirtQemu;
import org.libvirt.jna.CString;
import static org.libvirt.ErrorHandler.processError;
@@ -34,6 +35,7 @@ public final class Library {
};
final static Libvirt libvirt;
+ final static LibvirtQemu libvirtqemu;
// an empty string array constant
// prefer this over creating empty arrays dynamically.
@@ -47,6 +49,7 @@ public final class Library {
} catch (Exception e) {
e.printStackTrace();
}
+ libvirtqemu = LibvirtQemu.INSTANCE;
}
private Library() {}
diff --git a/src/main/java/org/libvirt/jna/LibvirtQemu.java
b/src/main/java/org/libvirt/jna/LibvirtQemu.java
new file mode 100644
index 0000000..82213e9
--- /dev/null
+++ b/src/main/java/org/libvirt/jna/LibvirtQemu.java
@@ -0,0 +1,16 @@
+package org.libvirt.jna;
+
+import com.sun.jna.Library;
+import com.sun.jna.Native;
+import com.sun.jna.Platform;
+
+/**
+ * The libvirt Qemu interface which is exposed via JNA
+ */
+
+public interface LibvirtQemu extends Library {
+
+ LibvirtQemu INSTANCE = (LibvirtQemu) Native.loadLibrary(Platform.isWindows() ?
"virt-qemu-0" : "virt-qemu", LibvirtQemu.class);
+
+ CString virDomainQemuAgentCommand(DomainPointer virDomainPtr, String cmd, int
timeout, int flags);
+}
--
1.9.1
11:04 p.m.
Hi.
At Mon, 9 Nov 2015 13:48:18 +0100,
Wido den Hollander wrote:
This allows us to send Qemu Guest Agent commands to running domains.
Signed-off-by: Wido den Hollander <wido(a)widodh.nl>
---
src/main/java/org/libvirt/Domain.java | 36 ++++++++++++++++++++++++++
src/main/java/org/libvirt/Library.java | 3 +++
src/main/java/org/libvirt/jna/LibvirtQemu.java | 16 ++++++++++++
3 files changed, 55 insertions(+)
create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java
diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
index 83a500c..c24df48 100644
--- a/src/main/java/org/libvirt/Domain.java
+++ b/src/main/java/org/libvirt/Domain.java
@@ -141,6 +143,23 @@ public class Domain {
public static final int NO_METADATA = (1 << 4);
}
+ static final class QemuAgentFlags {
+ /**
+ * Do not wait for a result
+ */
+ public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0;
+
+ /**
+ * Use default timeout value
+ */
+ public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1;
+
+ /**
+ * Block forever waiting for a result
+ */
+ public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2;
+ }
+
I would much prefer an Enum instead of some integer constants. Also,
those flags are currently of little use since the QemuAgentFlags class
is package private.
Also, which version are you targeting? Seems there's also a shutdown flag:
typedef enum {
VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN = -2,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2,
VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1,
VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60,
} virDomainQemuAgentCommandTimeoutValues;
And why are those flags mixed up? Seems like a bad idea to me. Are
those "values" intended to go into the "timeout" parameter or into
the
flags?
And since which release is this actually available in libvirt?
diff --git a/src/main/java/org/libvirt/Library.java
b/src/main/java/org/libvirt/Library.java
index 8e054c6..30f15be 100644
--- a/src/main/java/org/libvirt/Library.java
+++ b/src/main/java/org/libvirt/Library.java
@@ -2,6 +2,7 @@ package org.libvirt;
import org.libvirt.jna.Libvirt;
import org.libvirt.jna.Libvirt.VirEventTimeoutCallback;
+import org.libvirt.jna.LibvirtQemu;
import org.libvirt.jna.CString;
import static org.libvirt.ErrorHandler.processError;
@@ -34,6 +35,7 @@ public final class Library {
};
final static Libvirt libvirt;
+ final static LibvirtQemu libvirtqemu;
// an empty string array constant
// prefer this over creating empty arrays dynamically.
@@ -47,6 +49,7 @@ public final class Library {
} catch (Exception e) {
e.printStackTrace();
}
+ libvirtqemu = LibvirtQemu.INSTANCE;
}
I doubt that we always should load that library. In case the user has
an old version this would fail, wouldn't it?
--
Claudio
--
2:59 p.m.
On 12-11-15 23:04, Claudio Bley wrote:
Hi.
At Mon, 9 Nov 2015 13:48:18 +0100,
Wido den Hollander wrote:
>
> This allows us to send Qemu Guest Agent commands to running domains.
>
> Signed-off-by: Wido den Hollander <wido(a)widodh.nl>
> ---
> src/main/java/org/libvirt/Domain.java | 36 ++++++++++++++++++++++++++
> src/main/java/org/libvirt/Library.java | 3 +++
> src/main/java/org/libvirt/jna/LibvirtQemu.java | 16 ++++++++++++
> 3 files changed, 55 insertions(+)
> create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java
>
> diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
> index 83a500c..c24df48 100644
> --- a/src/main/java/org/libvirt/Domain.java
> +++ b/src/main/java/org/libvirt/Domain.java
> @@ -141,6 +143,23 @@ public class Domain {
> public static final int NO_METADATA = (1 << 4);
> }
>
> + static final class QemuAgentFlags {
> + /**
> + * Do not wait for a result
> + */
> + public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0;
> +
> + /**
> + * Use default timeout value
> + */
> + public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1;
> +
> + /**
> + * Block forever waiting for a result
> + */
> + public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2;
> + }
> +
I would much prefer an Enum instead of some integer constants. Also,
those flags are currently of little use since the QemuAgentFlags class
is package private.
Also, which version are you targeting? Seems there's also a shutdown flag:
typedef enum {
VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN = -2,
VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2,
VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1,
VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60,
} virDomainQemuAgentCommandTimeoutValues;
And why are those flags mixed up? Seems like a bad idea to me. Are
those "values" intended to go into the "timeout" parameter or into
the
flags?
And since which release is this actually available in libvirt?
Hmm, I looked at the master branch on Git. I didn't find the ones you
send. Let me work on that.
> diff --git a/src/main/java/org/libvirt/Library.java
b/src/main/java/org/libvirt/Library.java
> index 8e054c6..30f15be 100644
> --- a/src/main/java/org/libvirt/Library.java
> +++ b/src/main/java/org/libvirt/Library.java
> @@ -2,6 +2,7 @@ package org.libvirt;
>
> import org.libvirt.jna.Libvirt;
> import org.libvirt.jna.Libvirt.VirEventTimeoutCallback;
> +import org.libvirt.jna.LibvirtQemu;
> import org.libvirt.jna.CString;
> import static org.libvirt.ErrorHandler.processError;
>
> @@ -34,6 +35,7 @@ public final class Library {
> };
>
> final static Libvirt libvirt;
> + final static LibvirtQemu libvirtqemu;
>
> // an empty string array constant
> // prefer this over creating empty arrays dynamically.
> @@ -47,6 +49,7 @@ public final class Library {
> } catch (Exception e) {
> e.printStackTrace();
> }
> + libvirtqemu = LibvirtQemu.INSTANCE;
> }
I doubt that we always should load that library. In case the user has
an old version this would fail, wouldn't it?
Yes, that would indeed fail. What do you suggest? Load when required? My
JNA foo is not that good to get this implemented.
Wido
--
Claudio
11:26 p.m.
At Fri, 13 Nov 2015 14:59:52 +0100,
Wido den Hollander wrote:
On 12-11-15 23:04, Claudio Bley wrote:
> Hi.
>
> At Mon, 9 Nov 2015 13:48:18 +0100,
> Wido den Hollander wrote:
>>
>> This allows us to send Qemu Guest Agent commands to running domains.
>>
>> Signed-off-by: Wido den Hollander <wido(a)widodh.nl>
>> ---
>> src/main/java/org/libvirt/Domain.java | 36 ++++++++++++++++++++++++++
>> src/main/java/org/libvirt/Library.java | 3 +++
>> src/main/java/org/libvirt/jna/LibvirtQemu.java | 16 ++++++++++++
>> 3 files changed, 55 insertions(+)
>> create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java
>>
>> diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
>> index 83a500c..c24df48 100644
>> --- a/src/main/java/org/libvirt/Domain.java
>> +++ b/src/main/java/org/libvirt/Domain.java
>> @@ -141,6 +143,23 @@ public class Domain {
>> public static final int NO_METADATA = (1 << 4);
>> }
>>
>> + static final class QemuAgentFlags {
>> + /**
>> + * Do not wait for a result
>> + */
>> + public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0;
>> +
>> + /**
>> + * Use default timeout value
>> + */
>> + public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1;
>> +
>> + /**
>> + * Block forever waiting for a result
>> + */
>> + public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2;
>> + }
>> +
>
> I would much prefer an Enum instead of some integer constants. Also,
> those flags are currently of little use since the QemuAgentFlags class
> is package private.
>
> Also, which version are you targeting? Seems there's also a shutdown flag:
>
> typedef enum {
> VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN = -2,
> VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2,
> VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1,
> VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
> VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60,
> } virDomainQemuAgentCommandTimeoutValues;
>
> And why are those flags mixed up? Seems like a bad idea to me. Are
> those "values" intended to go into the "timeout" parameter or
into the
> flags?
>
> And since which release is this actually available in libvirt?
>
Hmm, I looked at the master branch on Git. I didn't find the ones you
send. Let me work on that.
They are defined in include/libvirt/libvirt-qemu.h.
Apparently, those constants are special timeout values and the
function does not take any flags currently. So the name
"QemuAgentFlags" was a bit misleading.
The SHUTDOWN value is merely an internal value in case when you want
to shut down the guest via the agent, basically an educated guess that
a 60 seconds timeout value should work for a shut down in most cases.
See dd725c53e905db51f39dbaa4a0673e8d1588301b
Since there are no flags currently, simply drop the flags
parameter. We add flags (as enums) later when the function starts to
support one.
Furthermore, I think I'm in favor of adding a handful of methods
instead of (ab)using some special values of a parameter.
/* blocking call */
String qemuAgentCommand(String cmd)
/* default timeout call */
String qemuAgentCommandDefaultTimeout(String cmd)
/* timeout in seconds. no special 0 constant needed. */
String qemuAgentCommandTimeout(String cmd, int timeoutSeconds) {
if (timeoutSeconds < 0) throw IllegalArgumentException("timeoutSeconds value
must be >= 0");
...
}
Do you have a link to some docs where the available commands are
described? We should add this to the javadoc.
What does a qemu command look like? It seems it's some kind of JSON
object. In that case, I think we should not make every user of the
library start to construct their own JSON strings.
May be a simple utility class, e.g.
public class QemuCommand {
public static QemuCommand create(String cmd, String args...) {
...
}
}
should be added and the qemuAgentCommand changed to take such a
parameter instead?
>> diff --git a/src/main/java/org/libvirt/Library.java
b/src/main/java/org/libvirt/Library.java
>> index 8e054c6..30f15be 100644
>> --- a/src/main/java/org/libvirt/Library.java
>> +++ b/src/main/java/org/libvirt/Library.java
>> @@ -2,6 +2,7 @@ package org.libvirt;
>>
>> import org.libvirt.jna.Libvirt;
>> import org.libvirt.jna.Libvirt.VirEventTimeoutCallback;
>> +import org.libvirt.jna.LibvirtQemu;
>> import org.libvirt.jna.CString;
>> import static org.libvirt.ErrorHandler.processError;
>>
>> @@ -34,6 +35,7 @@ public final class Library {
>> };
>>
>> final static Libvirt libvirt;
>> + final static LibvirtQemu libvirtqemu;
>>
>> // an empty string array constant
>> // prefer this over creating empty arrays dynamically.
>> @@ -47,6 +49,7 @@ public final class Library {
>> } catch (Exception e) {
>> e.printStackTrace();
>> }
>> + libvirtqemu = LibvirtQemu.INSTANCE;
>> }
>
> I doubt that we always should load that library. In case the user has
> an old version this would fail, wouldn't it?
>
Yes, that would indeed fail. What do you suggest? Load when required? My
JNA foo is not that good to get this implemented.
We can probably use some sort of singleton pattern
(e.g. initialization-on-demand holder).
--
Claudio--
9:26 p.m.
On 11/15/2015 11:26 PM, Claudio Bley wrote:
At Fri, 13 Nov 2015 14:59:52 +0100,
Wido den Hollander wrote:
>
>
>
> On 12-11-15 23:04, Claudio Bley wrote:
>> Hi.
>>
>> At Mon, 9 Nov 2015 13:48:18 +0100,
>> Wido den Hollander wrote:
>>>
>>> This allows us to send Qemu Guest Agent commands to running domains.
>>>
>>> Signed-off-by: Wido den Hollander <wido(a)widodh.nl>
>>> ---
>>> src/main/java/org/libvirt/Domain.java | 36
++++++++++++++++++++++++++
>>> src/main/java/org/libvirt/Library.java | 3 +++
>>> src/main/java/org/libvirt/jna/LibvirtQemu.java | 16 ++++++++++++
>>> 3 files changed, 55 insertions(+)
>>> create mode 100644 src/main/java/org/libvirt/jna/LibvirtQemu.java
>>>
>>> diff --git a/src/main/java/org/libvirt/Domain.java
b/src/main/java/org/libvirt/Domain.java
>>> index 83a500c..c24df48 100644
>>> --- a/src/main/java/org/libvirt/Domain.java
>>> +++ b/src/main/java/org/libvirt/Domain.java
>>> @@ -141,6 +143,23 @@ public class Domain {
>>> public static final int NO_METADATA = (1 << 4);
>>> }
>>>
>>> + static final class QemuAgentFlags {
>>> + /**
>>> + * Do not wait for a result
>>> + */
>>> + public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0;
>>> +
>>> + /**
>>> + * Use default timeout value
>>> + */
>>> + public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1;
>>> +
>>> + /**
>>> + * Block forever waiting for a result
>>> + */
>>> + public static final int VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2;
>>> + }
>>> +
>>
>> I would much prefer an Enum instead of some integer constants. Also,
>> those flags are currently of little use since the QemuAgentFlags class
>> is package private.
>>
>> Also, which version are you targeting? Seems there's also a shutdown flag:
>>
>> typedef enum {
>> VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN = -2,
>> VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK = -2,
>> VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT = -1,
>> VIR_DOMAIN_QEMU_AGENT_COMMAND_NOWAIT = 0,
>> VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN = 60,
>> } virDomainQemuAgentCommandTimeoutValues;
>>
>> And why are those flags mixed up? Seems like a bad idea to me. Are
>> those "values" intended to go into the "timeout" parameter or
into the
>> flags?
>>
>> And since which release is this actually available in libvirt?
>>
>
> Hmm, I looked at the master branch on Git. I didn't find the ones you
> send. Let me work on that.
They are defined in include/libvirt/libvirt-qemu.h.
Apparently, those constants are special timeout values and the
function does not take any flags currently. So the name
"QemuAgentFlags" was a bit misleading.
The SHUTDOWN value is merely an internal value in case when you want
to shut down the guest via the agent, basically an educated guess that
a 60 seconds timeout value should work for a shut down in most cases.
See dd725c53e905db51f39dbaa4a0673e8d1588301b
Since there are no flags currently, simply drop the flags
parameter. We add flags (as enums) later when the function starts to
support one.
Good point, I'll come up with a revised patch.
Furthermore, I think I'm in favor of adding a handful of methods
instead of (ab)using some special values of a parameter.
/* blocking call */
String qemuAgentCommand(String cmd)
/* default timeout call */
String qemuAgentCommandDefaultTimeout(String cmd)
/* timeout in seconds. no special 0 constant needed. */
String qemuAgentCommandTimeout(String cmd, int timeoutSeconds) {
if (timeoutSeconds < 0) throw IllegalArgumentException("timeoutSeconds value
must be >= 0");
...
}
Do you have a link to some docs where the available commands are
described? We should add this to the javadoc.
Yes, here they are: http://wiki.qemu.org/Features/QAPI/GuestAgent
What does a qemu command look like? It seems it's some kind of
JSON
object. In that case, I think we should not make every user of the
library start to construct their own JSON strings.
May be a simple utility class, e.g.
public class QemuCommand {
public static QemuCommand create(String cmd, String args...) {
...
}
}
should be added and the qemuAgentCommand changed to take such a
parameter instead?
Hmm, I don't know if we want to do that. I think we should always keep
the option open for the user. Libvirt doesn't provide it. We are only
mapping libvirt functions.
I also have some example code:
https://gist.github.com/wido/30457cbe66e3965f00de
I used that for testing.
>>> diff --git a/src/main/java/org/libvirt/Library.java
b/src/main/java/org/libvirt/Library.java
>>> index 8e054c6..30f15be 100644
>>> --- a/src/main/java/org/libvirt/Library.java
>>> +++ b/src/main/java/org/libvirt/Library.java
>>> @@ -2,6 +2,7 @@ package org.libvirt;
>>>
>>> import org.libvirt.jna.Libvirt;
>>> import org.libvirt.jna.Libvirt.VirEventTimeoutCallback;
>>> +import org.libvirt.jna.LibvirtQemu;
>>> import org.libvirt.jna.CString;
>>> import static org.libvirt.ErrorHandler.processError;
>>>
>>> @@ -34,6 +35,7 @@ public final class Library {
>>> };
>>>
>>> final static Libvirt libvirt;
>>> + final static LibvirtQemu libvirtqemu;
>>>
>>> // an empty string array constant
>>> // prefer this over creating empty arrays dynamically.
>>> @@ -47,6 +49,7 @@ public final class Library {
>>> } catch (Exception e) {
>>> e.printStackTrace();
>>> }
>>> + libvirtqemu = LibvirtQemu.INSTANCE;
>>> }
>>
>> I doubt that we always should load that library. In case the user has
>> an old version this would fail, wouldn't it?
>>
>
> Yes, that would indeed fail. What do you suggest? Load when required? My
> JNA foo is not that good to get this implemented.
We can probably use some sort of singleton pattern
(e.g. initialization-on-demand holder).
I might need some help with that! I'll come up with a revised patched
based on your comments.
Wido
8:43 a.m.
At Mon, 16 Nov 2015 21:26:57 +0100,
Wido den Hollander wrote:
> Since there are no flags currently, simply drop the flags
> parameter. We add flags (as enums) later when the function starts to
> support one.
>
Good point, I'll come up with a revised patch.
> Furthermore, I think I'm in favor of adding a handful of methods
> instead of (ab)using some special values of a parameter.
>
> /* blocking call */
> String qemuAgentCommand(String cmd)
>
> /* default timeout call */
> String qemuAgentCommandDefaultTimeout(String cmd)
>
> /* timeout in seconds. no special 0 constant needed. */
> String qemuAgentCommandTimeout(String cmd, int timeoutSeconds) {
> if (timeoutSeconds < 0) throw IllegalArgumentException("timeoutSeconds
value must be >= 0");
>
> ...
> }
>
> Do you have a link to some docs where the available commands are
> described? We should add this to the javadoc.
>
Yes, here they are: http://wiki.qemu.org/Features/QAPI/GuestAgent
> What does a qemu command look like? It seems it's some kind of JSON
> object. In that case, I think we should not make every user of the
> library start to construct their own JSON strings.
>
> May be a simple utility class, e.g.
>
> public class QemuCommand {
> public static QemuCommand create(String cmd, String args...) {
> ...
> }
> }
>
> should be added and the qemuAgentCommand changed to take such a
> parameter instead?
>
Hmm, I don't know if we want to do that. I think we should always keep
the option open for the user. Libvirt doesn't provide it. We are only
mapping libvirt functions.
Yes, but that doesn't mean we should not improve it, IMO. Thankfully,
in Java, we're not bound by the limitations of the C language. Ease of
use of the API is one of my foremost goals for libvirt-java. A plain
mapping of the C API functions would be far easier, but also downright
ugly (with the Java goggles on).
Having a (bit) of Haskell background, and being a Scala programmer by
day, a plain String in place of a proper type is just ugly as well, as
it does not carry any information as to what it's type really is and
what it's format is.
I see that the JSON structure of an agent command is pretty simple,
but we would have to support strings and numbers. I'd still very much
prefer a QemuCommand helper class.
If it turns out that it is not flexible enough, we can always add a
loophole later, e.g.:
class QemuCommand {
public static QemuCommand execute(String cmd, /* TBD */ args...) {
}
public static QemuCommand raw(String jsonObject) {
}
}
>> Yes, that would indeed fail. What do you suggest? Load when
required? My
>> JNA foo is not that good to get this implemented.
>
> We can probably use some sort of singleton pattern
> (e.g. initialization-on-demand holder).
>
I might need some help with that! I'll come up with a revised patched
based on your comments.
OK, I'm sure we can work that out. Basically, we probably need to
access the qemu library through a static method, so we avoid loading
the library when it is not necessary. Let me think about that a bit
more.
--
Claudio--
7:29 p.m.
On 11/17/2015 08:43 AM, Claudio Bley wrote:
At Mon, 16 Nov 2015 21:26:57 +0100,
Wido den Hollander wrote:
>
>
>
>> Since there are no flags currently, simply drop the flags
>> parameter. We add flags (as enums) later when the function starts to
>> support one.
>>
>
> Good point, I'll come up with a revised patch.
>
>> Furthermore, I think I'm in favor of adding a handful of methods
>> instead of (ab)using some special values of a parameter.
>>
>> /* blocking call */
>> String qemuAgentCommand(String cmd)
>>
>> /* default timeout call */
>> String qemuAgentCommandDefaultTimeout(String cmd)
>>
>> /* timeout in seconds. no special 0 constant needed. */
>> String qemuAgentCommandTimeout(String cmd, int timeoutSeconds) {
>> if (timeoutSeconds < 0) throw IllegalArgumentException("timeoutSeconds
value must be >= 0");
>>
>> ...
>> }
>>
>> Do you have a link to some docs where the available commands are
>> described? We should add this to the javadoc.
>>
>
> Yes, here they are: http://wiki.qemu.org/Features/QAPI/GuestAgent
>
>> What does a qemu command look like? It seems it's some kind of JSON
>> object. In that case, I think we should not make every user of the
>> library start to construct their own JSON strings.
>>
>> May be a simple utility class, e.g.
>>
>> public class QemuCommand {
>> public static QemuCommand create(String cmd, String args...) {
>> ...
>> }
>> }
>>
>> should be added and the qemuAgentCommand changed to take such a
>> parameter instead?
>>
>
> Hmm, I don't know if we want to do that. I think we should always keep
> the option open for the user. Libvirt doesn't provide it. We are only
> mapping libvirt functions.
Yes, but that doesn't mean we should not improve it, IMO. Thankfully,
in Java, we're not bound by the limitations of the C language. Ease of
use of the API is one of my foremost goals for libvirt-java. A plain
mapping of the C API functions would be far easier, but also downright
ugly (with the Java goggles on).
Having a (bit) of Haskell background, and being a Scala programmer by
day, a plain String in place of a proper type is just ugly as well, as
it does not carry any information as to what it's type really is and
what it's format is.
I see that the JSON structure of an agent command is pretty simple,
but we would have to support strings and numbers. I'd still very much
prefer a QemuCommand helper class.
True, I understand. Looking at this I would want to use the
JSONReader/JsonWriter from Java, but that's not available by default.
We probably don't want any external libs for libvirt-java. 'arguments'
for Qemu GA is rather simple, it's an array with Strings or Ints.
I rather not write my own JsonWriter, so any ideas here?
If it turns out that it is not flexible enough, we can always add a
loophole later, e.g.:
Indeed. We could also implement all available commands as a Enum:
http://git.qemu.org/?p=qemu.git;a=blob;f=qga/qapi-schema.json;h=78362e071...
It's just the arguments I'm stuck with. I have a few commits, do you
care to take a look at them:
https://github.com/wido/libvirt-java/commits/qemu-guest-command
Especially this one:
https://github.com/wido/libvirt-java/commit/1ff76ed0a8f8d58a102cd4e2bc948...
Wido
class QemuCommand {
public static QemuCommand execute(String cmd, /* TBD */ args...) {
}
public static QemuCommand raw(String jsonObject) {
}
}
>>> Yes, that would indeed fail. What do you suggest? Load when required? My
>>> JNA foo is not that good to get this implemented.
>>
>> We can probably use some sort of singleton pattern
>> (e.g. initialization-on-demand holder).
>>
>
> I might need some help with that! I'll come up with a revised patched
> based on your comments.
OK, I'm sure we can work that out. Basically, we probably need to
access the qemu library through a static method, so we avoid loading
the library when it is not necessary. Let me think about that a bit
more.
--
Claudio--
3221
days inactive
3236
days old
6 comments
2 participants
participants (2)
-
Claudio Bley -
Wido den Hollander