[libvirt] [PATCH] Xen Events Updated

[Daniel Veillard]

On Mon, Nov 24, 2008 at 03:44:28PM +0000, Daniel P. Berrange wrote:
> On Fri, Nov 21, 2008 at 02:11:39PM -0500, Ben Guthro wrote:
> > I have integrated your changes, and moved this bit referenced below, 
> > so it works with my setup. Could you please test with yours, and 
> > make sure I didn't break anything?
> 
> It didn't break anything, but I found more edge cases & a couple of bugs
> and optimizations.
> 
> Changes in this version
> 
>  - We now also monitor IN_MOVED_TO and IN_MOVED_FROM, so that we catch
>    'mv foo /etc/xen' and 'mv /etc/xen/foo /tmp' as define & undefine
>    events
>  - On the @introduced watch handler we'll retry if we don't find the
>    UUID / name first time around, to deal with inevitable race while
>    XenD populates this info in xenstore.
>  - In the  @introduced & @removed handlers, simply to just check the
>    domain ID, not name/uuid, since running VMs are unique on ID.
>  - Fix UUID stored to be the raw unsigned char, not NULL terminated
>    string form, so we're passing correct data into virGetDomain.
> 
> This is now working reliably with my RHEL-5  Xen for all events, and
> should be a little more resilent on new Xen too.

  Okay, based on previous reviews and this thread, I just suggest


> +++ b/configure.in	Mon Nov 24 09:08:01 2008 -0500
> @@ -147,6 +147,8 @@ dnl Allow to build without Xen, QEMU/KVM
>  dnl Allow to build without Xen, QEMU/KVM, test or remote driver
>  AC_ARG_WITH([xen],
>  [  --with-xen              add XEN support (on)],[],[with_xen=yes])
> +AC_ARG_WITH([xen-inotify],
> +[  --with-xen-inotify      add XEN inotify support (on)],[],[with_xen_inotify=yes])
>  AC_ARG_WITH([qemu],
>  [  --with-qemu             add QEMU/KVM support (on)],[],[with_qemu=yes])
>  AC_ARG_WITH([uml],
> @@ -333,6 +335,17 @@ AM_CONDITIONAL([WITH_XEN], [test "$with_
>  AM_CONDITIONAL([WITH_XEN], [test "$with_xen" = "yes"])
>  AC_SUBST([XEN_CFLAGS])
>  AC_SUBST([XEN_LIBS])
> +
> +dnl
> +dnl check for kernel headers required by xen_inotify
> +dnl
> +if test "$with_xen_inotify" != "no"; then
> +    AC_CHECK_HEADER([linux/inotify.h],[],[with_xen_inotify=no])
> +fi
> +if test "$with_xen_inotify" = "yes"; then
> +    AC_DEFINE_UNQUOTED([WITH_XEN_INOTIFY], 1,[whether Xen inotify sub-driver is enabled])
> +fi
> +AM_CONDITIONAL([WITH_XEN_INOTIFY], [test "$with_xen_inotify" = "yes"])

  I would suggest we also test there that xen is enable before adding
xen_inotify support. I don't think it makes sense to activate the later
if the former is not selected or detected.

  +1

Daniel

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