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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/22/25 16:16, Jim Fehlig wrote:
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.

This patch may be useful on it's own. I've included it, along with another small fix, for consideration outside of this series

https://lists.libvirt.org/archives/list/devel@xxxxxxxxxxxxxxxxx/thread/DATZFZY6ETRYOQ6ORQ2JIVHBTFGKUBJM/

Regards,
Jim


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 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;



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux