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@suse.de>
---
src/qemu/qemu_driver.c | 100 +++++++++++++++++++++++---------------
src/qemu/qemu_saveimage.c | 86 ++++++++------------------------
src/qemu/qemu_saveimage.h | 9 ++--
3 files changed, 82 insertions(+), 113 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ff2be6ffe9..e4cdd93b50 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5825,12 +5825,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 |
@@ -5841,10 +5842,18 @@ 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)
@@ -5898,16 +5907,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);
@@ -5969,15 +5975,16 @@ 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 (virQEMUSaveFdInit(&saveFd, path, 0, O_RDONLY, cfg) < 0)
+ return NULL;
- if (fd < 0)
+ if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
goto cleanup;
if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
@@ -5987,7 +5994,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;
}
@@ -5999,22 +6007,23 @@ 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 |
VIR_DOMAIN_SAVE_PAUSED, -1);
+ if (virQEMUSaveFdInit(&saveFd, path, 0, O_RDWR, cfg) < 0)
+ return -1;
+
if (flags & VIR_DOMAIN_SAVE_RUNNING)
state = 1;
else if (flags & VIR_DOMAIN_SAVE_PAUSED)
state = 0;
- fd = qemuSaveImageOpen(driver, NULL, path, &def, &data,
- false, NULL, true, false);
-
- if (fd < 0)
+ if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
goto cleanup;
if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0)
@@ -6041,15 +6050,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;
}
@@ -6057,8 +6066,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
ret = 0;
cleanup:
- VIR_FORCE_CLOSE(fd);
virQEMUSaveDataFree(data);
+ ret = virQEMUSaveFdFini(&saveFd, NULL, ret);
return ret;
}
@@ -6070,8 +6079,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);
@@ -6093,15 +6103,19 @@ 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;
}
@@ -6152,16 +6166,25 @@ 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;
+ }
+
+ ret = qemuSaveImageOpen(driver, NULL, &def, &data, true, &saveFd);
+ if (ret < 0) {
+ if (ret == -3)
ret = 1;
goto cleanup;
}
@@ -6205,15 +6228,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 91fba7bd7d..5a569fa52e 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -518,94 +518,49 @@ 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.
+ * 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 -1;
}
if (memcmp(header->magic, QEMU_SAVE_MAGIC, sizeof(header->magic)) != 0) {
- if (memcmp(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic)) == 0) {
- if (unlink_corrupt) {
- if (unlink(path) < 0) {
- virReportSystemError(errno,
- _("cannot remove corrupt file: %s"),
- path);
- return -1;
- } else {
- return -3;
- }
- }
-
+ if (memcmp(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic)) == 0)
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("save image is incomplete"));
- return -1;
+ else
+ virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("image magic is incorrect"));
+ if (unlink_corrupt) {
+ saveFd->need_unlink = true;
}
-
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("image magic is incorrect"));
return -1;
}
@@ -636,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;
@@ -645,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;
@@ -661,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.3On 5/6/22 3:11 PM, Claudio Fontana wrote:
> 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@suse.de>
> ---
> src/qemu/qemu_driver.c | 100 +++++++++++++++++++++++---------------
> src/qemu/qemu_saveimage.c | 86 ++++++++------------------------
> src/qemu/qemu_saveimage.h | 9 ++--
> 3 files changed, 82 insertions(+), 113 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ff2be6ffe9..e4cdd93b50 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5825,12 +5825,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 |
> @@ -5841,10 +5842,18 @@ 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)
> @@ -5898,16 +5907,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);
> @@ -5969,15 +5975,16 @@ 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 (virQEMUSaveFdInit(&saveFd, path, 0, O_RDONLY, cfg) < 0)
> + return NULL;
>
> - if (fd < 0)
> + if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
> goto cleanup;
>
> if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0)
> @@ -5987,7 +5994,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;
> }
>
> @@ -5999,22 +6007,23 @@ 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 |
> VIR_DOMAIN_SAVE_PAUSED, -1);
>
> + if (virQEMUSaveFdInit(&saveFd, path, 0, O_RDWR, cfg) < 0)
> + return -1;
> +
> if (flags & VIR_DOMAIN_SAVE_RUNNING)
> state = 1;
> else if (flags & VIR_DOMAIN_SAVE_PAUSED)
> state = 0;
>
> - fd = qemuSaveImageOpen(driver, NULL, path, &def, &data,
> - false, NULL, true, false);
> -
> - if (fd < 0)
> + if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0)
> goto cleanup;
>
> if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0)
> @@ -6041,15 +6050,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;
> }
> @@ -6057,8 +6066,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path,
> ret = 0;
>
> cleanup:
> - VIR_FORCE_CLOSE(fd);
> virQEMUSaveDataFree(data);
> + ret = virQEMUSaveFdFini(&saveFd, NULL, ret);
> return ret;
> }
>
> @@ -6070,8 +6079,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);
> @@ -6093,15 +6103,19 @@ 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;
> }
> @@ -6152,16 +6166,25 @@ 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;
> + }
> +
> + ret = qemuSaveImageOpen(driver, NULL, &def, &data, true, &saveFd);
> + if (ret < 0) {
> + if (ret == -3)
> ret = 1;
> goto cleanup;
> }
> @@ -6205,15 +6228,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 91fba7bd7d..5a569fa52e 100644
> --- a/src/qemu/qemu_saveimage.c
> +++ b/src/qemu/qemu_saveimage.c
> @@ -518,94 +518,49 @@ 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.
> + * 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 -1;
> }
>
> if (memcmp(header->magic, QEMU_SAVE_MAGIC, sizeof(header->magic)) != 0) {
> - if (memcmp(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic)) == 0) {
> - if (unlink_corrupt) {
> - if (unlink(path) < 0) {
> - virReportSystemError(errno,
> - _("cannot remove corrupt file: %s"),
> - path);
> - return -1;
> - } else {
> - return -3;
> - }
> - }
> -
Here the intentions of the original code are not 100% clear to me.
What do we want to do if header->magic does not match QEMU_SAVE_MAGIC?
- QEMU_SAVE_PARTIAL image : what do we want to do with that if unlink_corrupt is true, destroy it? This is what the code currently does.
- Other image (header does not match anything): what do we want to do with that if unlink_corrupt is true? Currently we return error in this case.
Is this really the original intention of the code?
Or was it rather the intention to keep QEMU_SAVE_PARTIAL images around, and delete the other?
I'll rewrite all of this in the next spin to match the original code if I don't hear anything, but I suspected that it might not be doing what was really intended.
Thanks,
Claudio
> + if (memcmp(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic)) == 0)
> virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> _("save image is incomplete"));
> - return -1;
> + else
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("image magic is incorrect"));
> + if (unlink_corrupt) {
> + saveFd->need_unlink = true;
> }
> -
> - virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> - _("image magic is incorrect"));
> return -1;
> }
>
> @@ -636,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;
> @@ -645,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;
> @@ -661,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,
>
© 2016 - 2026 Red Hat, Inc.