all the logic to open a fd, create a wrapper etc, is boilerplate
code that is best reused, so change the Open function to take
an existing already initialized virQEMUSaveFd.

Adapt all callers of qemuSaveImageOpen.

Signed-off-by: Claudio Fontana <cfont...@suse.de>
---
 src/qemu/qemu_driver.c    | 101 ++++++++++++++++++++++----------------
 src/qemu/qemu_saveimage.c |  56 +++++----------------
 src/qemu/qemu_saveimage.h |   9 ++--
 3 files changed, 73 insertions(+), 93 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0c6645ed89..9ad4945928 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5852,12 +5852,13 @@ qemuDomainRestoreInternal(virConnectPtr conn,
     virDomainObj *vm = NULL;
     g_autofree char *xmlout = NULL;
     const char *newxml = dxml;
-    int fd = -1;
     int ret = -1;
     virQEMUSaveData *data = NULL;
-    virFileWrapperFd *wrapperFd = NULL;
+    virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID;
     bool hook_taint = false;
     bool reset_nvram = false;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+    int oflags = O_RDONLY;
 
     virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
                   VIR_DOMAIN_SAVE_RUNNING |
@@ -5867,10 +5868,17 @@ qemuDomainRestoreInternal(virConnectPtr conn,
     if (flags & VIR_DOMAIN_SAVE_RESET_NVRAM)
         reset_nvram = true;
 
-    fd = qemuSaveImageOpen(driver, NULL, path, &def, &data,
-                           (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0,
-                           &wrapperFd, false, false);
-    if (fd < 0)
+    if (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) {
+        if (virFileDirectFdFlag() < 0) {
+            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+                           _("bypass cache unsupported by this system"));
+            return -1;
+        }
+        oflags |= O_DIRECT;
+    }
+    if (virQEMUSaveFdInit(&saveFd, path, oflags, cfg) < 0)
+        return -1;
+    if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
         goto cleanup;
 
     if (ensureACL(conn, def) < 0)
@@ -5924,16 +5932,13 @@ qemuDomainRestoreInternal(virConnectPtr conn,
                             flags) < 0)
         goto cleanup;
 
-    ret = qemuSaveImageStartVM(conn, driver, vm, &fd, data, path,
+    ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path,
                                false, reset_nvram, VIR_ASYNC_JOB_START);
 
     qemuProcessEndJob(vm);
 
  cleanup:
-    VIR_FORCE_CLOSE(fd);
-    if (virFileWrapperFdClose(wrapperFd) < 0)
-        ret = -1;
-    virFileWrapperFdFree(wrapperFd);
+    ret = virQEMUSaveFdFini(&saveFd, vm, ret);
     virQEMUSaveDataFree(data);
     if (vm && ret < 0)
         qemuDomainRemoveInactive(driver, vm);
@@ -5999,15 +6004,15 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const 
char *path,
     virQEMUDriver *driver = conn->privateData;
     char *ret = NULL;
     g_autoptr(virDomainDef) def = NULL;
-    int fd = -1;
     virQEMUSaveData *data = NULL;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+    virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID;
 
     virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL);
 
-    fd = qemuSaveImageOpen(driver, NULL, path, &def, &data,
-                           false, NULL, false, false);
-
-    if (fd < 0)
+    if (virQEMUSaveFdInit(&saveFd, path, O_RDONLY, cfg) < 0)
+        return NULL;
+    if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
         goto cleanup;
 
     if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
@@ -6017,7 +6022,8 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const 
char *path,
 
  cleanup:
     virQEMUSaveDataFree(data);
-    VIR_FORCE_CLOSE(fd);
+    if (virQEMUSaveFdFini(&saveFd, NULL, ret ? 0 : -1) < 0)
+        ret = NULL;
     return ret;
 }
 
@@ -6029,8 +6035,9 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,
     int ret = -1;
     g_autoptr(virDomainDef) def = NULL;
     g_autoptr(virDomainDef) newdef = NULL;
-    int fd = -1;
     virQEMUSaveData *data = NULL;
+    virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
     int state = -1;
 
     virCheckFlags(VIR_DOMAIN_SAVE_RUNNING |
@@ -6041,10 +6048,9 @@ 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 (fd < 0)
+    if (virQEMUSaveFdInit(&saveFd, path, O_RDWR, cfg) < 0)
+        return -1;
+    if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
         goto cleanup;
 
     if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0)
@@ -6071,15 +6077,15 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,
                                              VIR_DOMAIN_XML_MIGRATABLE)))
         goto cleanup;
 
-    if (lseek(fd, 0, SEEK_SET) != 0) {
+    if (lseek(saveFd.fd, 0, SEEK_SET) != 0) {
         virReportSystemError(errno, _("cannot seek in '%s'"), path);
         goto cleanup;
     }
 
-    if (virQEMUSaveDataWrite(data, fd, path) < 0)
+    if (virQEMUSaveDataWrite(data, saveFd.fd, path) < 0)
         goto cleanup;
 
-    if (VIR_CLOSE(fd) < 0) {
+    if (virQEMUSaveFdClose(&saveFd, NULL) < 0) {
         virReportSystemError(errno, _("failed to write header data to '%s'"), 
path);
         goto cleanup;
     }
@@ -6087,8 +6093,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const 
char *path,
     ret = 0;
 
  cleanup:
-    VIR_FORCE_CLOSE(fd);
     virQEMUSaveDataFree(data);
+    ret = virQEMUSaveFdFini(&saveFd, NULL, ret);
     return ret;
 }
 
@@ -6100,8 +6106,9 @@ 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;
+    virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
     qemuDomainObjPrivate *priv;
 
     virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL);
@@ -6122,15 +6129,18 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, 
unsigned int flags)
         goto cleanup;
     }
 
-    if ((fd = qemuSaveImageOpen(driver, priv->qemuCaps, path, &def, &data,
-                                false, NULL, false, false)) < 0)
+    if (virQEMUSaveFdInit(&saveFd, path, O_RDONLY, cfg) < 0)
+        goto cleanup;
+    if (qemuSaveImageOpen(driver, priv->qemuCaps, &def, &data, false,
+                          &saveFd) < 0)
         goto cleanup;
 
     ret = qemuDomainDefFormatXML(driver, priv->qemuCaps, def, flags);
 
  cleanup:
     virQEMUSaveDataFree(data);
-    VIR_FORCE_CLOSE(fd);
+    if (virQEMUSaveFdFini(&saveFd, vm, ret ? 0 : -1) < 0)
+        ret = NULL;
     virDomainObjEndAPI(&vm);
     return ret;
 }
@@ -6180,20 +6190,30 @@ qemuDomainObjRestore(virConnectPtr conn,
 {
     g_autoptr(virDomainDef) def = NULL;
     qemuDomainObjPrivate *priv = vm->privateData;
-    int fd = -1;
     int ret = -1;
     g_autofree char *xmlout = NULL;
     virQEMUSaveData *data = NULL;
-    virFileWrapperFd *wrapperFd = NULL;
+    virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID;
+    int oflags = O_RDONLY;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
 
-    fd = qemuSaveImageOpen(driver, NULL, path, &def, &data,
-                           bypass_cache, &wrapperFd, false, true);
-    if (fd < 0) {
-        if (fd == -3)
+    if (bypass_cache) {
+        if (virFileDirectFdFlag() < 0) {
+            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+                           _("bypass cache unsupported by this system"));
+            return -1;
+        }
+        oflags |= O_DIRECT;
+    }
+    if (virQEMUSaveFdInit(&saveFd, path, oflags, cfg) < 0)
+        goto cleanup;
+    ret = qemuSaveImageOpen(driver, NULL, &def, &data, true, &saveFd);
+    if (ret < 0) {
+        if (ret == -3)
             ret = 1;
         goto cleanup;
     }
-
+    ret = -1;
     if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
         int hookret;
 
@@ -6233,15 +6253,12 @@ qemuDomainObjRestore(virConnectPtr conn,
 
     virDomainObjAssignDef(vm, &def, true, NULL);
 
-    ret = qemuSaveImageStartVM(conn, driver, vm, &fd, data, path,
+    ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path,
                                start_paused, reset_nvram, asyncJob);
 
  cleanup:
     virQEMUSaveDataFree(data);
-    VIR_FORCE_CLOSE(fd);
-    if (virFileWrapperFdClose(wrapperFd) < 0)
-        ret = -1;
-    virFileWrapperFdFree(wrapperFd);
+    ret = virQEMUSaveFdFini(&saveFd, vm, ret);
     return ret;
 }
 
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 2a58bd23b8..dee87ecfad 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -659,61 +659,30 @@ qemuSaveImageGetCompressionProgram(const char 
*imageFormat,
  * @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
+ * @unlink_corrupt: mark the image file for removal if it is corrupted
+ * @saveFd: the save file
  *
- * 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).
+ * Returns 0 on success or -1 on failure.
+ * -3 is a special failure in which the saveFd has been marked for unlinking.
+ * On success, the appropriate fields are filled.
  */
 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)
+                  bool unlink_corrupt,
+                  virQEMUSaveFd *saveFd)
 {
-    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-    VIR_AUTOCLOSE fd = -1;
-    int ret = -1;
+    int ret;
     g_autoptr(virQEMUSaveData) data = NULL;
     g_autoptr(virDomainDef) def = NULL;
-    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;
 
     data = g_new0(virQEMUSaveData, 1);
-    ret = virQEMUSaveDataRead(data, fd, path);
+    ret = virQEMUSaveDataRead(data, saveFd->fd, saveFd->path);
     if (ret < 0) {
         if (unlink_corrupt && ret == -3) {
-            if (unlink(path) < 0) {
-                virReportSystemError(errno,
-                                     _("cannot remove corrupt file: %s"),
-                                     path);
-                return -1;
-            }
+            saveFd->need_unlink = true;
         }
         return ret;
     }
@@ -727,10 +696,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
     *ret_def = g_steal_pointer(&def);
     *ret_data = g_steal_pointer(&data);
 
-    ret = fd;
-    fd = -1;
-
-    return ret;
+    return 0;
 }
 
 int
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index 5768e31fa6..6db4e120cb 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -97,14 +97,11 @@ qemuSaveImageStartVM(virConnectPtr conn,
 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 unlink_corrupt,
+                  virQEMUSaveFd *saveFd)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6);
 
 int
 qemuSaveImageGetCompressionProgram(const char *imageFormat,
-- 
2.26.2

Reply via email to