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