Re: [PATCH V2 11/20] qemu: Decompose qemuSaveImageOpen

Split the reading of libvirt's save image metadata from the opening of the fd that will be passed to QEMU. This provides flexibility for an upcoming patch adding mapped-ram support for restore.

Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/qemu/qemu_driver.c | 37 ++++++++-------- src/qemu/qemu_saveimage.c | 89 ++++++++++++++++++++++++--------------- src/qemu/qemu_saveimage.h | 16 ++++--- src/qemu/qemu_snapshot.c | 9 ++-- 4 files changed, 89 insertions(+), 62 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 716eddba09..5afc2ea846 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5792,9 +5792,12 @@ qemuDomainRestoreInternal(virConnectPtr conn, if (flags & VIR_DOMAIN_SAVE_RESET_NVRAM) reset_nvram = true; - fd = qemuSaveImageOpen(driver, NULL, path, &def, &data, + if (qemuSaveImageGetMetadata(driver, NULL, path, &def, &data, false) < 0) + goto cleanup; + + fd = qemuSaveImageOpen(driver, path, (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, - &wrapperFd, false, false); + &wrapperFd, false); if (fd < 0) goto cleanup; @@ -5923,15 +5926,11 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, virQEMUDriver *driver = conn->privateData; char *ret = NULL; g_autoptr(virDomainDef) def = NULL; - int fd = -1; virQEMUSaveData *data = NULL; virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL); - fd = qemuSaveImageOpen(driver, NULL, path, &def, &data, - false, NULL, false, false); - - if (fd < 0) + if (qemuSaveImageGetMetadata(driver, NULL, path, &def, &data, false) < 0) goto cleanup; if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0) @@ -5941,7 +5940,6 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, cleanup: virQEMUSaveDataFree(data); - VIR_FORCE_CLOSE(fd); return ret; } @@ -5965,8 +5963,10 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, else if (flags & VIR_DOMAIN_SAVE_PAUSED) state = 0; - fd = qemuSaveImageOpen(driver, NULL, path, &def, &data, - false, NULL, true, false); + if (qemuSaveImageGetMetadata(driver, NULL, path, &def, &data, false) < 0) + goto cleanup; + + fd = qemuSaveImageOpen(driver, path, 0, NULL, false); if (fd < 0) goto cleanup; @@ -6024,7 +6024,6 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) g_autofree char *path = NULL; char *ret = NULL; g_autoptr(virDomainDef) def = NULL; - int fd = -1; virQEMUSaveData *data = NULL; qemuDomainObjPrivate *priv; @@ -6046,15 +6045,14 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) goto cleanup; } - if ((fd = qemuSaveImageOpen(driver, priv->qemuCaps, path, &def, &data, - false, NULL, false, false)) < 0) + if (qemuSaveImageGetMetadata(driver, priv->qemuCaps, path, + &def, &data, false) < 0) goto cleanup; ret = qemuDomainDefFormatXML(driver, priv->qemuCaps, def, flags); cleanup: virQEMUSaveDataFree(data); - VIR_FORCE_CLOSE(fd); virDomainObjEndAPI(&vm); return ret; } @@ -6110,14 +6108,17 @@ qemuDomainObjRestore(virConnectPtr conn, virQEMUSaveData *data = NULL; virFileWrapperFd *wrapperFd = NULL; - fd = qemuSaveImageOpen(driver, NULL, path, &def, &data, - bypass_cache, &wrapperFd, false, true); - if (fd < 0) { - if (fd == -3) + ret = qemuSaveImageGetMetadata(driver, NULL, path, &def, &data, true); + if (ret < 0) { + if (ret == -3) ret = 1; goto cleanup; } + fd = qemuSaveImageOpen(driver, path, bypass_cache, &wrapperFd, false); + if (fd < 0) + goto cleanup; + if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { int hookret; diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 245bbf9dfc..3a71c23c01 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -536,58 +536,35 @@ qemuSaveImageGetCompressionProgram(const char *imageFormat, /** - * qemuSaveImageOpen: + * qemuSaveImageGetMetadata: * @driver: qemu driver data * @qemuCaps: pointer to qemuCaps if the domain is running or NULL * @path: path of the save image * @ret_def: returns domain definition created from the XML stored in the image * @ret_data: returns structure filled with data from the image header - * @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 appropriate fields - * on success. On error returns -1 on most failures, -3 if corrupt image was - * unlinked (no error raised). + * Open the save image file, read libvirt's save image metadata, and populate + * the @ret_def and @ret_data structures. Returns 0 on success and -1 on most + * failures. Returns -3 if corrupt image was unlinked (no error raised). */ int -qemuSaveImageOpen(virQEMUDriver *driver, - virQEMUCaps *qemuCaps, - const char *path, - virDomainDef **ret_def, - virQEMUSaveData **ret_data, - bool bypass_cache, - virFileWrapperFd **wrapperFd, - bool open_write, - bool unlink_corrupt) +qemuSaveImageGetMetadata(virQEMUDriver *driver, + virQEMUCaps *qemuCaps, + const char *path, + virDomainDef **ret_def, + virQEMUSaveData **ret_data, + bool unlink_corrupt) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); VIR_AUTOCLOSE fd = -1; - int ret = -1; g_autoptr(virQEMUSaveData) data = NULL; virQEMUSaveHeader *header; g_autoptr(virDomainDef) def = NULL; - int oflags = open_write ? O_RDWR : O_RDONLY; size_t xml_len; size_t cookie_len; - if (bypass_cache) { - int directFlag = virFileDirectFdFlag(); - if (directFlag < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("bypass cache unsupported by this system")); - return -1; - } - oflags |= directFlag; - } - - if ((fd = qemuDomainOpenFile(cfg, NULL, path, oflags, NULL)) < 0) - return -1; - - if (bypass_cache && - !(*wrapperFd = virFileWrapperFdNew(&fd, path, - VIR_FILE_WRAPPER_BYPASS_CACHE))) + if ((fd = qemuDomainOpenFile(cfg, NULL, path, O_RDONLY, NULL)) < 0) return -1; data = g_new0(virQEMUSaveData, 1); @@ -685,6 +662,50 @@ qemuSaveImageOpen(virQEMUDriver *driver, *ret_def = g_steal_pointer(&def); *ret_data = g_steal_pointer(&data); + return 0; +} + + +/** + * qemuSaveImageOpen: + * @driver: qemu driver data + * @path: path of the save image + * @bypass_cache: bypass cache when opening the file + * @wrapperFd: returns the file wrapper structure + * @open_write: open the file for writing (for updates) + * + * Returns the opened fd of the save image file on success, -1 on failure. + */ +int +qemuSaveImageOpen(virQEMUDriver *driver, + const char *path, + bool bypass_cache, + virFileWrapperFd **wrapperFd, + bool open_write) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + VIR_AUTOCLOSE fd = -1; + int ret = -1; + int oflags = open_write ? O_RDWR : O_RDONLY; + + if (bypass_cache) { + int directFlag = virFileDirectFdFlag(); + if (directFlag < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("bypass cache unsupported by this system")); + return -1; + } + oflags |= directFlag; + } + + if ((fd = qemuDomainOpenFile(cfg, NULL, path, oflags, NULL)) < 0) + return -1; + + if (bypass_cache && + !(*wrapperFd = virFileWrapperFdNew(&fd, path, + VIR_FILE_WRAPPER_BYPASS_CACHE))) + return -1; + ret = fd; fd = -1; diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 481e280d75..7340925274 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -89,17 +89,21 @@ qemuSaveImageStartVM(virConnectPtr conn, virDomainAsyncJob asyncJob) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6); +int +qemuSaveImageGetMetadata(virQEMUDriver *driver, + virQEMUCaps *qemuCaps, + const char *path, + virDomainDef **ret_def, + virQEMUSaveData **ret_data, + bool unlink_corrupt) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); + int qemuSaveImageOpen(virQEMUDriver *driver, - virQEMUCaps *qemuCaps, const char *path, - virDomainDef **ret_def, - virQEMUSaveData **ret_data, bool bypass_cache, virFileWrapperFd **wrapperFd, - bool open_write, - bool unlink_corrupt) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + bool open_write); int qemuSaveImageGetCompressionProgram(const char *imageFormat, diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 07883d67fa..5991455a4e 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2389,11 +2389,12 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, g_autoptr(virDomainDef) savedef = NULL; memdata->path = snapdef->memorysnapshotfile; - memdata->fd = qemuSaveImageOpen(driver, NULL, memdata->path, - &savedef, &memdata->data, - false, NULL, - false, false); + if (qemuSaveImageGetMetadata(driver, NULL, memdata->path, &savedef, + &memdata->data, false) < 0) + return -1; + memdata->fd = qemuSaveImageOpen(driver, memdata->path, + false, NULL, false); if (memdata->fd < 0) return -1;