[libvirt RFCv8 09/27] qemu: saveimage: convert qemuSaveImageOpen to use virQEMUSaveFd

Claudio Fontana cfontana at suse.de
Sat May 7 13:43:02 UTC 2022


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 <cfontana at suse.de>
---
 src/qemu/qemu_driver.c    | 101 ++++++++++++++++++++++----------------
 src/qemu/qemu_saveimage.c |  86 ++++++++------------------------
 src/qemu/qemu_saveimage.h |   9 ++--
 3 files changed, 83 insertions(+), 113 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ce399cd197..f3d5f3937d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5824,12 +5824,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 |
@@ -5840,10 +5841,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, 0, oflags, cfg) < 0)
+        return -1;
+    if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
         goto cleanup;
 
     if (ensureACL(conn, def) < 0)
@@ -5897,16 +5905,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);
@@ -5964,15 +5969,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, 0, O_RDONLY, cfg) < 0)
+        return NULL;
+    if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
         goto cleanup;
 
     if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
@@ -5982,7 +5987,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;
 }
 
@@ -5994,8 +6000,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 |
@@ -6006,10 +6013,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, 0, O_RDWR, cfg) < 0)
+        return -1;
+    if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
         goto cleanup;
 
     if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0)
@@ -6036,15 +6042,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;
     }
@@ -6052,8 +6058,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
     ret = 0;
 
  cleanup:
-    VIR_FORCE_CLOSE(fd);
     virQEMUSaveDataFree(data);
+    ret = virQEMUSaveFdFini(&saveFd, NULL, ret);
     return ret;
 }
 
@@ -6065,8 +6071,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);
@@ -6088,15 +6095,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, 0, 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;
 }
@@ -6147,20 +6157,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, 0, 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;
 
@@ -6200,15 +6220,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 30e9a16307..33807fe373 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -514,94 +514,53 @@ 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;
     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)))
-        return -1;
 
     data = g_new0(virQEMUSaveData, 1);
 
     header = &data->header;
-    if (saferead(fd, header, sizeof(*header)) != sizeof(*header)) {
-        if (unlink_corrupt) {
-            if (unlink(path) < 0) {
-                virReportSystemError(errno,
-                                     _("cannot remove corrupt file: %s"),
-                                     path);
-                return -1;
-            } else {
-                return -3;
-            }
-        }
-
+    if (saferead(saveFd->fd, header, sizeof(*header)) != sizeof(*header)) {
         virReportError(VIR_ERR_OPERATION_FAILED,
                        "%s", _("failed to read qemu header"));
+        if (unlink_corrupt) {
+            saveFd->need_unlink = true;
+            return -3;
+        }
         return -1;
     }
 
     if (memcmp(header->magic, QEMU_SAVE_MAGIC, sizeof(header->magic)) != 0) {
         if (memcmp(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic)) == 0) {
+            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+                           _("save image is incomplete"));
             if (unlink_corrupt) {
-                if (unlink(path) < 0) {
-                    virReportSystemError(errno,
-                                         _("cannot remove corrupt file: %s"),
-                                         path);
-                    return -1;
-                } else {
-                    return -3;
-                }
+                saveFd->need_unlink = true;
+                return -3;
             }
-
+        } else {
             virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                           _("save image is incomplete"));
-            return -1;
+                           _("image magic is incorrect"));
         }
-
-        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-                       _("image magic is incorrect"));
         return -1;
     }
 
@@ -632,7 +591,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
 
     data->xml = g_new0(char, xml_len);
 
-    if (saferead(fd, data->xml, xml_len) != xml_len) {
+    if (saferead(saveFd->fd, data->xml, xml_len) != xml_len) {
         virReportError(VIR_ERR_OPERATION_FAILED,
                        "%s", _("failed to read domain XML"));
         return -1;
@@ -641,7 +600,7 @@ qemuSaveImageOpen(virQEMUDriver *driver,
     if (cookie_len > 0) {
         data->cookie = g_new0(char, cookie_len);
 
-        if (saferead(fd, data->cookie, cookie_len) != cookie_len) {
+        if (saferead(saveFd->fd, data->cookie, cookie_len) != cookie_len) {
             virReportError(VIR_ERR_OPERATION_FAILED, "%s",
                            _("failed to read cookie"));
             return -1;
@@ -657,10 +616,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 41937e5eb5..5dc63f3661 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -92,14 +92,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.35.3



More information about the libvir-list mailing list