Re: [libvirt] [PATCH 0.10.2-maint 2/2] Convert libxl driver to Xen 4.2
Eric Blake wrote:
From: Jim Fehlig <jfehlig(a)suse.com>
Based on a patch originally authored by Daniel De Graaf
http://lists.xen.org/archives/html/xen-devel/2012-05/msg00565.html
This patch converts the Xen libxl driver to support only Xen >= 4.2.
Support for Xen 4.1 libxl is dropped since that version of libxl is
designated 'technology preview' only and is incompatible with Xen 4.2
libxl. Additionally, the default toolstack in Xen 4.1 is still xend,
for which libvirt has a stable, functional driver.
(cherry picked from commit dfa1e1dd5325f3a6bc8865312cde557044bf0b93)
Conflicts:
src/libxl/libxl_conf.c - commit e5e8d5 not backported
src/libxl/libxl_driver.c - commit 1c04f99 not backported
---
configure.ac | 8 +-
docs/drvxen.html.in | 8 +
libvirt.spec.in | 4 +-
src/libxl/libxl_conf.c | 325 ++++++++++---------------------
src/libxl/libxl_conf.h | 16 +-
src/libxl/libxl_driver.c | 490 ++++++++++++++++++++++++++++-------------------
6 files changed, 407 insertions(+), 444 deletions(-)
[...]
static int
libxlMakeVfbList(libxlDriverPrivatePtr driver,
- virDomainDefPtr def, libxl_domain_config *d_config)
+ virDomainDefPtr def,
+ libxl_domain_config *d_config)
{
virDomainGraphicsDefPtr *l_vfbs = def->graphics;
int nvfbs = def->ngraphics;
@@ -750,10 +759,10 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver,
}
for (i = 0; i < nvfbs; i++) {
- libxl_device_vfb_init(&x_vfbs[i], i);
- libxl_device_vkb_init(&x_vkbs[i], i);
+ libxl_device_vfb_init(&x_vfbs[i]);
+ libxl_device_vkb_init(&x_vkbs[i]);
FYI, the vfb and vkb devid are not initialized here, resulting in hangs
of PV guests on xen-unstable. I've submitted a patch to xen-devel to
initialize devid for these devices to a sane value in libxl
http://lists.xen.org/archives/html/xen-devel/2013-01/msg00137.html
Hmm, it might be best to initialize devid in libxlMakeVfbList() anyhow,
similar to libxlMakeNicList().
[...]
@@ -84,6 +98,163 @@ libxlDriverUnlock(libxlDriverPrivatePtr driver)
virMutexUnlock(&driver->lock);
}
+
+static void libxlFDEventCallback(int watch ATTRIBUTE_UNUSED,
+ int fd,
+ int vir_events,
+ void *fdinfo)
+{
+ struct libxlOSEventHookFDInfo *info = fdinfo;
+ int events = 0;
+
+ if (vir_events & VIR_EVENT_HANDLE_READABLE)
+ events |= POLLIN;
+ if (vir_events & VIR_EVENT_HANDLE_WRITABLE)
+ events |= POLLOUT;
+ if (vir_events & VIR_EVENT_HANDLE_ERROR)
+ events |= POLLERR;
+ if (vir_events & VIR_EVENT_HANDLE_HANGUP)
+ events |= POLLHUP;
+
+ libxl_osevent_occurred_fd(info->priv->ctx, info->xl_priv, fd, 0, events);
+}
Another FYI, recent testing with xen-unstable has uncovered a race
between the fd (and timer) callback and libxl deregistering the fd (or
timer). I.e. libxl calls libxlFDDeregisterEventHook(), sometime later
the libxl libxlFDEventCallback() is invoked and we call into libxl with
stale xl_priv. I've been experimenting with a patch to resolve this
issue but not satisfied with it quite yet.
[...]
@@ -125,17 +292,6 @@ libxlDomainEventQueue(libxlDriverPrivatePtr
driver, virDomainEventPtr event)
virDomainEventStateQueue(driver->domainEventState, event);
}
-/*
- * Remove reference to domain object.
- */
-static void
-libxlDomainObjUnref(void *data)
-{
- virDomainObjPtr vm = data;
-
- virObjectUnref(vm);
-}
-
In hindsight, I wonder if this and associated changes are correct. I've
also noticed a race where we call virDomainObjUnlock() on a vm that has
already been freed. E.g. in libxlDomainSaveFlags(), we call
virDomainObjUnlock() on vm, but that vm may have already been reaped
(and libxlDomainObjPrivateFree() already called) via libxlEventHandler().
That said, the races I've mentioned exist in the original patch. I've
reviewed this backport and it looks good IMO.
Regards,
Jim