On Mon, Mar 21, 2011 at 10:36:55AM +0100, Thibault VINCENT wrote:
> >From 827261dbe2ab8f1cfd8c84df14bb42fd04df2307 Mon Sep 17 00:00:00 2001
> From: Thibault VINCENT <thibault.vincent@xxxxxxxxxxxx>
> Date: Mon, 21 Mar 2011 10:18:06 +0100
> Subject: [PATCH] qemu: add two hook script events "prepare" and "release"
>
> * Fix for bug #618970
>
> * The "prepare" hook is called very early in the VM statup process
> before device labeling, so that it can allocate ressources not
> managed by libvirt, such as DRBD, or for instance create missing
> bridges and vlan interfaces.
>
> * To shutdown these devices, the "release" hook is also required at
> the very end of the VM destruction.
>
> Signed-off-by: Thibault VINCENT <thibault.vincent@xxxxxxxxxxxx>
> ---
> src/qemu/qemu_process.c | 26 ++++++++++++++++++++++++++
> src/util/hooks.c | 4 +++-
> src/util/hooks.h | 2 ++
> 3 files changed, 31 insertions(+), 1 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 793a43c..7831c3b 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1927,6 +1927,22 @@ int qemuProcessStart(virConnectPtr conn,
>
> vm->def->id = driver->nextvmid++;
>
> + /* Run a early hook to set-up missing devices */
> + if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
> + char *xml = virDomainDefFormat(vm->def, 0);
> + int hookret;
> +
> + hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
> + VIR_HOOK_QEMU_OP_PREPARE, VIR_HOOK_SUBOP_BEGIN,
> NULL, xml);
> + VIR_FREE(xml);
> +
> + /*
> + * If the script raised an error abort the launch
> + */
> + if (hookret < 0)
> + goto cleanup;
> + }
Hum, the main problem I see there is that we may fail startup there
and go to cleanup. In that case the domain is not started but no hook is
called to reverse the operation. I wonder if VIR_HOOK_QEMU_OP_RELEASE
shouldn't be called in the cleanup ... if present, to avoid resource
leak when startup fails.
> /* Must be run before security labelling */
> VIR_DEBUG0("Preparing host devices");
> if (qemuPrepareHostDevices(driver, vm->def) < 0)
> @@ -2419,6 +2435,16 @@ retry:
> VIR_FREE(priv->vcpupids);
> priv->nvcpupids = 0;
>
> + /* The "release" hook cleans up additional ressources */
> + if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
> + char *xml = virDomainDefFormat(vm->def, 0);
> +
> + /* we can't stop the operation even if the script raised an
> error */
> + virHookCall(VIR_HOOK_DRIVER_QEMU, vm->def->name,
> + VIR_HOOK_QEMU_OP_RELEASE, VIR_HOOK_SUBOP_END, NULL,
> xml);
> + VIR_FREE(xml);
> + }
> +
That's okay, but possibly not sufficient.
> if (vm->newDef) {
> virDomainDefFree(vm->def);
> vm->def = vm->newDef;
> diff --git a/src/util/hooks.c b/src/util/hooks.c
> index 5ba2036..c258318 100644
> --- a/src/util/hooks.c
> +++ b/src/util/hooks.c
> @@ -70,8 +70,10 @@ VIR_ENUM_IMPL(virHookSubop, VIR_HOOK_SUBOP_LAST,
> "end")
>
> VIR_ENUM_IMPL(virHookQemuOp, VIR_HOOK_QEMU_OP_LAST,
> + "prepare",
> "start",
> - "stopped")
> + "stopped",
> + "release")
prepare should be added after stopped to avoid changing the order.
> VIR_ENUM_IMPL(virHookLxcOp, VIR_HOOK_LXC_OP_LAST,
> "start",
> diff --git a/src/util/hooks.h b/src/util/hooks.h
> index f311ed1..66b378a 100644
> --- a/src/util/hooks.h
> +++ b/src/util/hooks.h
> @@ -52,8 +52,10 @@ enum virHookSubopType {
> };
>
> enum virHookQemuOpType {
> + VIR_HOOK_QEMU_OP_PREPARE, /* domain startup initiated */
> VIR_HOOK_QEMU_OP_START, /* domain is about to start */
> VIR_HOOK_QEMU_OP_STOPPED, /* domain has stopped */
> + VIR_HOOK_QEMU_OP_RELEASE, /* domain destruction is over */
same thing, only add at the end.
> VIR_HOOK_QEMU_OP_LAST,
> };
Fixing the 2 last problems is trivial and I did it in my tree, but I
don't feel like commiting this without handling the failure to start
the domain (running VIR_HOOK_QEMU_OP_RELEASE with a new virHookSubopType
VIR_HOOK_SUBOP_FAILED for example).
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list