Split the reading of libvirt's save image metadata from the opening of the fd that will be passed to QEMU. This allows improved error handling and provides more flexibility for code reading saved images. Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx> --- 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 d2eddbd9ae..75de0b1fd5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5775,9 +5775,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; @@ -5906,15 +5909,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) @@ -5924,7 +5923,6 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, cleanup: virQEMUSaveDataFree(data); - VIR_FORCE_CLOSE(fd); return ret; } @@ -5948,8 +5946,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; @@ -6007,7 +6007,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; @@ -6029,15 +6028,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; } @@ -6093,14 +6091,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 69617e07eb..76d9e96792 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -522,58 +522,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); @@ -671,6 +648,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 0e58dd14b6..a3b9182258 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -69,17 +69,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 80cd54bf33..e5c41fcf67 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2377,11 +2377,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; -- 2.43.0