Re: [libvirt] [PATCH 1/2] Java bindings for domain events

2008-11-19 Thread Daniel P. Berrange 
On Wed, Nov 19, 2008 at 08:34:41AM +0100, Daniel Veillard wrote:
 On Tue, Nov 18, 2008 at 01:12:42PM -0500, David Lively wrote:
  On Tue, 2008-11-18 at 16:51 +, Daniel P. Berrange wrote:
   On Tue, Nov 18, 2008 at 11:06:10AM -0500, David Lively wrote:
The attached patch (against libvirt-java) implements Java bindings for
libvirt domain events.  This version provides a libvirt EventImpl
running in its own Java Thread, and provides per-Connect synchronization
that makes using the bindings thread-safe.  (Note the Domain, Network,
StoragePool, and StorageVol methods also synchronize on their Connect
object, as required by libvirt.  I have similar changes for
NodeDevice.java that need to be made when that code is checked in.)
   
   I don't particularly like the event loop code because it is adding a huge 
   pile of non-portable JNI code that won't work on Windows, which lacks
   pipe() and poll(). Java already provides a portable pure Java API for 
   building a poll() like event loop in form of NIO.
   
 http://www.xhaus.com/alan/python/jynio/select.html
  
  Yeah, Daniel V and I had briefly considered this, and rejected it on the
  basis of it's complicated and (more importantly) some negative
  feedback I hear from our Java folks on the java.nio Select mechanism.
  
  But I agree the java.nio Select mechanism should greatly decrease the
  amount of JNI code in the Java EventImpl.  I need to look over the docs
  again, but I think it's just a matter of implementing a
  SelectableChannel on top of a fd.  (That JNI code will presumably be
  very different in Win32 and Unix, but it should be a relatively small
  amount of JNI code in comparison to my current impl.)
  
  So I'll look over the java.nio Select documentation and start thinking
  about a more portable approach ... (and also talk more with our Java
  folks about their Select gripes).
 
   I guess it's better to invest a bit more time and come up with a
 solution relying as much as possible on Java threading, I/O and
 synchronization. After all we should capitalize as much as possible
 on the portability work done in the Java engine, and limit the
 C part of the bindings to the strict minimum JNI (as much as possible).
   On one hand we want the bindings to be the easiest possible to use
 and avoid threading limitation imposed to the client code, on the other
 hand limit the C part on those issue, of course that means growing
 the java side of the bindings, but it really should be easier to
 maintain and port than equivalent C code, even if NIO is not the
 nicest Java API :-\

Also, while I remember any event loop implementation in Java should
be an optional add-on class, not a mandatory part of the Java libvirt
bindings. Applications may well already have an event loop they wish
to use - for example a java desktop application will have an event
loop provided  by GTK or QT. So all that would be require is a Java
binding to the libvirt-glib module, so you cn register libvirt with
Glib from Java.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] PATCH: Change signature of virEventAddHandle to allow multiple calls per FD

2008-11-19 Thread Daniel Veillard 
On Mon, Nov 17, 2008 at 04:58:51PM +, Daniel P. Berrange wrote:
 As discussed previously, this patch changes the semantics of the public
 API for dealing with file handle watches. Previously we would track the
 watch based on  the file handle number directly. With this change, the
 virEventAddHandle method returns an integer 'watch' number. This watch
 number is required when unregistering or updating a watch. The watch is
 also passed into the callback when an event occurrs. This allows for
 multiple watches to be registered against the same file descriptor.
 
 There was quite alot of fallout from this patch requiring many callers
 to be updated to comply with the new semantics.

  Okay, +1

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] PATCH: Pass a callback for freeing opaque data when registering handle/timer events

2008-11-19 Thread Daniel Veillard 
On Mon, Nov 17, 2008 at 05:02:40PM +, Daniel P. Berrange wrote:
 When registering for a file descriptor event, or timer events, the event
 callback has an associated 'void *opaque' data blob. When removing a
 registered event, the removal may be done asynchronously to allow safe
 removal from within a callback. This means that it is not safe for the
 application to assume they can free the 'void *opaque' data immediately
 after calling virEventRemoveHandle/Timer. So, we extend the AddHandle/Timer
 method to allow a 2nd callback to be provided. This callback is used to
 free the 'void *opaque' data at the appropriate (safe) point in time.

  okay, it makes more sense to provide the API framework on how to free
the data

 diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
 --- a/include/libvirt/libvirt.h
 +++ b/include/libvirt/libvirt.h
 @@ -1132,21 +1132,39 @@ typedef void (*virEventHandleCallback)(i
  typedef void (*virEventHandleCallback)(int watch, int fd, int events, void 
 *opaque);
  
  /**
 + * virEventHandleFreeFunc:
 + *
 + * @opaque: user data to be free'd
 + *
 + * Callback invoked when memory associated with a file handle
 + * watch is being freed. Will be passed a pointer to the  application's
 + * opaque data blob.
 + */
 +typedef void (*virEventHandleFreeFunc)(void *opaque);
 +
 +/**
   * virEventAddHandleFunc:
   * @fd: file descriptor to listen on
   * @event: bitset of events on which to fire the callback
 - * @cb: the callback to be called
 + * @cb: the callback to be called when an event occurrs
 + * @ff: the callback invoked to free opaque data blob
   * @opaque: user data to pass to the callback
   *
   * Part of the EventImpl, this callback Adds a file handle callback to
   * listen for specific events. The same file handle can be registered
   * multiple times provided the requested event sets are non-overlapping
   *
 + * If the opaque user data requires free'ing when the handle
 + * is unregistered, then a 2nd callback can be supplied for
 + * this purpose.
 + *
   * Returns a handle watch number to be used for updating
   * and unregistering for events
   */
  typedef int (*virEventAddHandleFunc)(int fd, int event,
 - virEventHandleCallback cb, void 
 *opaque);
 + virEventHandleCallback cb,
 + virEventHandleFreeFunc ff,
 + void *opaque);
  
  /**
   * virEventUpdateHandleFunc:
 @@ -1163,7 +1181,11 @@ typedef void (*virEventUpdateHandleFunc)
   * @watch: file descriptor watch to stop listening on
   *
   * Part of the EventImpl, this user-provided callback is notified when
 - * an fd is no longer being listened on
 + * an fd is no longer being listened on.
 + *
 + * If a virEventHandleFreeFunc was supplied when the handle was
 + * registered, it will be invoked some time during, or after this 
 + * function call, when it is safe to release the user data.
   */
  typedef int (*virEventRemoveHandleFunc)(int watch);
  
 @@ -1178,17 +1200,35 @@ typedef void (*virEventTimeoutCallback)(
  typedef void (*virEventTimeoutCallback)(int timer, void *opaque);
  
  /**
 + * virEventTimeoutFreeFunc:
 + *
 + * @opaque: user data to be free'd
 + *
 + * Callback invoked when memory associated with a timer 
 + * is being freed. Will be passed a pointer to the  application's
 + * opaque data blob.
 + */
 +typedef void (*virEventTimeoutFreeFunc)(void *opaque);
 +
 +/**
   * virEventAddTimeoutFunc:
   * @timeout: The timeout to monitor
   * @cb: the callback to call when timeout has expired
 + * @ff: the callback invoked to free opaque data blob
   * @opaque: user data to pass to the callback
   *
   * Part of the EventImpl, this user-defined callback handles adding an
   * event timeout.
   *
 + * If the opaque user data requires free'ing when the handle
 + * is unregistered, then a 2nd callback can be supplied for
 + * this purpose. 
 + *
   * Returns a timer value
   */
 -typedef int (*virEventAddTimeoutFunc)(int timeout, virEventTimeoutCallback 
 cb,
 +typedef int (*virEventAddTimeoutFunc)(int timeout,
 +  virEventTimeoutCallback cb,
 +  virEventTimeoutFreeFunc ff,
void *opaque);
  
  /**
 @@ -1207,6 +1247,10 @@ typedef void (*virEventUpdateTimeoutFunc
   *
   * Part of the EventImpl, this user-defined callback removes a timer
   *
 + * If a virEventTimeoutFreeFunc was supplied when the handle was
 + * registered, it will be invoked some time during, or after this 
 + * function call, when it is safe to release the user data.
 + *
   * Returns 0 on success, -1 on failure
   */
  typedef int (*virEventRemoveTimeoutFunc)(int timer);

  Okay, the API change looks fine, no problem for me.


  and everything else seems to be a direct set of change related to the
extra callback function,

  +1

Daniel

-- 
Daniel Veillard  |

Re: [libvirt] PATCH: User mode linux driver

2008-11-19 Thread Daniel Veillard 
On Wed, Nov 19, 2008 at 01:12:55PM +, Daniel P. Berrange wrote:
 This patch is an update of
 
 http://www.redhat.com/archives/libvir-list/2008-October/msg00355.html
 
 providing a user mode linux driver. I've fixed a number of stupid bugs
 since the first posting, so its more reliable  less crash prone on
 startup. I also added a short driver doc page for the website. Even
 though this driver isn't finished (it still lacks networking), I'd
 like to get this info 0.5.0 release so it can at least be tried out by
 interested people. It shouldn't impact any users of existing drivers.

  Okay, principle sounds right, it's not bad to push this now even if
there is a few sharp edges :-)

[...]
 @@ -2016,32 +2023,30 @@ static virDomainDefPtr virDomainDefParse
  }
  VIR_FREE(nodes);
  
 -/*
 - * If no serial devices were listed, then look for console
 - * devices which is the legacy syntax for the same thing
 - */
 -if (def-nserials == 0) {
 -if ((node = virXPathNode(conn, ./devices/console[1], ctxt)) != 
 NULL) {
 -virDomainChrDefPtr chr = virDomainChrDefParseXML(conn,
 - node);
 -if (!chr)
 -goto error;
 +if ((node = virXPathNode(conn, ./devices/console[1], ctxt)) != NULL) {
 +virDomainChrDefPtr chr = virDomainChrDefParseXML(conn,
 + node);
 +if (!chr)
 +goto error;
  
 -chr-dstPort = 0;
 -/*
 - * For HVM console actually created a serial device
 - * while for non-HVM it was a parvirt console
 - */
 -if (STREQ(def-os.type, hvm)) {
 +chr-dstPort = 0;
 +/*
 + * For HVM console actually created a serial device
 + * while for non-HVM it was a parvirt console
 + */
 +if (STREQ(def-os.type, hvm)) {
 +if (def-nserials != 0) {
 +virDomainChrDefFree(chr);
 +} else {
  if (VIR_ALLOC_N(def-serials, 1)  0) {
  virDomainChrDefFree(chr);
  goto no_memory;
  }
  def-nserials = 1;
  def-serials[0] = chr;
 -} else {
 -def-console = chr;
  }
 +} else {
 +def-console = chr;
  }
  }

  Hummm, that change to console code parsing seems rather unrelated,
isn't it ?

[...]
 +int umlBuildCommandLine(virConnectPtr conn,
 +struct uml_driver *driver ATTRIBUTE_UNUSED,
 +virDomainObjPtr vm,
 +const char ***retargv,
 +const char ***retenv,
 +int **tapfds,
 +int *ntapfds) {
[...]
 +#define ADD_ARG_SPACE   \
 +do { \

  Align \\ with others
In general I prefer to see macros definitions outside of C code blocks,
I don't know why that makes me a bit nervous.


 +for (i = 0 ; i  16 ; i++) {
 +char *ret;
 +if (i == 0  vm-def-console)
 +ret = umlBuildCommandLineChr(conn, vm-def-console, con);
 +else
 +if (asprintf(ret, con%d=none, i)  0)
 +goto no_memory;
 +ADD_ARG(ret);
 +}
 +
 +for (i = 0 ; i  16 ; i++) {
 +virDomainChrDefPtr chr = NULL;
 +char *ret;
 +for (j = 0 ; j  vm-def-nserials ; j++)
 +if (vm-def-serials[j]-dstPort == i)
 +chr = vm-def-serials[j];
 +if (chr)
 +ret = umlBuildCommandLineChr(conn, chr, ssl);
 +else
 +if (asprintf(ret, ssl%d=none, i)  0)
 +goto no_memory;
 +ADD_ARG(ret);
 +}

  I'm a bit puzzled by 16 is that an internal UML limit ?

[...]
 --- /dev/null
 +++ b/src/uml_conf.h
[...]
 +#define umlDebug(fmt, ...) do {} while(0)

  humpf we really need to get this debug internal API set up, my fault
I'm too slow !

[...]
 +++ b/src/uml_driver.c
 @@ -0,0 +1,1671 @@
 +/*
 + * driver.c: core driver methods for managing qemu guests

  s/qemu/uml/

  maybe we could abstract even more internal APIs ;-)

[...]
 +/* umlDebug statements should be changed to use this macro instead. */

  we should have fixed the qemu driver before doing the copy it seems

 +#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__)
 +#define DEBUG0(msg) VIR_DEBUG(__FILE__, %s, msg)
 +
 +#define umlLog(level, msg...) fprintf(stderr, msg)
[...]
 +if (fscanf(pidinfo, %*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u 
 %llu %llu, usertime, systime) != 2) {

  heh, I didn't knew the * assignment-suppression character, I learned
  something !


  okay, no surprises,

   fine by me, +1

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search

Re: [libvirt] PATCH: User mode linux driver

2008-11-19 Thread Daniel Veillard 
On Wed, Nov 19, 2008 at 02:48:44PM +, Daniel P. Berrange wrote:
 On Wed, Nov 19, 2008 at 02:49:01PM +0100, Daniel Veillard wrote:
  On Wed, Nov 19, 2008 at 01:12:55PM +, Daniel P. Berrange wrote:
   This patch is an update of
Hummm, that change to console code parsing seems rather unrelated,
  isn't it ?
 
 It is needed actually - the old logic was subtley flawed. The original
 logic was saying if no serial devices were present, and a console
 was specified then make that console look like a serial device. This is
 compatability for old mistake where we treated Xen HVM serial console
 as a paravirt console/. This fixup logic was accidentailly being applied
 to all types of domain, when it should only have been applied to HVM
 guests. This causes problem with User Mode Linux, where you explicitly
 have 0 or more paravirt consoles, and 0 or more serial ports. So my
 change here, just restricts the fixup to hvm only.

  ah, okay !

   +for (i = 0 ; i  16 ; i++) {
   +char *ret;
   +if (i == 0  vm-def-console)
   +ret = umlBuildCommandLineChr(conn, vm-def-console, con);
   +else
   +if (asprintf(ret, con%d=none, i)  0)
   +goto no_memory;
   +ADD_ARG(ret);
   +}
   +
   +for (i = 0 ; i  16 ; i++) {
   +virDomainChrDefPtr chr = NULL;
   +char *ret;
   +for (j = 0 ; j  vm-def-nserials ; j++)
   +if (vm-def-serials[j]-dstPort == i)
   +chr = vm-def-serials[j];
   +if (chr)
   +ret = umlBuildCommandLineChr(conn, chr, ssl);
   +else
   +if (asprintf(ret, ssl%d=none, i)  0)
   +goto no_memory;
   +ADD_ARG(ret);
   +}
  
I'm a bit puzzled by 16 is that an internal UML limit ?
 
 Yes, thats a hardcoded user mode linux limit.

  well, maybe isolate that with a #define at the top of the module then,

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] Java bindings for domain events

2008-11-19 Thread David Lively 
On Wed, 2008-11-19 at 10:48 +, Daniel P. Berrange wrote:
 On Wed, Nov 19, 2008 at 08:34:41AM +0100, Daniel Veillard wrote:
  On Tue, Nov 18, 2008 at 01:12:42PM -0500, David Lively wrote:
   On Tue, 2008-11-18 at 16:51 +, Daniel P. Berrange wrote:
On Tue, Nov 18, 2008 at 11:06:10AM -0500, David Lively wrote:
 The attached patch (against libvirt-java) implements Java bindings for
 libvirt domain events.  This version provides a libvirt EventImpl
 running in its own Java Thread, and provides per-Connect 
 synchronization
 that makes using the bindings thread-safe.  (Note the Domain, Network,
 StoragePool, and StorageVol methods also synchronize on their Connect
 object, as required by libvirt.  I have similar changes for
 NodeDevice.java that need to be made when that code is checked in.)

I don't particularly like the event loop code because it is adding a 
huge 
pile of non-portable JNI code that won't work on Windows, which lacks
pipe() and poll(). Java already provides a portable pure Java API for 
building a poll() like event loop in form of NIO.

  http://www.xhaus.com/alan/python/jynio/select.html
   
   Yeah, Daniel V and I had briefly considered this, and rejected it on the
   basis of it's complicated and (more importantly) some negative
   feedback I hear from our Java folks on the java.nio Select mechanism.
   
   But I agree the java.nio Select mechanism should greatly decrease the
   amount of JNI code in the Java EventImpl.  I need to look over the docs
   again, but I think it's just a matter of implementing a
   SelectableChannel on top of a fd.  (That JNI code will presumably be
   very different in Win32 and Unix, but it should be a relatively small
   amount of JNI code in comparison to my current impl.)
   
   So I'll look over the java.nio Select documentation and start thinking
   about a more portable approach ... (and also talk more with our Java
   folks about their Select gripes).
  
I guess it's better to invest a bit more time and come up with a
  solution relying as much as possible on Java threading, I/O and
  synchronization. After all we should capitalize as much as possible
  on the portability work done in the Java engine, and limit the
  C part of the bindings to the strict minimum JNI (as much as possible).

Agreed -- BUT if we don't make libvirt allow asynchronous EventImpl
callbacks (the second patch in this series (plus requiring pthreads,
which sounds like a Big Deal for WIN32)), we'll have to stop the
EventImpl callbacks from happening:
 (a) at the same time *ANY* connection (in the same process) is in use
 (because EventImpl callbacks aren't necessarily associated with any
 particular connection, though they might be)
 (b) at the same time another EventImpl callback is happening

The patch already synchronizes operations using virConnect objects with
each other.  To avoid making illegal EventImpl callbacks from Java for
the current libvirt, I have to lock every Connect object known to Java
and hold off creating new connections (via open  friends) around an
EventImpl callback.  This sounds rather appalling to me, but it's
starting to sound like the only practical route in the short term
(unless it turns out we can rely on pthreads in WIN32 ...).

Alternatively, I could just use one global lock for everything, but that
essentially single-threads all libvirt operations, which seems *really*
undesirable.

On one hand we want the bindings to be the easiest possible to use
  and avoid threading limitation imposed to the client code, on the other
  hand limit the C part on those issue, of course that means growing
  the java side of the bindings, but it really should be easier to
  maintain and port than equivalent C code, even if NIO is not the
  nicest Java API :-\

Yeah, I suspect so.  I'm starting to implement this path ...

 Also, while I remember any event loop implementation in Java should
 be an optional add-on class, not a mandatory part of the Java libvirt
 bindings. Applications may well already have an event loop they wish
 to use - for example a java desktop application will have an event
 loop provided  by GTK or QT. So all that would be require is a Java
 binding to the libvirt-glib module, so you cn register libvirt with
 Glib from Java.

Gotcha.  I've just installed glib-java-devel, so I'll make sure this
easily integrates with a glib event loop as exposed in glib-java.

Dave


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] Java bindings for domain events

2008-11-19 Thread Daniel P. Berrange 
On Wed, Nov 19, 2008 at 10:35:45AM -0500, David Lively wrote:
 On Wed, 2008-11-19 at 10:48 +, Daniel P. Berrange wrote:
  Also, while I remember any event loop implementation in Java should
  be an optional add-on class, not a mandatory part of the Java libvirt
  bindings. Applications may well already have an event loop they wish
  to use - for example a java desktop application will have an event
  loop provided  by GTK or QT. So all that would be require is a Java
  binding to the libvirt-glib module, so you cn register libvirt with
  Glib from Java.
 
 Gotcha.  I've just installed glib-java-devel, so I'll make sure this
 easily integrates with a glib event loop as exposed in glib-java.

The current libvirt-glib integration code is here

  http://libvirt.org/hg/libvirt-glib/


Though I'll need to update it once I comitt the virFreeCallback changes
shortly...

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] Java bindings for domain events

2008-11-19 Thread David Lively 
On Wed, 2008-11-19 at 10:35 -0500, David Lively wrote:
 The patch already synchronizes operations using virConnect objects with
 each other.  To avoid making illegal EventImpl callbacks from Java for
 the current libvirt, I have to lock every Connect object known to Java
 and hold off creating new connections (via open  friends) around an
 EventImpl callback.  This sounds rather appalling to me, but it's
 starting to sound like the only practical route in the short term
 (unless it turns out we can rely on pthreads in WIN32 ...).

Hmmm ... maybe the less appalling :-) route is practical.  Currently, we
require only the Windows equivalent of a simple pthread mutex.  We just
need to support declaration, initialization, lock, unlock, and
destruction, something like the following (thanks to Tom Hazel for
pointing me to the Windows Mutex stuff):

#if (defined _WIN32 || defined __WIN32__)
#define PTHREAD_MUTEX_T(v) HANDLE v
#define pthread_mutex_init(lk,p) ((*(lk)) = CreateMutex(0, FALSE, 0))
#define pthread_mutex_destroy(lk) CloseHandle(*(lk))
#define pthread_mutex_lock(lk) WaitForSingleObject(*(lk), INFINITE)
#define pthread_mutex_unlock(lk) ReleaseMutex(*(lk))
#define pthread_sigmask(h, s, o) sigprocmask((h), (s), (o))
#endif

I'm not a Windows guy, so maybe I'm missing something.  But this doesn't
seem like a Big Deal ...

Dave


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] fix WIN64 compatibility bug in external EventImpl API

2008-11-19 Thread Daniel P. Berrange 
On Wed, Nov 19, 2008 at 11:41:43AM -0500, David Lively wrote:
 While starting to think about Windows compability, I realized the newly
 exposed API for registering an external EventImpl is not adequate.
 Currently it's assuming 32-bit unix fds.  But Windows uses a pointer
 (HANDLE) here.  So we need to generalize this interface so it can be
 implemented for 64-bit Windows.  The attached patch does this.  (I'm
 sure it conflicts with work Dan B is doing, so I'm hoping he'll just
 incorporate this into his changes.)

I'm not sure whether this is actually required. We're using gnulib for
socket stuff, and that wraps the Winsock socket() call so that it returns
a real file descriptor rather than a socket handle. It does this calling
_open_osfhandle which appears to be declared to accept a 'long' and return
an 'int' - at least in MinGW headers.

MinGW  doesn't implement Win64 though, so its possible the header isn't
quite right. If you have a Win64 box available, could you post what you
see as the declarations for

_open_osfhandle
_get_osfhandle


Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Java bindings for domain events

2008-11-19 Thread Daniel P. Berrange 
On Tue, Nov 18, 2008 at 01:16:46PM -0500, David Lively wrote:
 On Mon, 2008-11-17 at 22:22 +, Daniel P. Berrange wrote:
  On Mon, Nov 17, 2008 at 03:55:13PM -0500, David Lively wrote:
 
  Functionally this all looks fine.
  
  From a style point of view, we should keep consistency with the other
  virEventAddHandle func in terms of typing / param ordering. I prefer
  to have a typedef for the 'freefunc', even though its trivial, because
  I hate reading function prototypes :-) Whether we have the freefunc,
  before or after the 'void opaque' in the register method I don't
  really mind one way or the other as long as we're consistent. Having
  the freefunc last is probably best, since its very often just going
  to be NULL.
  
  Daniel
 
 Ok, here's a version with virFreeCallback as the freefunc (now
 called freecb) typedef.

Thanks I've committed this patch now.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] PATCH: Pass a callback for freeing opaque data when registering handle/timer events

2008-11-19 Thread Daniel Berrange 
On Wed, Nov 19, 2008 at 02:21:40PM +0100, Daniel Veillard wrote:
 On Mon, Nov 17, 2008 at 05:02:40PM +, Daniel P. Berrange wrote:
  When registering for a file descriptor event, or timer events, the event
  callback has an associated 'void *opaque' data blob. When removing a
  registered event, the removal may be done asynchronously to allow safe
  removal from within a callback. This means that it is not safe for the
  application to assume they can free the 'void *opaque' data immediately
  after calling virEventRemoveHandle/Timer. So, we extend the AddHandle/Timer
  method to allow a 2nd callback to be provided. This callback is used to
  free the 'void *opaque' data at the appropriate (safe) point in time.
 
   okay, it makes more sense to provide the API framework on how to free
 the data

I've committed this change, but using the generic virFreeCallback
typedef, instead of my custom one. Also I put the virFreeCallback as
the last arg, to match the style of David Lively's equivalent patch
to the virDomainEventRegister call.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] PATCH: Change signature of virEventAddHandle to allow multiple calls per FD

2008-11-19 Thread Daniel P. Berrange 
On Wed, Nov 19, 2008 at 02:23:03PM +0100, Daniel Veillard wrote:
 On Mon, Nov 17, 2008 at 04:58:51PM +, Daniel P. Berrange wrote:
  As discussed previously, this patch changes the semantics of the public
  API for dealing with file handle watches. Previously we would track the
  watch based on  the file handle number directly. With this change, the
  virEventAddHandle method returns an integer 'watch' number. This watch
  number is required when unregistering or updating a watch. The watch is
  also passed into the callback when an event occurrs. This allows for
  multiple watches to be registered against the same file descriptor.
  
  There was quite alot of fallout from this patch requiring many callers
  to be updated to comply with the new semantics.
 
   Okay, +1

Thanks, this is committed too now.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] PATCH: allow to set nvcpus = 0

2008-11-19 Thread Evgeniy Sokolov 



On Mon, Nov 17, 2008 at 06:44:07PM +0300, Evgeniy Sokolov wrote:

OpenVZ uses all CPUs available in system
- by default (number of CPUs did not set)
- number of CPUs = 0

Currenty, libvirt don't allow to set nvcpus = 0

Attached patch removes limitation in libvirt set nvcpu = 0, but add it 
to each driver which allow to set number of virtual CPU. For OpenVZ set 
default number of CPUs = 0.


This is the wrong way to handle this. If OpenVZ allows the container
to use all the host CPUs, then the vCPUs number should reflect the
number of pCPUs, not 0. So when loading the openvz config, if there
is no CPUS=  setting in the config file, the driver should fill in
the number of host pCPUs. Likewise, when setting the openvz config,
if the vCPUs in the XML is =  pCPUs, then it should just leave
out the CPUS= setting, so OpenVZ uses all CPUs.


It is not like in OpenVZ, but good way. Attached patch implement it.
Index: openvz_conf.c
===
RCS file: /data/cvs/libvirt/src/openvz_conf.c,v
retrieving revision 1.50
diff -u -r1.50 openvz_conf.c
--- openvz_conf.c	17 Nov 2008 09:55:59 -	1.50
+++ openvz_conf.c	19 Nov 2008 16:16:39 -
@@ -49,6 +49,7 @@
 #include buf.h
 #include memory.h
 #include util.h
+#include nodeinfo.h
 
 static char *openvzLocateConfDir(void);
 static int openvzGetVPSUUID(int vpsid, char *uuidstr);
@@ -427,10 +428,11 @@
 goto cleanup;
 } else if (ret  0) {
 dom-def-vcpus = strtoI(temp);
-} else {
-dom-def-vcpus = 1;
 }
 
+if (ret == 0 || dom-def-vcpus == 0)
+dom-def-vcpus = openvzGetNodeCPUs();
+
 /* XXX load rest of VM config data  */
 
 openvzReadNetworkConf(NULL, dom-def, veid);
@@ -457,6 +459,19 @@
 return -1;
 }
 
+unsigned int
+openvzGetNodeCPUs(void) 
+{
+virNodeInfo nodeinfo;
+
+if (virNodeInfoPopulate(NULL, nodeinfo)  0) {
+openvzError(NULL, VIR_ERR_INTERNAL_ERROR,
+  _(Cound not read nodeinfo));
+return 0;
+}
+
+return nodeinfo.cpus;
+}
 
 int
 openvzWriteConfigParam(int vpsid, const char *param, const char *value)
Index: openvz_conf.h
===
RCS file: /data/cvs/libvirt/src/openvz_conf.h,v
retrieving revision 1.17
diff -u -r1.17 openvz_conf.h
--- openvz_conf.h	12 Nov 2008 16:35:47 -	1.17
+++ openvz_conf.h	19 Nov 2008 16:16:39 -
@@ -68,5 +68,6 @@
 void openvzFreeDriver(struct openvz_driver *driver);
 int strtoI(const char *str);
 int openvzSetDefinedUUID(int vpsid, unsigned char *uuid);
+unsigned int openvzGetNodeCPUs(void);
 
 #endif /* OPENVZ_CONF_H */
Index: openvz_driver.c
===
RCS file: /data/cvs/libvirt/src/openvz_driver.c,v
retrieving revision 1.60
diff -u -r1.60 openvz_driver.c
--- openvz_driver.c	17 Nov 2008 11:44:51 -	1.60
+++ openvz_driver.c	19 Nov 2008 16:16:39 -
@@ -834,7 +834,7 @@
 char   str_vcpus[32];
 const char *prog[] = { VZCTL, --quiet, set, vm ? vm-def-name : NULL,
--cpus, str_vcpus, --save, NULL };
-
+unsigned int pcpus;
 
 if (!vm) {
 openvzError(conn, VIR_ERR_INVALID_DOMAIN,
@@ -848,6 +848,10 @@
 return -1;
 }
 
+pcpus = openvzGetNodeCPUs();
+if (pcpus  0  pcpus  nvcpus)
+nvcpus = pcpus;
+
 snprintf(str_vcpus, 31, %d, nvcpus);
 str_vcpus[31] = '\0';
 
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] fix WIN64 compatibility bug in external EventImpl API

2008-11-19 Thread David Lively 
On Wed, 2008-11-19 at 16:49 +, Daniel P. Berrange wrote:
 On Wed, Nov 19, 2008 at 11:41:43AM -0500, David Lively wrote:
  While starting to think about Windows compability, I realized the newly
  exposed API for registering an external EventImpl is not adequate.
  Currently it's assuming 32-bit unix fds.  But Windows uses a pointer
  (HANDLE) here.  So we need to generalize this interface so it can be
  implemented for 64-bit Windows.  The attached patch does this.  (I'm
  sure it conflicts with work Dan B is doing, so I'm hoping he'll just
  incorporate this into his changes.)
 
 I'm not sure whether this is actually required. We're using gnulib for
 socket stuff, and that wraps the Winsock socket() call so that it returns
 a real file descriptor rather than a socket handle. It does this calling
 _open_osfhandle which appears to be declared to accept a 'long' and return
 an 'int' - at least in MinGW headers.

That means that the Windows application using libvirt must use gnulib as
well.  If the Windows version of libvirt actually exports the gnulib
bindings and headers, then I guess that's not a problem.  But does it
export gnulib?


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] fix WIN64 compatibility bug in external EventImpl API

2008-11-19 Thread Daniel P. Berrange 
On Wed, Nov 19, 2008 at 01:28:23PM -0500, David Lively wrote:
 On Wed, 2008-11-19 at 16:49 +, Daniel P. Berrange wrote:
  On Wed, Nov 19, 2008 at 11:41:43AM -0500, David Lively wrote:
   While starting to think about Windows compability, I realized the newly
   exposed API for registering an external EventImpl is not adequate.
   Currently it's assuming 32-bit unix fds.  But Windows uses a pointer
   (HANDLE) here.  So we need to generalize this interface so it can be
   implemented for 64-bit Windows.  The attached patch does this.  (I'm
   sure it conflicts with work Dan B is doing, so I'm hoping he'll just
   incorporate this into his changes.)
  
  I'm not sure whether this is actually required. We're using gnulib for
  socket stuff, and that wraps the Winsock socket() call so that it returns
  a real file descriptor rather than a socket handle. It does this calling
  _open_osfhandle which appears to be declared to accept a 'long' and return
  an 'int' - at least in MinGW headers.
 
 That means that the Windows application using libvirt must use gnulib as
 well.  If the Windows version of libvirt actually exports the gnulib
 bindings and headers, then I guess that's not a problem.  But does it
 export gnulib?

No, the gnulib stuff is internal only - we don't force any apps to also
use gnulib.

It does however mean we should document that the 'fd' arg of the
the  virEventAddHandle callback is an file handle, and not a socket
handle on Win32, so apps are clear on what to expect.

If they're not using gnulib themselves, they can trivially convert
back to a true socket handle if desired.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] SolidICE integration

2008-11-19 Thread Shahar Frank 
Hi All,

I am sending this again because the last mail was using a wrong source...

So here it is:


Hi All, here is an initial version of the operations required for SolidICE
and the proposed high level interface.

I would like to discuss how to map it to libvirt calls and what libvirt
calls are missing.

Thanks,

Shahar Frank

HostLevel
-
enumarateLuns():
return:
returns all luns visible by the host.
In case multipathing is enabled then only the multipath
device should   be in the return list.

enumerateISCSILuns(target):
params:
target - ISCSI target
Description:
enumerate all ISCSI luns on a specific target
return:
list of
getBlockInfo(name or UUID):
return:
at least uuid  size
enumerateVGs():
return:
VG UUID list


open issues - should we assume ISCSI auto connect or FC login?



StoragePool level
-
createVolumeGroup(name, devices):
params:
name = name
devices - a list of UUIDs
description:
Should create a volume group out of the UUID list
In case multipathing is enabled then only the multipath
device  should be used
In case it is not a PV yet then a PV is to be created.
Return:
Uuid of the VG

removeVolumeGroup(VG):
params:
VG - uuid of the Volume group
Description:
Remove all remains for the VolumeGroup (LVs, PVs ...)
Return:
Sucsess/failure

mountNFS(server, remotePath, localPath, params)
params:
server - the NFS server name or IP
temotePath - remote server export path
localPath - local path to mount on
params - nfs mount params
description:
mounts the remote export onto the local path using the
mountparams
return:
success/failure


unmount(localPath, forceFlag)
params:
localPath - the local path to unmount
forceFlag - a flag indicating whether to force unmount
description:
unmounts local path (trivial)
return:
success/failure

Open issues:
1. NFS configuration persistency?
2. What happens in case the libvirt is restarted should it be
reconfigured?


VolumeGroup level
-
listPV(VG):
params:
VG - Volume group UUID
Description:
Lists all PV in a specific volume group
Return:
A list of PV UUIDs



infoPV(VG, PV)
params:
VG - Volume group UUID
PV - Physical volume UUID
description:
trivial
return:
returns the PV information (at least size  path)



extendVolumeGroup(VG, devices):
params:
VG - Volume Group UUID
devices - a list of UUIDs
description:
This function adds Phisical Volume to the VolumeGroup.
Each UUID device is checked, In case multipathing is
enabled
then only the multipath device should be used (PV create
in case
it's not) and added to the volume group
return:
success/fauilure


removePV(VG, PV):
params:
VG - Volume Group UUID
PV - Physical volume's UUID to be removed
description:
removes the physical volume
return:
success failure

LV operations
--
listLVs(VG):
params:
VG - Volume Group UUID
description:
trivial
return:
list of all LV UUIDs

infoLV(VG, LV):
params:
VG - Volume Group UUID
LV - Logical Volume UUID
description:
trivial
return:
returns LV information (at least size and path)

createLV(VG, LvName, size):
params:
VG - Volume Group UUID
LvName - name of the logical volume

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] fix WIN64 compatibility bug in external EventImpl API

2008-11-19 Thread David Lively 
On Wed, 2008-11-19 at 18:33 +, Daniel P. Berrange wrote:
 On Wed, Nov 19, 2008 at 01:28:23PM -0500, David Lively wrote:
  On Wed, 2008-11-19 at 16:49 +, Daniel P. Berrange wrote:
   On Wed, Nov 19, 2008 at 11:41:43AM -0500, David Lively wrote:
While starting to think about Windows compability, I realized the newly
exposed API for registering an external EventImpl is not adequate.
Currently it's assuming 32-bit unix fds.  But Windows uses a pointer
(HANDLE) here.  So we need to generalize this interface so it can be
implemented for 64-bit Windows.  The attached patch does this.  (I'm
sure it conflicts with work Dan B is doing, so I'm hoping he'll just
incorporate this into his changes.)
   
   I'm not sure whether this is actually required. We're using gnulib for
   socket stuff, and that wraps the Winsock socket() call so that it returns
   a real file descriptor rather than a socket handle. It does this calling
   _open_osfhandle which appears to be declared to accept a 'long' and return
   an 'int' - at least in MinGW headers.
  
  That means that the Windows application using libvirt must use gnulib as
  well.  If the Windows version of libvirt actually exports the gnulib
  bindings and headers, then I guess that's not a problem.  But does it
  export gnulib?
 
 No, the gnulib stuff is internal only - we don't force any apps to also
 use gnulib.
 
 It does however mean we should document that the 'fd' arg of the
 the  virEventAddHandle callback is an file handle, and not a socket
 handle on Win32, so apps are clear on what to expect.

Does Windows support integer file handles?  Or are they a winsock
concept - in which case we're assuming the app uses winsock, right?

[Sorry - I'm not trying to be thick -- I'm not a Windows guy at all.
But I have hazy memories of winsock not being standard WIN32 ...]

Dave

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] fix WIN64 compatibility bug in external EventImpl API

2008-11-19 Thread Daniel P. Berrange 
On Wed, Nov 19, 2008 at 03:11:35PM -0500, David Lively wrote:
 On Wed, 2008-11-19 at 18:33 +, Daniel P. Berrange wrote:
  On Wed, Nov 19, 2008 at 01:28:23PM -0500, David Lively wrote:
   On Wed, 2008-11-19 at 16:49 +, Daniel P. Berrange wrote:
On Wed, Nov 19, 2008 at 11:41:43AM -0500, David Lively wrote:
 While starting to think about Windows compability, I realized the 
 newly
 exposed API for registering an external EventImpl is not adequate.
 Currently it's assuming 32-bit unix fds.  But Windows uses a pointer
 (HANDLE) here.  So we need to generalize this interface so it can be
 implemented for 64-bit Windows.  The attached patch does this.  (I'm
 sure it conflicts with work Dan B is doing, so I'm hoping he'll just
 incorporate this into his changes.)

I'm not sure whether this is actually required. We're using gnulib for
socket stuff, and that wraps the Winsock socket() call so that it 
returns
a real file descriptor rather than a socket handle. It does this calling
_open_osfhandle which appears to be declared to accept a 'long' and 
return
an 'int' - at least in MinGW headers.
   
   That means that the Windows application using libvirt must use gnulib as
   well.  If the Windows version of libvirt actually exports the gnulib
   bindings and headers, then I guess that's not a problem.  But does it
   export gnulib?
  
  No, the gnulib stuff is internal only - we don't force any apps to also
  use gnulib.
  
  It does however mean we should document that the 'fd' arg of the
  the  virEventAddHandle callback is an file handle, and not a socket
  handle on Win32, so apps are clear on what to expect.
 
 Does Windows support integer file handles?  Or are they a winsock
 concept - in which case we're assuming the app uses winsock, right?

It is a complicated situation :-)

The standard Winsock API equivalent to socket() is called
WSASocket() and returns a datatype SOCKET, which I believe is
just an alias for HANDLE.

GnuLib's goal is to fix non-POSIX-ness, so it provides a impl
called socket() which returns an 'int' as per POSIX. Internally
this is implemented by calling WSASocket(), and then using the
_open_osfhandle() method to convert the SOCKET into a regular
'int' file handle which is compatible with Windows' UNIX API
layer. eg close, ioctl, etc

So, yes, you are correct, that regular Winsock is not compatible with
int file handles, but by using GNUlib and the compat layer, we do
re-gain proper compatability.

The compatability works both ways - if an application using libvirt
really does want the original SOCKET handle again, it can convert
the file handle back.

So _open_osfhandle() and _get_osfhandle() are idempotent

MicroSoft have some docs on this here

  http://msdn.microsoft.com/en-us/library/bdts1c9x(VS.71).aspx
  http://msdn.microsoft.com/en-us/library/ks2530z6.aspx

As an example, we use GNULIB in GTK-VNC too, and need to integrate
with GLib event loop. GLib on Win32 is written to assume use of
traditional SOCKET handles, so when we register the GTK-VNC socket 
with GLib, we use _get_osfhandle to convert the file descriptor back
into a SOCKET handle.

NB, the OSF handles are avialable in Win 98, Win Me, Win NT, Win 2000,
Win XP

We don't care about Win 95 so Win CE is the only currently existing
Windows platform we'd be unable to support in this way. I'm personally
not worried by this restriction...

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list