Re: [libvirt] [PATCH java] Add support for Libvirt Qemu Library
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--