[libvirt] [PATCH 0/5] Refactor save image opening and add restore hook
This series refactors (splits) the save image open function into separate chunks and introduces a filter-hook that is called when restoring a save image. Peter Krempa (5): qemu: save image: Split out user provided XML checker qemu: save image: Add possibility to return XML stored in the image qemu: save image: Split out new definition check/update qemu: save image: Split out checks done only when editing the save img qemu: hook: Provide hook when restoring a domain save image docs/hooks.html.in | 11 +++ src/qemu/qemu_driver.c | 261 ++++++++++++++++++++++++++++++++++--------------- src/util/virhook.c | 3 +- src/util/virhook.h | 1 + 4 files changed, 195 insertions(+), 81 deletions(-) -- 2.1.0
Extract code used to check save image XMLs provided by users to separate use. --- src/qemu/qemu_driver.c | 100 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 66 insertions(+), 34 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 15ad64d..e41a08e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5311,6 +5311,68 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn, return ret; } + +/** + * qemuDomainSaveImageUpdateDef: + * @driver: qemu driver data + * @def: def of the domain from the save image + * @newxml: user provided replacement XML + * + * Returns the new domain definition in case @newxml is ABI compatible with the + * guest. + */ +static virDomainDefPtr +qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver, + virDomainDefPtr def, + const char *newxml) +{ + virDomainDefPtr ret = NULL; + virDomainDefPtr newdef_migr = NULL; + virDomainDefPtr newdef = NULL; + virCapsPtr caps = NULL; + + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + + if (!(newdef = virDomainDefParseString(newxml, caps, driver->xmlopt, + QEMU_EXPECTED_VIRT_TYPES, + VIR_DOMAIN_XML_INACTIVE))) + goto cleanup; + + if (!(newdef_migr = qemuDomainDefCopy(driver, + newdef, + VIR_DOMAIN_XML_MIGRATABLE))) + goto cleanup; + + if (!virDomainDefCheckABIStability(def, newdef_migr)) { + virResetLastError(); + + /* Due to a bug in older version of external snapshot creation + * code, the XML saved in the save image was not a migratable + * XML. To ensure backwards compatibility with the change of the + * saved XML type, we need to check the ABI compatibility against + * the user provided XML if the check against the migratable XML + * fails. Snapshots created prior to v1.1.3 have this issue. */ + if (!virDomainDefCheckABIStability(def, newdef)) + goto cleanup; + + /* use the user provided XML */ + ret = newdef; + newdef = NULL; + } else { + ret = newdef_migr; + newdef_migr = NULL; + } + + cleanup: + virObjectUnref(caps); + virDomainDefFree(newdef); + virDomainDefFree(newdef_migr); + + return ret; +} + + /* Return -1 on most failures after raising error, -2 if edit was specified * but xmlin and state (-1 for no change, 0 for paused, 1 for running) do * not represent any changes (no error raised), -3 if corrupt image was @@ -5431,45 +5493,15 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, QEMU_EXPECTED_VIRT_TYPES, VIR_DOMAIN_XML_INACTIVE))) goto error; - if (xmlin) { - virDomainDefPtr def2 = NULL; - virDomainDefPtr newdef = NULL; - if (!(def2 = virDomainDefParseString(xmlin, caps, driver->xmlopt, - QEMU_EXPECTED_VIRT_TYPES, - VIR_DOMAIN_XML_INACTIVE))) - goto error; + if (xmlin) { + virDomainDefPtr tmp; - newdef = qemuDomainDefCopy(driver, def2, VIR_DOMAIN_XML_MIGRATABLE); - if (!newdef) { - virDomainDefFree(def2); + if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlin))) goto error; - } - - if (!virDomainDefCheckABIStability(def, newdef)) { - virDomainDefFree(newdef); - virResetLastError(); - - /* Due to a bug in older version of external snapshot creation - * code, the XML saved in the save image was not a migratable - * XML. To ensure backwards compatibility with the change of the - * saved XML type, we need to check the ABI compatibility against - * the user provided XML if the check against the migratable XML - * fails. Snapshots created prior to v1.1.3 have this issue. */ - if (!virDomainDefCheckABIStability(def, def2)) { - virDomainDefFree(def2); - goto error; - } - - /* use the user provided XML */ - newdef = def2; - def2 = NULL; - } else { - virDomainDefFree(def2); - } virDomainDefFree(def); - def = newdef; + def = tmp; } VIR_FREE(xml); -- 2.1.0
On Wed, Sep 17, 2014 at 17:18:35 +0200, Peter Krempa wrote:
Extract code used to check save image XMLs provided by users to separate use. --- src/qemu/qemu_driver.c | 100 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 66 insertions(+), 34 deletions(-)
ACK Jirka
Add a new parameter that will allow to return the XML stored in the save image for further manipulation and adjust the callers. This option will be used in later patches. --- src/qemu/qemu_driver.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e41a08e..a276ea5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5382,6 +5382,7 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, const char *path, virDomainDefPtr *ret_def, virQEMUSaveHeaderPtr ret_header, + char **xmlout, bool bypass_cache, virFileWrapperFdPtr *wrapperFd, const char *xmlin, int state, bool edit, @@ -5504,7 +5505,10 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, def = tmp; } - VIR_FREE(xml); + if (xmlout) + *xmlout = xml; + else + VIR_FREE(xml); *ret_def = def; *ret_header = header; @@ -5660,7 +5664,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, else if (flags & VIR_DOMAIN_SAVE_PAUSED) state = 0; - fd = qemuDomainSaveImageOpen(driver, path, &def, &header, + fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, &wrapperFd, dxml, state, false, false); if (fd < 0) @@ -5721,8 +5725,8 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, /* We only take subset of virDomainDefFormat flags. */ virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); - fd = qemuDomainSaveImageOpen(driver, path, &def, &header, false, NULL, - NULL, -1, false, false); + fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, + false, NULL, NULL, -1, false, false); if (fd < 0) goto cleanup; @@ -5759,8 +5763,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, else if (flags & VIR_DOMAIN_SAVE_PAUSED) state = 0; - fd = qemuDomainSaveImageOpen(driver, path, &def, &header, false, NULL, - dxml, state, true, false); + fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, + false, NULL, dxml, state, true, false); if (fd < 0) { /* Check for special case of no change needed. */ @@ -5824,7 +5828,7 @@ qemuDomainObjRestore(virConnectPtr conn, virQEMUSaveHeader header; virFileWrapperFdPtr wrapperFd = NULL; - fd = qemuDomainSaveImageOpen(driver, path, &def, &header, + fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, bypass_cache, &wrapperFd, NULL, -1, false, true); if (fd < 0) { -- 2.1.0
On Wed, Sep 17, 2014 at 17:18:36 +0200, Peter Krempa wrote:
Add a new parameter that will allow to return the XML stored in the save image for further manipulation and adjust the callers. This option will be used in later patches. --- src/qemu/qemu_driver.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
ACK Jirka
Split out the call to the update method only to places where it is actually used rather than having a mega-method that does all the stuff. --- src/qemu/qemu_driver.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a276ea5..0151fd2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5495,16 +5495,6 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, VIR_DOMAIN_XML_INACTIVE))) goto error; - if (xmlin) { - virDomainDefPtr tmp; - - if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlin))) - goto error; - - virDomainDefFree(def); - def = tmp; - } - if (xmlout) *xmlout = xml; else @@ -5647,6 +5637,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, { virQEMUDriverPtr driver = conn->privateData; virDomainDefPtr def = NULL; + virDomainDefPtr newdef = NULL; virDomainObjPtr vm = NULL; int fd = -1; int ret = -1; @@ -5673,6 +5664,14 @@ qemuDomainRestoreFlags(virConnectPtr conn, if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) goto cleanup; + if (dxml) { + if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml))) + goto cleanup; + + virDomainDefFree(def); + def = newdef; + } + if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, VIR_DOMAIN_OBJ_LIST_ADD_LIVE | @@ -5749,6 +5748,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, virQEMUDriverPtr driver = conn->privateData; int ret = -1; virDomainDefPtr def = NULL; + virDomainDefPtr newdef = NULL; int fd = -1; virQEMUSaveHeader header; char *xml = NULL; @@ -5776,7 +5776,10 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0) goto cleanup; - xml = qemuDomainDefFormatXML(driver, def, + if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml))) + goto cleanup; + + xml = qemuDomainDefFormatXML(driver, newdef, VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE); @@ -5807,6 +5810,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, cleanup: virDomainDefFree(def); + virDomainDefFree(newdef); VIR_FORCE_CLOSE(fd); VIR_FREE(xml); return ret; -- 2.1.0
On Wed, Sep 17, 2014 at 17:18:37 +0200, Peter Krempa wrote:
Split out the call to the update method only to places where it is actually used rather than having a mega-method that does all the stuff. --- src/qemu/qemu_driver.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)
It was easier to review this patch squashed into the following one since it allowed me to check you correctly updated all callers of qemuDomainSaveImageOpen. ACK Jirka
Move them to the single corresponding function rather than having them in the common chunk of code. --- src/qemu/qemu_driver.c | 76 +++++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0151fd2..1d82e93 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5373,10 +5373,22 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver, } -/* Return -1 on most failures after raising error, -2 if edit was specified - * but xmlin and state (-1 for no change, 0 for paused, 1 for running) do - * not represent any changes (no error raised), -3 if corrupt image was - * unlinked (no error raised), and opened fd on success. */ +/** + * qemuDomainSaveImageOpen: + * @driver: qemu driver data + * @path: path of the save image + * @ret_def: returns domain definition created from the XML stored in the image + * @ret_header: returns structure filled with data from the image header + * @xmlout: returns the XML from the image file (may be NULL) + * @bypass_cache: bypass cache when opening the file + * @wrapperFd: returns the file wrapper structure + * @open_write: open the file for writing (for updates) + * @unlink_corrupt: remove the image file if it is corrupted + * + * Returns the opened fd of the save image file and fills the apropriate fields + * on success. On error returns -1 on most failures, -3 if corrupt image was + * unlinked (no error raised). + */ static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) qemuDomainSaveImageOpen(virQEMUDriverPtr driver, const char *path, @@ -5385,14 +5397,14 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, char **xmlout, bool bypass_cache, virFileWrapperFdPtr *wrapperFd, - const char *xmlin, int state, bool edit, + bool open_write, bool unlink_corrupt) { int fd = -1; virQEMUSaveHeader header; char *xml = NULL; virDomainDefPtr def = NULL; - int oflags = edit ? O_RDWR : O_RDONLY; + int oflags = open_write ? O_RDWR : O_RDONLY; virCapsPtr caps = NULL; if (bypass_cache) { @@ -5477,18 +5489,6 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, goto error; } - if (edit && STREQ(xml, xmlin) && - (state < 0 || state == header.was_running)) { - VIR_FREE(xml); - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("cannot close file: %s"), path); - goto error; - } - return -2; - } - if (state >= 0) - header.was_running = state; - /* Create a domain from this XML */ if (!(def = virDomainDefParseString(xml, caps, driver->xmlopt, QEMU_EXPECTED_VIRT_TYPES, @@ -5643,21 +5643,15 @@ qemuDomainRestoreFlags(virConnectPtr conn, int ret = -1; virQEMUSaveHeader header; virFileWrapperFdPtr wrapperFd = NULL; - int state = -1; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED, -1); - if (flags & VIR_DOMAIN_SAVE_RUNNING) - state = 1; - else if (flags & VIR_DOMAIN_SAVE_PAUSED) - state = 0; - fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, - &wrapperFd, dxml, state, false, false); + &wrapperFd, false, false); if (fd < 0) goto cleanup; @@ -5680,6 +5674,11 @@ qemuDomainRestoreFlags(virConnectPtr conn, goto cleanup; def = NULL; + if (flags & VIR_DOMAIN_SAVE_RUNNING) + header.was_running = 1; + else if (flags & VIR_DOMAIN_SAVE_PAUSED) + header.was_running = 0; + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -5725,7 +5724,7 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, virCheckFlags(VIR_DOMAIN_XML_SECURE, NULL); fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, - false, NULL, NULL, -1, false, false); + false, NULL, false, false); if (fd < 0) goto cleanup; @@ -5763,22 +5762,30 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, else if (flags & VIR_DOMAIN_SAVE_PAUSED) state = 0; - fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, - false, NULL, dxml, state, true, false); + fd = qemuDomainSaveImageOpen(driver, path, &def, &header, &xml, + false, NULL, true, false); - if (fd < 0) { - /* Check for special case of no change needed. */ - if (fd == -2) - ret = 0; + if (fd < 0) goto cleanup; - } if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0) goto cleanup; + if (STREQ(xml, dxml) && + (state < 0 || state == header.was_running)) { + /* no change to the XML */ + ret = 0; + goto cleanup; + } + + if (state > 0) + header.was_running = state; + if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml))) goto cleanup; + VIR_FREE(xml); + xml = qemuDomainDefFormatXML(driver, newdef, VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE | @@ -5833,8 +5840,7 @@ qemuDomainObjRestore(virConnectPtr conn, virFileWrapperFdPtr wrapperFd = NULL; fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, - bypass_cache, &wrapperFd, NULL, -1, false, - true); + bypass_cache, &wrapperFd, false, true); if (fd < 0) { if (fd == -3) ret = 1; -- 2.1.0
On Wed, Sep 17, 2014 at 17:18:38 +0200, Peter Krempa wrote:
Move them to the single corresponding function rather than having them in the common chunk of code. --- src/qemu/qemu_driver.c | 76 +++++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0151fd2..1d82e93 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c ... @@ -5477,18 +5489,6 @@ qemuDomainSaveImageOpen(virQEMUDriverPtr driver, goto error; }
- if (edit && STREQ(xml, xmlin) && - (state < 0 || state == header.was_running)) { - VIR_FREE(xml); - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("cannot close file: %s"), path); - goto error; - } - return -2; - } - if (state >= 0) - header.was_running = state; - /* Create a domain from this XML */ if (!(def = virDomainDefParseString(xml, caps, driver->xmlopt, QEMU_EXPECTED_VIRT_TYPES, ... @@ -5763,22 +5762,30 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, else if (flags & VIR_DOMAIN_SAVE_PAUSED) state = 0;
- fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, - false, NULL, dxml, state, true, false); + fd = qemuDomainSaveImageOpen(driver, path, &def, &header, &xml, + false, NULL, true, false);
- if (fd < 0) { - /* Check for special case of no change needed. */ - if (fd == -2) - ret = 0; + if (fd < 0) goto cleanup; - }
if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0) goto cleanup;
+ if (STREQ(xml, dxml) && + (state < 0 || state == header.was_running)) { + /* no change to the XML */ + ret = 0; + goto cleanup; + } + + if (state > 0) + header.was_running = state;
I believe the condition should be state >= 0.
+ if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml))) goto cleanup;
+ VIR_FREE(xml); + xml = qemuDomainDefFormatXML(driver, newdef, VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE | ...
ACK Jirka
On 09/22/14 15:41, Jiri Denemark wrote:
On Wed, Sep 17, 2014 at 17:18:38 +0200, Peter Krempa wrote:
Move them to the single corresponding function rather than having them in the common chunk of code. --- src/qemu/qemu_driver.c | 76 +++++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 35 deletions(-)
I've pushed patches 1-4 with the suggested fixes. Thanks. Peter
--- docs/hooks.html.in | 11 ++++++++ src/qemu/qemu_driver.c | 69 +++++++++++++++++++++++++++++++++++++++++++++----- src/util/virhook.c | 3 ++- src/util/virhook.h | 1 + 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/docs/hooks.html.in b/docs/hooks.html.in index 07b9d49..265dbb7 100644 --- a/docs/hooks.html.in +++ b/docs/hooks.html.in @@ -177,6 +177,17 @@ 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 1.2.9</span>, the qemu hook script is + also called when restoring a saved image either via the API or + automatically when restoring a managed save machine. It is called + as: <pre>/etc/libvirt/hooks/qemu guest_name restore 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, restore of the + image will be aborted. This hook may be used, e.g., to change + location of disk images for incoming domains.</li> <li><span class="since">Since 0.9.13</span>, the qemu hook script is also called when the libvirtd daemon restarts and reconnects to previously running QEMU processes. If the script fails, the diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1d82e93..2dd2e48 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5636,20 +5636,23 @@ qemuDomainRestoreFlags(virConnectPtr conn, unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; + qemuDomainObjPrivatePtr priv = NULL; virDomainDefPtr def = NULL; - virDomainDefPtr newdef = NULL; virDomainObjPtr vm = NULL; + char *xml = NULL; + char *xmlout = NULL; int fd = -1; int ret = -1; virQEMUSaveHeader header; virFileWrapperFdPtr wrapperFd = NULL; + bool hook_taint = false; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED, -1); - fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, + fd = qemuDomainSaveImageOpen(driver, path, &def, &header, &xml, (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, &wrapperFd, false, false); if (fd < 0) @@ -5658,12 +5661,29 @@ qemuDomainRestoreFlags(virConnectPtr conn, if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) goto cleanup; - if (dxml) { - if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml))) + if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { + int hookret; + + if ((hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, def->name, + VIR_HOOK_QEMU_OP_RESTORE, + VIR_HOOK_SUBOP_BEGIN, + NULL, + dxml ? dxml : xml, + &xmlout)) < 0) goto cleanup; - virDomainDefFree(def); - def = newdef; + if (hookret == 0 && xmlout) { + virDomainDefPtr tmp; + + VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout); + + if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlout))) + goto cleanup; + + virDomainDefFree(def); + def = tmp; + hook_taint = true; + } } if (!(vm = virDomainObjListAdd(driver->domains, def, @@ -5679,6 +5699,11 @@ qemuDomainRestoreFlags(virConnectPtr conn, else if (flags & VIR_DOMAIN_SAVE_PAUSED) header.was_running = 0; + if (hook_taint) { + priv = vm->privateData; + priv->hookRun = true; + } + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -5697,6 +5722,8 @@ qemuDomainRestoreFlags(virConnectPtr conn, cleanup: virDomainDefFree(def); VIR_FORCE_CLOSE(fd); + VIR_FREE(xml); + VIR_FREE(xmlout); virFileWrapperFdFree(wrapperFd); if (vm) virObjectUnlock(vm); @@ -5834,12 +5861,15 @@ qemuDomainObjRestore(virConnectPtr conn, bool bypass_cache) { virDomainDefPtr def = NULL; + qemuDomainObjPrivatePtr priv = vm->privateData; int fd = -1; int ret = -1; + char *xml = NULL; + char *xmlout = NULL; virQEMUSaveHeader header; virFileWrapperFdPtr wrapperFd = NULL; - fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, + fd = qemuDomainSaveImageOpen(driver, path, &def, &header, &xml, bypass_cache, &wrapperFd, false, true); if (fd < 0) { if (fd == -3) @@ -5847,6 +5877,29 @@ qemuDomainObjRestore(virConnectPtr conn, goto cleanup; } + if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { + int hookret; + + if ((hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, def->name, + VIR_HOOK_QEMU_OP_RESTORE, + VIR_HOOK_SUBOP_BEGIN, + NULL, xml, &xmlout)) < 0) + goto cleanup; + + if (hookret == 0 && xmlout) { + virDomainDefPtr tmp; + + VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout); + + if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlout))) + goto cleanup; + + virDomainDefFree(def); + def = tmp; + priv->hookRun = true; + } + } + if (STRNEQ(vm->def->name, def->name) || memcmp(vm->def->uuid, def->uuid, VIR_UUID_BUFLEN)) { char vm_uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -5870,6 +5923,8 @@ qemuDomainObjRestore(virConnectPtr conn, VIR_WARN("Failed to close %s", path); cleanup: + VIR_FREE(xml); + VIR_FREE(xmlout); virDomainDefFree(def); VIR_FORCE_CLOSE(fd); virFileWrapperFdFree(wrapperFd); diff --git a/src/util/virhook.c b/src/util/virhook.c index ac7b40f..25d0783 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -77,7 +77,8 @@ VIR_ENUM_IMPL(virHookQemuOp, VIR_HOOK_QEMU_OP_LAST, "migrate", "started", "reconnect", - "attach") + "attach", + "restore") VIR_ENUM_IMPL(virHookLxcOp, VIR_HOOK_LXC_OP_LAST, "start", diff --git a/src/util/virhook.h b/src/util/virhook.h index 5bc0a5f..550ef84 100644 --- a/src/util/virhook.h +++ b/src/util/virhook.h @@ -60,6 +60,7 @@ typedef enum { VIR_HOOK_QEMU_OP_STARTED, /* domain has started */ VIR_HOOK_QEMU_OP_RECONNECT, /* domain is being reconnected by libvirt */ VIR_HOOK_QEMU_OP_ATTACH, /* domain is being attached to be libvirt */ + VIR_HOOK_QEMU_OP_RESTORE, /* domain is being restored */ VIR_HOOK_QEMU_OP_LAST, } virHookQemuOpType; -- 2.1.0
On Wed, Sep 17, 2014 at 17:18:39 +0200, Peter Krempa wrote:
--- docs/hooks.html.in | 11 ++++++++ src/qemu/qemu_driver.c | 69 +++++++++++++++++++++++++++++++++++++++++++++----- src/util/virhook.c | 3 ++- src/util/virhook.h | 1 + 4 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/docs/hooks.html.in b/docs/hooks.html.in index 07b9d49..265dbb7 100644 --- a/docs/hooks.html.in +++ b/docs/hooks.html.in @@ -177,6 +177,17 @@ 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 1.2.9</span>, the qemu hook script is + also called when restoring a saved image either via the API or + automatically when restoring a managed save machine. It is called + as: <pre>/etc/libvirt/hooks/qemu guest_name restore 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, restore of the + image will be aborted. This hook may be used, e.g., to change + location of disk images for incoming domains.</li>
Copy&paste? "incoming domains" does not make a lot of sense in case of restore :-)
<li><span class="since">Since 0.9.13</span>, the qemu hook script is also called when the libvirtd daemon restarts and reconnects to previously running QEMU processes. If the script fails, the diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1d82e93..2dd2e48 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5636,20 +5636,23 @@ qemuDomainRestoreFlags(virConnectPtr conn, unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; + qemuDomainObjPrivatePtr priv = NULL; virDomainDefPtr def = NULL; - virDomainDefPtr newdef = NULL; virDomainObjPtr vm = NULL; + char *xml = NULL; + char *xmlout = NULL; int fd = -1; int ret = -1; virQEMUSaveHeader header; virFileWrapperFdPtr wrapperFd = NULL; + bool hook_taint = false;
virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED, -1);
- fd = qemuDomainSaveImageOpen(driver, path, &def, &header, NULL, + fd = qemuDomainSaveImageOpen(driver, path, &def, &header, &xml, (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, &wrapperFd, false, false); if (fd < 0) @@ -5658,12 +5661,29 @@ qemuDomainRestoreFlags(virConnectPtr conn, if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) goto cleanup;
- if (dxml) { - if (!(newdef = qemuDomainSaveImageUpdateDef(driver, def, dxml))) + if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { + int hookret; + + if ((hookret = virHookCall(VIR_HOOK_DRIVER_QEMU, def->name, + VIR_HOOK_QEMU_OP_RESTORE, + VIR_HOOK_SUBOP_BEGIN, + NULL, + dxml ? dxml : xml, + &xmlout)) < 0) goto cleanup;
- virDomainDefFree(def); - def = newdef; + if (hookret == 0 && xmlout) { + virDomainDefPtr tmp; + + VIR_DEBUG("Using hook-filtered domain XML: %s", xmlout); + + if (!(tmp = qemuDomainSaveImageUpdateDef(driver, def, xmlout))) + goto cleanup; + + virDomainDefFree(def); + def = tmp; + hook_taint = true; + }
I think you wanted to write this in a bit different way... This way dxml is ignored when the hook is not present.
}
if (!(vm = virDomainObjListAdd(driver->domains, def,
... Jirka
participants (2)
-
Jiri Denemark -
Peter Krempa