qemu: saveimage: Use 'virFileWrapperFd' when loading non-sparse saveimage

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>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 739e4d5db6c2b59635ed8beb03933785f9b3b7a4..3126d997de06a730fe1fff7f79a92ae1aa5b297f 100644 (file)
--- 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 a3519f8538c92b6938821b81c8cbb0039e58ca6a..64fbcd5f519aaf11ced84655ba3e7486b6e4e98d 100644 (file)
--- 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 887545899f79a48e6d8c09aa4fa28ebee5995d3b..0d5a383212c40782927360e712adb62639d542ea 100644 (file)
--- 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 70e4c37144d343fe787636ba6cfdb35f88d3fe1f..82ae38ca29f15b9454a11775b6cd50c6bdb6ba09 100644 (file)
--- 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;