[libvirt] [PATCH V5] Add libxenlight driver
Daniel Veillard
veillard at redhat.com
Thu Mar 10 08:54:52 UTC 2011
On Wed, Mar 09, 2011 at 11:45:49PM -0700, Jim Fehlig wrote:
> Add a new xen driver based on libxenlight [1], which is the primary
> toolstack starting with Xen 4.1.0. The driver is stateful, runs
> privileged only, and is accessed with libxl:/// URI.
>
> V5:
> - Ensure events are unregistered when domain private data
> is destroyed. Discovered and fixed by Markus Gross.
>
> V4:
> - Handle restart of libvirtd, reconnecting to previously
> started domains
> - Rebased to current master
> - Tested against Xen 4.1 RC7-pre (c/s 22961:c5d121fd35c0)
>
> V3:
> - Reserve vnc port within driver when autoport=yes
>
> V2:
> - Update to Xen 4.1 RC6-pre (c/s 22940:5a4710640f81)
> - Rebased to current master
> - Plug memory leaks found by Stefano Stabellini and valgrind
> - Handle SHUTDOWN_crash domain death event
>
> [1] http://lists.xensource.com/archives/html/xen-devel/2009-11/msg00436.html
[...]
> @@ -497,6 +499,46 @@ fi
> AC_SUBST([LIBXENSERVER_CFLAGS])
> AC_SUBST([LIBXENSERVER_LIBS])
>
> +old_LIBS="$LIBS"
> +old_CFLAGS="$CFLAGS"
> +LIBXL_LIBS=""
> +LIBXL_CFLAGS=""
> +dnl search for libxl, aka libxenlight
> +fail=0
> +if test "$with_libxl" != "no" ; then
> + if test "$with_libxl" != "yes" && test "$with_libxl" != "check" ; then
> + LIBXL_CFLAGS="-I$with_libxl/include"
> + LIBXL_LIBS="-L$with_libxl"
> + fi
> + CFLAGS="$CFLAGS $LIBXL_CFLAGS"
> + LIBS="$LIBS $LIBXL_LIBS"
> + AC_CHECK_LIB([xenlight], [libxl_ctx_init], [
> + with_libxl=yes
> + LIBXL_LIBS="$LIBXL_LIBS -lxenlight -lxenstore -lxenctrl -lxenguest -luuid -lutil -lblktapctl"
> + ],[
> + if test "$with_libxl" = "yes"; then
> + fail=1
> + fi
> + with_libxl=no
Small indentation logic problem here
> + ],[
> + -lxenstore -lxenctrl -lxenguest -luuid -lutil -lblktapctl
> + ])
> +fi
[...]
> @@ -2370,6 +2412,7 @@ AC_MSG_NOTICE([ OpenVZ: $with_openvz])
> AC_MSG_NOTICE([ VMware: $with_vmware])
> AC_MSG_NOTICE([ VBox: $with_vbox])
> AC_MSG_NOTICE([ XenAPI: $with_xenapi])
> +AC_MSG_NOTICE([libxenlight: $with_libxl])
instead
AC_MSG_NOTICE([ libxl: $with_libxl])
or
AC_MSG_NOTICE([xenlight: $with_libxl])
would keep messages nicely aligned
> AC_MSG_NOTICE([ LXC: $with_lxc])
> AC_MSG_NOTICE([ PHYP: $with_phyp])
> AC_MSG_NOTICE([ ONE: $with_one])
> @@ -2479,6 +2522,11 @@ AC_MSG_NOTICE([ xenapi: $LIBXENSERVER_CFLAGS $LIBXENSERVER_LIBS])
[...]
> diff --git a/docs/drvxenlight.html.in b/docs/drvxenlight.html.in
> new file mode 100644
> index 0000000..fe60786
> --- /dev/null
> +++ b/docs/drvxenlight.html.in
> @@ -0,0 +1,53 @@
> +<html>
> + <body>
> + <h1>libxenlight hypervisor driver</h1>
> +
> + <ul id="toc"></ul>
> +
> + <p>
> + The libvirt libxenlight driver provides the ability to manage virtual
> + machines on any Xen release from 4.1.0 onwards.
> + </p>
> +
> + <h2><a name="prereq">Deployment pre-requisites</a></h2>
> +
> + <p>
> + This driver uses Xen's libxenlight toolstack, which is the default
> + toolstack configuration starting with Xen 4.1.0. The traditional
> + xm/xend toolstack is still provided, but it is no longer maintained
> + and may be removed in a future Xen release.
> + </p>
> +
> + <p>
> + The libxenlight toolstack uses xenstored and blktap2. Ensure
> + xenstored is running, or use the xencommons init script provided.
> + Ensure your kernel supports the blktap2 module and it is loaded.
> + </p>
> +
> + <h2><a name="uri">Connections to libxenlight driver</a></h2>
> +
> + <p>
> + The libvirt libxenlight driver is a stateful, privileged driver,
> + with a driver name of 'libxl'. Some example conection URIs for
> + the libxenlight driver are:
> + </p>
> +
> +<pre>
> +libxl:/// (local access, direct)
> +libxl://example.com/ (remote access, TLS/x509)
> +libxl+tcp://example.com/ (remote access, SASl/Kerberos)
> +libxl+ssh://root@example.com/ (remote access, SSH tunnelled)
> +</pre>
> +
> + <h2><a name="xmlconfig">Example domain XML config</a></h2>
> +
> + <p>
> + The libxenlight toolstack attempts to be compatible with the
> + legacy xm/xend toolstack, supporting the traditional python
> + configuration files (xen-xm). Fortunately, the libvirt XML
> + syntax is unchanged with the libxenlight driver. Consult
> + the <a href="drvxen.html#xmlconfig">Xen driver examples</a>.
> + </p>
> +
> + </body>
> +</html>
It's great to have documentation, but ...
I just regret that we are unable to hide how we connect to the Xen
server, after all libvirt was precisely designed to try to minimize
the change on the application stack as the lower layers of
virtualization evolves, and here we fail. Sure the URI is a very minimal
part compared to the actual XML description and code but the fact we are
using a different driver internally could possibly be masked to the user.
Can we make an attempt at hiding how we connect to Xen here like we
did with the "unified" driver but while keeping with different
subdirectories and drivers. Ideally if virt-manager could connect though
the new stack without knowing, then that mean we suceeded and really
adding value here. I understand that the new driver is well "new" so
possibly we could use an heuristic or user preference to detect which
driver to pick during the transition for target which may hold both
stacks.
[...]
> +if WITH_LIBXL
> + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/libxl"
> + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/libxl"
> +endif
Seems libvirt.spec.in also need to get a patch for those paths
[...]
> +static virCapsPtr
> +libxlMakeCapabilitiesInternal(const char *hostmachine,
> + libxl_physinfo *phy_info,
> + char *capabilities)
> +{
> + char *str, *token;
> + regmatch_t subs[4];
> + char *saveptr = NULL;
> + int i;
> +
> + int host_pae = 0;
> + struct guest_arch guest_archs[32];
> + int nr_guest_archs = 0;
> + virCapsPtr caps = NULL;
> +
> + memset(guest_archs, 0, sizeof(guest_archs));
> +
> + // TODO: extract pae from phy_info->phys_cap
> + // for now, better default is 1
> + (void)phy_info;
> + host_pae = 1;
Can we fix this before commiting this ;-) ?
There is also a couple of TODO in the network device code for MTU and
IP, I wonder how urgent that is really.
> + /* Split capabilities string into tokens. strtok_r is OK here because
> + * we "own" the buffer. Parse out the features from each token.
> + */
> + for (str = capabilities, nr_guest_archs = 0;
> + nr_guest_archs < sizeof guest_archs / sizeof guest_archs[0]
<taste> I really prefer sizeof(foo) to sizeof foo </taste>
[...]
> + if (hvm) {
> + b_info->u.hvm.pae = def->features & (1 << VIR_DOMAIN_FEATURE_PAE);
> + b_info->u.hvm.apic = def->features & (1 << VIR_DOMAIN_FEATURE_APIC);
> + b_info->u.hvm.acpi = def->features & (1 << VIR_DOMAIN_FEATURE_ACPI);
> + /*
> + * The following comment and calculation were taken directly from
> + * libxenlight's internal function libxl_get_required_shadow_memory():
> + *
> + * 256 pages (1MB) per vcpu, plus 1 page per MiB of RAM for the P2M map,
> + * plus 1 page per MiB of RAM to shadow the resident processes.
> + */
> + b_info->shadow_memkb = 4 * (256 * b_info->cur_vcpus +
> + 2 * (b_info->max_memkb / 1024));
hum, reminds me of the old xen days :-)
[...]
> +/*
> + * Start a domain through libxenlight.
> + *
> + * virDomainObjPtr should be locked on invocation
> + */
> +static int
> +libxlVmStart(libxlDriverPrivatePtr driver,
> + virDomainObjPtr vm, bool start_paused)
> +{
[...]
> + if(libxl_userdata_store(&priv->ctx, domid, "libvirt-xml",
if ( with a space please
> + (uint8_t *)dom_xml, strlen(dom_xml) + 1)) {
> + libxlError(VIR_ERR_INTERNAL_ERROR,
> + _("libxenlight failed to store userdata"));
> + goto error;
> + }
Bonus point for the Xen guys here, the per-domain data storage is a
great idea, suits us well and simplify data handling a lot ! I assume
it follows on migrations etc... Maybe we should register "libvirt-xml"
in their small registry in tools/libxl/libxl.h
[...]
> +static void
> +libxlReconnectDomain(void *payload,
> + const void *name ATTRIBUTE_UNUSED,
> + void *opaque)
> +{
> + virDomainObjPtr vm = payload;
> + libxlDriverPrivatePtr driver = opaque;
> + int rc;
> + libxl_dominfo d_info;
> + int len;
> + uint8_t *data = NULL;
> +
> + virDomainObjLock(vm);
> +
> + /* Does domain still exist? */
> + rc = libxl_domain_info(&driver->ctx, &d_info, vm->def->id);
> + if (rc == ERROR_INVAL) {
> + goto out;
> + } else if (rc != 0) {
> + VIR_DEBUG("libxl_domain_info failed (code %d), ignoring domain %d",
> + rc, vm->def->id);
> + goto out;
> + }
> +
> + /* Is this a domain that was under libvirt control? */
> + if (libxl_userdata_retrieve(&driver->ctx, vm->def->id,
> + "libvirt-xml", &data, &len)) {
> + VIR_DEBUG("libxl_userdata_retrieve failed, ignoring domain %d", vm->def->id);
It would be nice to grab in domains created by other means if we have
a chance.
[...]
> +static int
> +libxlStartup(int privileged) {
> + const libxl_version_info *ver_info;
> + char *log_file = NULL;
> +
> + /* Check that the user is root, silently disable if not */
> + if (!privileged) {
> + VIR_INFO0("Not running privileged, disabling libxenlight driver");
> + return 0;
> + }
okay so fo local management we will always go though remote to talk to
the libvirtd daemon, right ?
> +
> +static virDrvOpenStatus
> +libxlOpen(virConnectPtr conn,
> + virConnectAuthPtr auth ATTRIBUTE_UNUSED,
> + int flags ATTRIBUTE_UNUSED)
> +{
> + if (conn->uri == NULL) {
> + if (libxl_driver == NULL)
> + return VIR_DRV_OPEN_DECLINED;
> +
> + conn->uri = xmlParseURI("libxl:///");
> + if (!conn->uri) {
> + virReportOOMError();
> + return VIR_DRV_OPEN_ERROR;
> + }
> + } else {
> + if (conn->uri->scheme == NULL || STRNEQ(conn->uri->scheme, "libxl"))
> + return VIR_DRV_OPEN_DECLINED;
> +
> + /* If server name is given, its for remote driver */
> + if (conn->uri->server != NULL)
> + return VIR_DRV_OPEN_DECLINED;
> +
> + if (libxl_driver == NULL) {
> + libxlError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("libxenlight state driver is not active"));
> + return VIR_DRV_OPEN_ERROR;
> + }
> +
> + /* /session isn't supported in libxenlight */
> + if (conn->uri->path &&
> + STRNEQ(conn->uri->path, "") &&
> + STRNEQ(conn->uri->path, "/") &&
> + STRNEQ(conn->uri->path, "/system")) {
> + libxlError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected Xen URI path '%s', try libxl:///"),
> + NULLSTR(conn->uri->path));
> + return VIR_DRV_OPEN_ERROR;
> + }
> + }
okay, based on my comment above maybe the driver could be hooked to
xen:/// in some ways.
> + conn->privateData = libxl_driver;
> +
> + return VIR_DRV_OPEN_SUCCESS;
> +};
[...]
> +static int
> +libxlNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
> +{
[...]
> + info->mhz = phy_info.cpu_khz / 1000;
I wonder what led them to keep a khz granularity...
[...]
> +static int
> +libxlDomainShutdown(virDomainPtr dom)
> +{
> + libxlDriverPrivatePtr driver = dom->conn->privateData;
> + virDomainObjPtr vm;
[...]
> + if (libxl_domain_shutdown(&priv->ctx, dom->id, LIBXL_DOM_REQ_POWEROFF) != 0) {
> + libxlError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to shutdown domain '%d' with libxenlight"),
> + dom->id);
> + goto cleanup;
> + }
> +
> + /* vm is marked shutoff (or removed from domains list if not persistent)
> + * in shutdown event handler.
> + */
> + ret = 0;
[...]
> +}
> +
> +static int
> +libxlDomainReboot(virDomainPtr dom, unsigned int flags ATTRIBUTE_UNUSED)
> +{
[...]
> + if (libxl_domain_shutdown(&priv->ctx, dom->id, LIBXL_DOM_REQ_REBOOT) != 0) {
> + libxlError(VIR_ERR_INTERNAL_ERROR,
> + _("Failed to reboot domain '%d' with libxenlight"),
> + dom->id);
> + goto cleanup;
> + }
> + ret = 0;
> +
> +cleanup:
> + if (vm)
> + virDomainObjUnlock(vm);
> + libxlDriverUnlock(driver);
> + return ret;
> +}
Okay, I tried t make sure libxl_domain_shutdown() is really
asynchronous in all cases, I somehow failed, can you confirm ?
[...]
Okay, overall I would tend to ACK that patch based purely on the code,
but I would like to get first a small discussion about somehow merging
it in the xen:/// framework. Once commited it will be hard to change
and impossible after a release, so we need to decide there before
pushing it IMHO.
Option might be if the default xen driver isn't registered, or
make both exclusive, or a temporary user environment, but I will have
a slight feeling of failure if we can't get to hide properly the
underneath change,
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/
More information about the libvir-list
mailing list