The migrate hook will affect the migrated guest definition. Allow these domains be marked as tainted in the libxl driver. --- src/libxl/libxl_domain.c | 10 ++++++++++ src/libxl/libxl_domain.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0e26b91..886b40f 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -1135,6 +1135,16 @@ libxlDomainStart(libxlDriverPrivatePtr driver, vm->def, hostdev_flags) < 0) goto cleanup_dom; + if (priv->hookRun) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(vm->def->uuid, uuidstr); + + VIR_WARN("Domain id='%d' name='%s' uuid='%s' is tainted: hook", + vm->def->id, + vm->def->name, + uuidstr); + } + /* Unlock virDomainObj while creating the domain */ virObjectUnlock(vm); diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index af11a2c..3a3890b 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -69,6 +69,8 @@ struct _libxlDomainObjPrivate { char *lockState; struct libxlDomainJobObj job; + + bool hookRun; /* true if there was a hook run over this domain */ }; -- 2.6.6

On 07/27/2016 02:42 PM, Cédric Bosdonnat wrote: Also noticed in the rest of the patches, but I think you forgot to include the SoB tag (Signed-off-by). After this patch it will always returns an error. Although patch 3 of this series does add the "ret = 0" here. I think it should a part of this patch.

Introduce libxl hook and use it for start, prepare, started, stop, stopped, migrate events. --- docs/hooks.html.in | 53 ++++++++++++++++++++++++++++++-- src/libxl/libxl_domain.c | 75 +++++++++++++++++++++++++++++++++++++++++++++ src/libxl/libxl_driver.c | 24 +++++++++++++++ src/libxl/libxl_migration.c | 58 +++++++++++++++++++++++++++++++++++ src/util/virhook.c | 16 +++++++++- src/util/virhook.h | 13 ++++++++ 6 files changed, 236 insertions(+), 3 deletions(-) diff --git a/docs/hooks.html.in b/docs/hooks.html.in index d4f4ac3..11073cb 100644 --- a/docs/hooks.html.in +++ b/docs/hooks.html.in @@ -17,8 +17,10 @@ (<span class="since">since 0.8.0</span>)<br/><br/></li> <li>A QEMU guest is started or stopped (<span class="since">since 0.8.0</span>)<br/><br/></li> - <li>An LXC guest is started or stopped + <li>An LXC guest is started or stopped (<span class="since">since 0.8.0</span>)<br/><br/></li> + <li>A libxl-handled Xen guest is started or stopped + (<span class="since">since 2.1.0</span>)<br/><br/></li> <li>A network is started or stopped or an interface is plugged/unplugged to/from the network (<span class="since">since 1.2.2</span>)<br/><br/></li> @@ -41,7 +43,7 @@ <br/> <h2><a name="names">Script names</a></h2> - <p>At present, there are three hook scripts that can be called:</p> + <p>At present, there are five hook scripts that can be called:</p> <ul> <li><code>/etc/libvirt/hooks/daemon</code><br/><br/> Executed when the libvirt daemon is started, stopped, or reloads @@ -50,6 +52,9 @@ Executed when a QEMU guest is started, stopped, or migrated<br/><br/></li> <li><code>/etc/libvirt/hooks/lxc</code><br /><br/> Executed when an LXC guest is started or stopped</li> + <li><code>/etc/libvirt/hooks/libxl</code><br/><br/> + Executed when a libxl-handled Xen guest is started, stopped, or + migrated<br/><br/></li> <li><code>/etc/libvirt/hooks/network</code><br/><br/> Executed when a network is started or stopped or an interface is plugged/unplugged to/from the network</li> @@ -235,6 +240,50 @@ </li> </ul> + <h5><a name="libxl">/etc/libvirt/hooks/libxl</a></h5> + <ul> + <li>Before a Xen guest is started using libxl driver, the libxl hook + script is called in three locations; if any location fails, the guest + is not started. The first location, <span class="since">since + 2.1.0</span>, is before libvirt performs any resource + labeling, and the hook can allocate resources not managed by + libvirt. This is called as:<br/> + <pre>/etc/libvirt/hooks/libxl guest_name prepare begin -</pre> + The second location, available <span class="since">Since + 2.1.0</span>, occurs after libvirt has finished labeling + all resources, but has not yet started the guest, called as:<br/> + <pre>/etc/libvirt/hooks/libxl guest_name start begin -</pre> + The third location, <span class="since">2.1.0</span>, + occurs after the domain has successfully started up:<br/> + <pre>/etc/libvirt/hooks/libxl guest_name started begin -</pre> + </li> + <li>When a libxl-handled Xen guest is stopped, the libxl hook script + is called in two locations, to match the startup. + First, <span class="since">since 2.1.0</span>, the hook is + called before libvirt restores any labels:<br/> + <pre>/etc/libvirt/hooks/libxl guest_name stopped end -</pre> + Then, after libvirt has released all resources, the hook is + called again, <span class="since">since 2.1.0</span>, to allow + any additional resource cleanup:<br/> + <pre>/etc/libvirt/hooks/libxl guest_name release end -</pre></li> + <li><span class="since">Since 2.1.0</span>, the libxl hook script + is also called at the beginning of incoming migration. It is called + as: <pre>/etc/libvirt/hooks/libxl guest_name migrate begin -</pre> + with domain XML sent to standard input of the script. In this case, + the script acts as a filter and is supposed to modify the domain + XML and print it out on its standard output. Empty output is + identical to copying the input XML without changing it. In case the + script returns failure or the output XML is not valid, incoming + migration will be canceled. This hook may be used, e.g., to change + location of disk images for incoming domains.</li> + <li><span class="since">Since 2.1.0</span>, the libxl hook script + is also called when the libvirtd daemon restarts and reconnects + to previously running Xen domains. If the script fails, the + existing Xen domains will be killed off. It is called as: + <pre>/etc/libvirt/hooks/libxl guest_name reconnect begin -</pre> + </li> + </ul> + <h5><a name="network">/etc/libvirt/hooks/network</a></h5> <ul> <li><span class="since">Since 1.2.2</span>, before a network is started, diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 886b40f..d04dc5e 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -32,6 +32,7 @@ #include "viratomic.h" #include "virfile.h" #include "virerror.h" +#include "virhook.h" #include "virlog.h" #include "virstring.h" #include "virtime.h" @@ -737,6 +738,17 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, hostdev_flags |= VIR_HOSTDEV_SP_USB; #endif + /* now that we know it's stopped call the hook if present */ + if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) { + char *xml = virDomainDefFormat(vm->def, cfg->caps, 0); + + /* we can't stop the operation even if the script raised an error */ + virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name, + VIR_HOOK_LIBXL_OP_STOPPED, VIR_HOOK_SUBOP_END, + NULL, xml, NULL); + VIR_FREE(xml); + } + virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def, hostdev_flags, NULL); @@ -788,6 +800,17 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, VIR_FREE(file); } + /* The "release" hook cleans up additional resources */ + if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) { + char *xml = virDomainDefFormat(vm->def, cfg->caps, 0); + + /* we can't stop the operation even if the script raised an error */ + virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name, + VIR_HOOK_LIBXL_OP_RELEASE, VIR_HOOK_SUBOP_END, + NULL, xml, NULL); + VIR_FREE(xml); + } + if (vm->newDef) { virDomainDefFree(vm->def); vm->def = vm->newDef; @@ -1107,6 +1130,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver, if (virDomainObjSetDefTransient(cfg->caps, driver->xmlopt, vm) < 0) goto cleanup; + /* Run an early hook to set-up missing devices */ + if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) { + char *xml = virDomainDefFormat(vm->def, cfg->caps, 0); + int hookret; + + hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name, + VIR_HOOK_LIBXL_OP_PREPARE, VIR_HOOK_SUBOP_BEGIN, + NULL, xml, NULL); + VIR_FREE(xml); + + /* + * If the script raised an error abort the launch + */ + if (hookret < 0) + goto cleanup_dom; + } + if (virDomainLockProcessStart(driver->lockManager, "xen:///system", vm, @@ -1135,6 +1175,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver, vm->def, hostdev_flags) < 0) goto cleanup_dom; + /* now that we know it is about to start call the hook if present */ + if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) { + char *xml = virDomainDefFormat(vm->def, cfg->caps, 0); + int hookret; + + hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name, + VIR_HOOK_LIBXL_OP_START, VIR_HOOK_SUBOP_BEGIN, + NULL, xml, NULL); + VIR_FREE(xml); + + /* + * If the script raised an error abort the launch + */ + if (hookret < 0) + goto cleanup_dom; + } + if (priv->hookRun) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(vm->def->uuid, uuidstr); @@ -1143,6 +1200,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, vm->def->id, vm->def->name, uuidstr); + } /* Unlock virDomainObj while creating the domain */ @@ -1215,6 +1273,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver, if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); + /* finally we can call the 'started' hook script if any */ + if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) { + char *xml = virDomainDefFormat(vm->def, cfg->caps, 0); + int hookret; + + hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name, + VIR_HOOK_LIBXL_OP_STARTED, VIR_HOOK_SUBOP_BEGIN, + NULL, xml, NULL); + VIR_FREE(xml); + + /* + * If the script raised an error abort the launch + */ + if (hookret < 0) + goto cleanup_dom; + } + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, restore_fd < 0 ? VIR_DOMAIN_EVENT_STARTED_BOOTED : diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 5008c00..f500892 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -42,6 +42,7 @@ #include "virfile.h" #include "viralloc.h" #include "viruuid.h" +#include "virhook.h" #include "vircommand.h" #include "libxl_domain.h" #include "libxl_driver.h" @@ -415,6 +416,29 @@ libxlReconnectDomain(virDomainObjPtr vm, /* Enable domain death events */ libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW); + /* now that we know it's reconnected call the hook if present */ + if (virHookPresent(VIR_HOOK_DRIVER_LIBXL) && + STRNEQ("Domain-0", vm->def->name)) { + char *xml = virDomainDefFormat(vm->def, cfg->caps, 0); + int hookret; + + /* we can't stop the operation even if the script raised an error */ + hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, vm->def->name, + VIR_HOOK_LIBXL_OP_RECONNECT, VIR_HOOK_SUBOP_BEGIN, + NULL, xml, NULL); + VIR_FREE(xml); + if (hookret < 0) { + /* Stop the domain if the hook failed */ + if (virDomainObjIsActive(vm)) { + libxlDomainDestroyInternal(driver, vm); + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_FAILED); + } + goto error; + } + } + + ret = 0; + cleanup: libxl_dominfo_dispose(&d_info); virObjectUnlock(vm); diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index bf285a4..1885d49 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -36,6 +36,7 @@ #include "virstring.h" #include "virobject.h" #include "virthread.h" +#include "virhook.h" #include "rpc/virnetsocket.h" #include "libxl_domain.h" #include "libxl_driver.h" @@ -490,9 +491,11 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, unsigned int flags) { libxlDriverPrivatePtr driver = dconn->privateData; + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); libxlMigrationCookiePtr mig = NULL; virDomainObjPtr vm = NULL; char *hostname = NULL; + char *xmlout = NULL; unsigned short port; char portstr[100]; virURIPtr uri = NULL; @@ -500,6 +503,8 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, size_t nsocks = 0; int nsocks_listen = 0; libxlMigrationDstArgs *args = NULL; + bool taint_hook = false; + libxlDomainObjPrivatePtr priv = NULL; size_t i; int ret = -1; @@ -513,6 +518,51 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, goto error; } + /* Let migration hook filter domain XML */ + if (virHookPresent(VIR_HOOK_DRIVER_LIBXL)) { + char *xml; + int hookret; + + if (!(xml = virDomainDefFormat(*def, cfg->caps, + VIR_DOMAIN_XML_SECURE | + VIR_DOMAIN_XML_MIGRATABLE))) + goto error; + + hookret = virHookCall(VIR_HOOK_DRIVER_LIBXL, (*def)->name, + VIR_HOOK_LIBXL_OP_MIGRATE, VIR_HOOK_SUBOP_BEGIN, + NULL, xml, &xmlout); + VIR_FREE(xml); + + if (hookret < 0) { + goto error; + } else if (hookret == 0) { + if (virStringIsEmpty(xmlout)) { + VIR_DEBUG("Migrate hook filter returned nothing; using the" + " original XML"); + } else { + virDomainDefPtr newdef; + + VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout); + newdef = virDomainDefParseString(xmlout, cfg->caps, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); + if (!newdef) + goto error; + + /* TODO At some stage we will want to have some check of what the user + * did in the hook. */ + + virDomainDefFree(*def); + *def = newdef; + /* We should taint the domain here. However, @vm and therefore + * privateData too are still NULL, so just notice the fact and + * taint it later. */ + taint_hook = true; + } + } + } + + if (!(vm = virDomainObjListAdd(driver->domains, *def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | @@ -520,6 +570,12 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, NULL))) goto error; *def = NULL; + priv = vm->privateData; + + if (taint_hook) { + /* Domain XML has been altered by a hook script. */ + priv->hookRun = true; + } /* Create socket connection to receive migration data */ if (!uri_in) { @@ -640,6 +696,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, } done: + VIR_FREE(xmlout); libxlMigrationCookieFree(mig); if (!uri_in) VIR_FREE(hostname); @@ -647,6 +704,7 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, virURIFree(uri); if (vm) virObjectUnlock(vm); + virObjectUnref(cfg); return ret; } diff --git a/src/util/virhook.c b/src/util/virhook.c index a8422a2..facd74a 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -51,13 +51,15 @@ VIR_ENUM_DECL(virHookSubop) VIR_ENUM_DECL(virHookQemuOp) VIR_ENUM_DECL(virHookLxcOp) VIR_ENUM_DECL(virHookNetworkOp) +VIR_ENUM_DECL(virHookLibxlOp) VIR_ENUM_IMPL(virHookDriver, VIR_HOOK_DRIVER_LAST, "daemon", "qemu", "lxc", - "network") + "network", + "libxl") VIR_ENUM_IMPL(virHookDaemonOp, VIR_HOOK_DAEMON_OP_LAST, "start", @@ -96,6 +98,15 @@ VIR_ENUM_IMPL(virHookNetworkOp, VIR_HOOK_NETWORK_OP_LAST, "unplugged", "updated") +VIR_ENUM_IMPL(virHookLibxlOp, VIR_HOOK_LIBXL_OP_LAST, + "start", + "stopped", + "prepare", + "release", + "migrate", + "started", + "reconnect") + static int virHooksFound = -1; /** @@ -261,6 +272,9 @@ virHookCall(int driver, case VIR_HOOK_DRIVER_LXC: opstr = virHookLxcOpTypeToString(op); break; + case VIR_HOOK_DRIVER_LIBXL: + opstr = virHookLibxlOpTypeToString(op); + break; case VIR_HOOK_DRIVER_NETWORK: opstr = virHookNetworkOpTypeToString(op); } diff --git a/src/util/virhook.h b/src/util/virhook.h index 4015426..205249c 100644 --- a/src/util/virhook.h +++ b/src/util/virhook.h @@ -31,6 +31,7 @@ typedef enum { VIR_HOOK_DRIVER_QEMU, /* QEmu domains related events */ VIR_HOOK_DRIVER_LXC, /* LXC domains related events */ VIR_HOOK_DRIVER_NETWORK, /* network related events */ + VIR_HOOK_DRIVER_LIBXL, /* Xen libxl domains related events */ VIR_HOOK_DRIVER_LAST, } virHookDriverType; @@ -87,6 +88,18 @@ typedef enum { VIR_HOOK_NETWORK_OP_LAST, } virHookNetworkOpType; +typedef enum { + VIR_HOOK_LIBXL_OP_START, /* domain is about to start */ + VIR_HOOK_LIBXL_OP_STOPPED, /* domain has stopped */ + VIR_HOOK_LIBXL_OP_PREPARE, /* domain startup initiated */ + VIR_HOOK_LIBXL_OP_RELEASE, /* domain destruction is over */ + VIR_HOOK_LIBXL_OP_MIGRATE, /* domain is being migrated */ + VIR_HOOK_LIBXL_OP_STARTED, /* domain has started */ + VIR_HOOK_LIBXL_OP_RECONNECT, /* domain is being reconnected by libvirt */ + + VIR_HOOK_LIBXL_OP_LAST, +} virHookLibxlOpType; + int virHookInitialize(void); int virHookPresent(int driver); -- 2.6.6

On Wed, 2016-07-27 at 17:40 +0100, Joao Martins wrote: Usually we have doc within the commit introducing the feature. git log docs/hooks.html.in can confirm it. Indeed, it's used in the qemu driver, but not in the lxc one... I'll add it. I'll resubmit with your comments addressed. Thanks for the review. -- Cedric