From: Peter Krempa <pkrempa@redhat.com> Always instantiate a 'virFileWrapperFd' (iohelper) to wrap the saveimage file descriptor of a non-'sparse' format saveimage. For 'sparse' images we also need to ensure that the FD returned when opening the save image is an actual file FD (thus not the FD from the helper process used to bypass root-squashed NFS) as qemu requires an actual file in those cases. This patch reworks 'qemuSaveImageOpen' to create the wrapper process based on whether the 'wrapperFd' variable is non-NULL rather than based on a combination of 'sparse' and 'bypass_cache' flags. The caller will then based on the image format and the need for the wrapper use the appropriate settings. As with this patch all non-sparse images will always pass a pipe instead of a file to qemu it also fixes problems with qemu-11.0 where the 'fd' migration protocol rejects FDs which point to a file. Resolves: https://issues.redhat.com/browse/RHEL-76301 Closes: https://gitlab.com/libvirt/libvirt/-/issues/850 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 32 ++++++++++++++++++++++++++------ src/qemu/qemu_saveimage.c | 24 +++++++++++++----------- src/qemu/qemu_saveimage.h | 4 +++- src/qemu/qemu_snapshot.c | 12 ++++++++++-- 4 files changed, 52 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c2d810c013..03cdbd9dc3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5813,6 +5813,7 @@ qemuDomainRestoreInternal(virConnectPtr conn, bool hook_taint = false; bool reset_nvram = false; bool sparse = false; + bool bypass_cache = (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0; g_autoptr(qemuMigrationParams) restoreParams = NULL; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | @@ -5844,11 +5845,30 @@ qemuDomainRestoreInternal(virConnectPtr conn, (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE)))) goto cleanup; - fd = qemuSaveImageOpen(driver, path, - (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, - sparse, &wrapperFd, false); - if (fd < 0) - goto cleanup; + if (sparse) { + if ((fd = qemuSaveImageOpen(driver, path, bypass_cache, NULL, false)) < 0) + goto cleanup; + + /* In sparse mode the FD needs to be a real file as qemu accesses it + * directly thus: + * - ensure that we actualy got a file FD + * - there's no need to seek it as qemu does it itself + */ + if (!virFileFDIsRegular(fd)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("path '%1$s' can't be opened directly (without the use of helper proces) which is incompatible with 'sparse' save image"), + path); + goto cleanup; + } + } else { + if ((fd = qemuSaveImageOpen(driver, path, bypass_cache, &wrapperFd, false)) < 0) + goto cleanup; + + /* When virFileWrapperFD is used 'fd' can't be seeked so to make it + * point to the data read the header */ + if (qemuSaveImageFDSkipHeader(fd) < 0) + goto cleanup; + } if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { int hookret; @@ -6039,7 +6059,7 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, conn, &def, &data) < 0) goto cleanup; - fd = qemuSaveImageOpen(driver, path, false, false, NULL, true); + fd = qemuSaveImageOpen(driver, path, false, NULL, true); if (fd < 0) goto cleanup; diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index a3519f8538..64fbcd5f51 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -312,6 +312,13 @@ qemuSaveImageReadHeader(int fd, virQEMUSaveData **ret_data) } +int +qemuSaveImageFDSkipHeader(int fd) +{ + return qemuSaveImageReadHeader(fd, NULL); +} + + /** * qemuSaveImageDecompressionStart: * @data: data from memory state file @@ -697,7 +704,6 @@ qemuSaveImageGetMetadata(virQEMUDriver *driver, * @driver: qemu driver data * @path: path of the save image * @bypass_cache: bypass cache when opening the file - * @sparse: Image contains mapped-ram save format * @wrapperFd: returns the file wrapper structure * @open_write: open the file for writing (for updates) * @@ -707,7 +713,6 @@ int qemuSaveImageOpen(virQEMUDriver *driver, const char *path, bool bypass_cache, - bool sparse, virFileWrapperFd **wrapperFd, bool open_write) { @@ -729,16 +734,13 @@ qemuSaveImageOpen(virQEMUDriver *driver, if ((fd = qemuDomainOpenFile(cfg, NULL, path, oflags, NULL)) < 0) return -1; - /* If sparse, no need for the iohelper or positioning the file pointer. */ - if (!sparse) { - if (bypass_cache && - !(*wrapperFd = virFileWrapperFdNew(&fd, path, - VIR_FILE_WRAPPER_BYPASS_CACHE))) - return -1; + if (wrapperFd) { + unsigned int fdflags = VIR_FILE_WRAPPER_NON_BLOCKING; + + if (bypass_cache) + fdflags |= VIR_FILE_WRAPPER_BYPASS_CACHE; - /* Read the header to position the file pointer for QEMU. Unfortunately we - * can't use lseek with virFileWrapperFD. */ - if (qemuSaveImageReadHeader(fd, NULL) < 0) + if (!(*wrapperFd = virFileWrapperFdNew(&fd, path, fdflags))) return -1; } diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 887545899f..0d5a383212 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -92,7 +92,6 @@ int qemuSaveImageOpen(virQEMUDriver *driver, const char *path, bool bypass_cache, - bool sparse, virFileWrapperFd **wrapperFd, bool open_write) ATTRIBUTE_NONNULL(2); @@ -127,6 +126,9 @@ qemuSaveImageCreate(virDomainObj *vm, unsigned int flags, virDomainAsyncJob asyncJob); +int +qemuSaveImageFDSkipHeader(int fd); + int virQEMUSaveDataWrite(virQEMUSaveData *data, int fd, diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 70e4c37144..82ae38ca29 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2423,6 +2423,7 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm, typedef struct _qemuSnapshotRevertMemoryData { int fd; char *path; + virFileWrapperFd *wrapperFd; virQEMUSaveData *data; } qemuSnapshotRevertMemoryData; @@ -2430,6 +2431,8 @@ static void qemuSnapshotClearRevertMemoryData(qemuSnapshotRevertMemoryData *memdata) { VIR_FORCE_CLOSE(memdata->fd); + ignore_value(virFileWrapperFdClose(memdata->wrapperFd)); + virFileWrapperFdFree(memdata->wrapperFd); virQEMUSaveDataFree(memdata->data); } G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuSnapshotRevertMemoryData, qemuSnapshotClearRevertMemoryData); @@ -2506,10 +2509,15 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, return -1; memdata->fd = qemuSaveImageOpen(driver, memdata->path, - false, false, NULL, false); + false, &memdata->wrapperFd, false); if (memdata->fd < 0) return -1; + /* If 'wrapperFd' is used the FD can't be seeked so we need to make + * it point to the actual data, thus seek across the header */ + if (qemuSaveImageFDSkipHeader(memdata->fd) < 0) + return -1; + if (!virDomainDefCheckABIStability(savedef, domdef, driver->xmlopt)) return -1; } @@ -2705,7 +2713,7 @@ qemuSnapshotRevertActive(virDomainObj *vm, bool defined = false; int rc; g_autoptr(virDomainSnapshotDef) tmpsnapdef = NULL; - g_auto(qemuSnapshotRevertMemoryData) memdata = { -1, NULL, NULL }; + g_auto(qemuSnapshotRevertMemoryData) memdata = { .fd = -1 }; bool started = false; start_flags |= VIR_QEMU_PROCESS_START_PAUSED; -- 2.53.0