[PATCH 11/20] qemu: Decompose qemuSaveImageOpen

Jim Fehlig via Devel posted 20 patches 3 months, 2 weeks ago
[PATCH 11/20] qemu: Decompose qemuSaveImageOpen
Posted by Jim Fehlig via Devel 3 months, 2 weeks ago
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@suse.com>
---
 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 977763e5d8..87d75b6baa 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5783,9 +5783,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;
 
@@ -5914,15 +5917,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)
@@ -5932,7 +5931,6 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path,
 
  cleanup:
     virQEMUSaveDataFree(data);
-    VIR_FORCE_CLOSE(fd);
     return ret;
 }
 
@@ -5956,8 +5954,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;
@@ -6015,7 +6015,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;
 
@@ -6037,15 +6036,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;
 }
@@ -6101,14 +6099,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 98a1ad638d..125064ab66 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -570,58 +570,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);
@@ -725,6 +702,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 eb2835f11b..e101fdba6e 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -74,17 +74,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 9833fb6206..7d05ce76f4 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2118,11 +2118,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.35.3