Re: [PATCH java] Add support for Libvirt Qemu Library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]