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=78362e071d18de1f04f3b5730d8a32dcb46ee74c;hb=HEAD
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/1ff76ed0a8f8d58a102cd4e2bc948df99a472e20
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--
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list