Re: [libvirt] Libvirt Java Bindings - adding domain event support - pull request
At Thu, 27 Feb 2014 03:47:31 +,
Chris Ellis wrote:
Hi all
I'm new to this list, I've been making use of the Libvirt Java bindings
recently. I wanted to make use of domain events yesterday
so my application can be alerted when the state of a domain changes etc.
However I quickly discovered that domain events are completely broken in
the current Java bindings.
So I have implemented support for domain events in the Java
bindings.
Searching the mailing list or even asking on the list before
implementing something would have probably saved you some work...
Currently I've only implemented the following domain
event IDs:
* VIR_DOMAIN_EVENT_ID_LIFECYCLE
* VIR_DOMAIN_EVENT_ID_REBOOT
* VIR_DOMAIN_EVENT_ID_RTC_CHANGE
As Daniel already pointed out, there's my patch set here
https://www.redhat.com/archives/libvir-list/2014-February/msg00823.html
which contains event support for I/O error, life cycle, PM wakeup, PM
suspend and reboot events (because that's what I needed).
Implementing these is enough to test that my implementation is sane,
I'm hoping to implement the majority of the other events
soon.
Events can be listened to by, registering via the Connect object, as
follows:
con.domainEventRegisterAny(DomainEventID.VIR_DOMAIN_EVENT_ID_LIFECYCLE,
new DomainLifecycleEventHandler() {
@Override
public void onStarted(Connect connection, Domain domain, DomainEventType
event, DomainEventStartedDetailType detail) throws LibvirtException {
System.out.println(Got start event: + event + :: + detail + for
domain + domain.getName());
}
});
As Eric pointed out, there's no need to strictly adhere to the
function naming of libirt's C API.
I dislike overloading though (because it makes the code ambiguous and
doesn't buy you much).
I've put my clone of the libvirt-java git repository on Github, my
modifications to the Java binding are in a separate branch and
should be simple to merge. The changes can be viewed at:
https://github.com/intrbiz/libvirt-java/compare/master...ce-domain-events
I've had a look at it and I see a few similarities to my
implementation. So, we're on the right track, I guess... :-)
For my implementation I've chosen to put the focus on simplicity for
the user and I tried not to limit the usage of the public interface in
any way. In particular this means:
- using the usual Java term listener and add/remove(Listener) idiom
- do not give null a special meaning when passed as a domain
parameter of a method of the public API
(instead, connect.add*Listener(..) registers a listener for all
domains of a connection, whereas domain.add*Listener(..) only for
the domain at hand)
- the user is free to implement several listeners in one class,
reacting on different events in a central place
(this is not possible with your approach and would not be possible
when using method overloading)
- keep the /callback/ method's argument list as short as possible
(e.g. in your DomainLifecycleEventHandler there's an onStarted method
which takes 5 arguments, but it would actually only require two --
the domain and the event detail)
- hide the underlying implementation as far as possible
(the user shouldn't be bothered with callback IDs, event IDs et cetera)
- do not run an event loop in a different thread by default
(this might not be what the user wants, for several reasons)
- do not specify a throws LibvirtException clause for any of the
listener methods
(this is actually forcing the user to handle any such exception
right on the spot in the listener method itself (or wrap the
LibvirtException into some kind of unchecked exception which gets
handled by the JNA callback exception handler))
Another difference to your implementation is the handling of the
different event types for the live cycle events. You have chosen to
branch on the events and implement a method for each type of event. I
like the idea in general (because it solves the problem of
transporting the event detail in regard to the event type to the
user's code in a type safe way), but to me it seems a bit too
complicated if the user wants to handle some type of events in the
same way. A simple switch statement would be a lot easier instead of
branching on the event types and then merging the code paths together
again. Of course, the user could override the onEvent method in that
case but one has to plan which approach beforehand.
I'm in favor of giving the user only one approach, not both. If it
seems feasible the user could implement the switch on the event type
herself.
But maybe a LifecycleAdapter class could be added for convenience
which does the branching...
[See, nobody knows what I'm talking about without seeing the
code. That's why we like to have the patches directly send to the
list.]
I'm keen to get these enhancements / fixes merged into libvirt-java.
My patches were excepted already, but I'll wait for a short term if
you
Re: [libvirt] Libvirt Java Bindings - adding domain event support - pull request
Hi Claudio
On Mon, Mar 3, 2014 at 3:00 PM, Claudio Bley cb...@av-test.de wrote:
At Thu, 27 Feb 2014 03:47:31 +,
Chris Ellis wrote:
Hi all
I'm new to this list, I've been making use of the Libvirt Java bindings
recently. I wanted to make use of domain events yesterday
so my application can be alerted when the state of a domain changes etc.
However I quickly discovered that domain events are completely broken in
the current Java bindings.
So I have implemented support for domain events in the Java
bindings.
Searching the mailing list or even asking on the list before
implementing something would have probably saved you some work...
Arguably I should have spend longer looking, I did google around but
obviously not well enough. From my perspective, my evenings worth
of effort isn't wasted, I've learnt more of the libvirt internals and
advanced
my own project, I can easily backtrack to make use of your code.
Currently I've only implemented the following domain
event IDs:
* VIR_DOMAIN_EVENT_ID_LIFECYCLE
* VIR_DOMAIN_EVENT_ID_REBOOT
* VIR_DOMAIN_EVENT_ID_RTC_CHANGE
As Daniel already pointed out, there's my patch set here
https://www.redhat.com/archives/libvir-list/2014-February/msg00823.html
which contains event support for I/O error, life cycle, PM wakeup, PM
suspend and reboot events (because that's what I needed).
Implementing these is enough to test that my implementation is sane,
I'm hoping to implement the majority of the other events
soon.
Events can be listened to by, registering via the Connect object, as
follows:
con.domainEventRegisterAny(DomainEventID.VIR_DOMAIN_EVENT_ID_LIFECYCLE,
new DomainLifecycleEventHandler() {
@Override
public void onStarted(Connect connection, Domain domain,
DomainEventType
event, DomainEventStartedDetailType detail) throws LibvirtException {
System.out.println(Got start event: + event + :: + detail +
for
domain + domain.getName());
}
});
As Eric pointed out, there's no need to strictly adhere to the
function naming of libirt's C API.
I dislike overloading though (because it makes the code ambiguous and
doesn't buy you much).
I had been trying to keep my code inline with the C API, as I find
libvirt-java a little
confused over whether it is Java like or C like.
I've put my clone of the libvirt-java git repository on Github, my
modifications to the Java binding are in a separate branch and
should be simple to merge. The changes can be viewed at:
https://github.com/intrbiz/libvirt-java/compare/master...ce-domain-events
I've had a look at it and I see a few similarities to my
implementation. So, we're on the right track, I guess... :-)
For my implementation I've chosen to put the focus on simplicity for
the user and I tried not to limit the usage of the public interface in
any way. In particular this means:
- using the usual Java term listener and add/remove(Listener) idiom
- do not give null a special meaning when passed as a domain
parameter of a method of the public API
(instead, connect.add*Listener(..) registers a listener for all
domains of a connection, whereas domain.add*Listener(..) only for
the domain at hand)
- the user is free to implement several listeners in one class,
reacting on different events in a central place
(this is not possible with your approach and would not be possible
when using method overloading)
- keep the /callback/ method's argument list as short as possible
(e.g. in your DomainLifecycleEventHandler there's an onStarted method
which takes 5 arguments, but it would actually only require two --
the domain and the event detail)
- hide the underlying implementation as far as possible
(the user shouldn't be bothered with callback IDs, event IDs et cetera)
- do not run an event loop in a different thread by default
(this might not be what the user wants, for several reasons)
This is 6 of one, half a dozen of the other. IMHO the majority of users
probably just want the library to work, rather than having to faff with how
to execute the event loop.
- do not specify a throws LibvirtException clause for any of the
listener methods
(this is actually forcing the user to handle any such exception
right on the spot in the listener method itself (or wrap the
LibvirtException into some kind of unchecked exception which gets
handled by the JNA callback exception handler))
Another difference to your implementation is the handling of the
different event types for the live cycle events. You have chosen to
branch on the events and implement a method for each type of event. I
like the idea in general (because it solves the problem of
transporting the event detail in regard to the event type to the
user's code in a type safe way), but to me it seems a bit too
complicated if the user wants to handle some type of events in the
same way. A
Re: [libvirt] Libvirt Java Bindings - adding domain event support - pull request
On 02/26/2014 08:47 PM, Chris Ellis wrote:
Hi all
I'm new to this list, I've been making use of the Libvirt Java bindings
recently. I wanted to make use of domain events yesterday
so my application can be alerted when the state of a domain changes etc.
Sideline observations (I'm not a user of the Java bindings):
Events can be listened to by, registering via the Connect object, as
follows:
con.domainEventRegisterAny(DomainEventID.VIR_DOMAIN_EVENT_ID_LIFECYCLE,
new DomainLifecycleEventHandler() {
That's a bit verbose. For example, the C code had to use 'RegisterAny'
because it already had an older 'Register', and C doesn't allow
overloads. But in Java, there is no loss of information if you just do:
con.domainEventRegister(DomainEventID.LIFECYCLE,
new DomainLifecycleEventHandler() {...})
Or even be more object oriented, and have 15 overloads, one for each
type of handler class, so that the caller can merely do:
con.domainEventRegister(new DomainLifecycleEventHandler() {...})
and it becomes obvious based on the type of handler that I passed in
which C event id must be used.
I've put my clone of the libvirt-java git repository on Github, my
modifications to the Java binding are in a separate branch and
should be simple to merge. The changes can be viewed at:
https://github.com/intrbiz/libvirt-java/compare/master...ce-domain-events
On this list, it's easier to review patches if you use 'git send-email'
to post your series on the list, than it it to make us chase down a
random git repository.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Libvirt Java Bindings - adding domain event support - pull request
On Thu, Feb 27, 2014 at 03:47:31AM +, Chris Ellis wrote: Hi all I'm new to this list, I've been making use of the Libvirt Java bindings recently. I wanted to make use of domain events yesterday so my application can be alerted when the state of a domain changes etc. [snip] I've put my clone of the libvirt-java git repository on Github, my modifications to the Java binding are in a separate branch and should be simple to merge. The changes can be viewed at: https://github.com/intrbiz/libvirt-java/compare/master...ce-domain-events I'm keen to get these enhancements / fixes merged into libvirt-java. I would also like to submit further fixes to race conditions in the free() handling. Any advice / thoughts / comments welcome. The libvirt java maintenance work had stalled a bit in recent history, but we've got a new person to take charge of it now. He's got a huge patch series that is pending merge: https://www.redhat.com/archives/libvir-list/2014-February/msg00823.html This does some work with events, though I'm not sure if it will be enough for your needs yet or not. If not I'm sure Claudio will welcome any help you can provide creating patches for the missing pieces. I believe he's offline this week though, so you might not get a response for a little while longer. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Libvirt Java Bindings - adding domain event support - pull request
Hi all
I'm new to this list, I've been making use of the Libvirt Java bindings
recently. I wanted to make use of domain events yesterday
so my application can be alerted when the state of a domain changes etc.
However I quickly discovered that domain events are completely broken in
the current Java bindings. Whilst some of the Libvirt
functions required to support domain events are exposed, the critical
underpinnings are not. Specifically there is no support
for setting up the event loop.
So I have implemented support for domain events in the Java bindings.
Currently I've only implemented the following domain
event IDs:
* VIR_DOMAIN_EVENT_ID_LIFECYCLE
* VIR_DOMAIN_EVENT_ID_REBOOT
* VIR_DOMAIN_EVENT_ID_RTC_CHANGE
Implementing these is enough to test that my implementation is sane, I'm
hoping to implement the majority of the other events
soon.
Events can be listened to by, registering via the Connect object, as
follows:
con.domainEventRegisterAny(DomainEventID.VIR_DOMAIN_EVENT_ID_LIFECYCLE,
new DomainLifecycleEventHandler() {
@Override
public void onStarted(Connect connection, Domain domain, DomainEventType
event, DomainEventStartedDetailType detail) throws LibvirtException {
System.out.println(Got start event: + event + :: + detail + for
domain + domain.getName());
}
});
I've put my clone of the libvirt-java git repository on Github, my
modifications to the Java binding are in a separate branch and
should be simple to merge. The changes can be viewed at:
https://github.com/intrbiz/libvirt-java/compare/master...ce-domain-events
I'm keen to get these enhancements / fixes merged into libvirt-java. I
would also like to submit further fixes to race conditions
in the free() handling.
Any advice / thoughts / comments welcome.
Regards,
Chris Ellis
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
