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 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;
--
2.43.0
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@lists.libvirt.org/thread/DATZFZY6ETRYOQ6ORQ2JIVHBTFGKUBJM/ Regards, Jim > > 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 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; >
© 2016 - 2025 Red Hat, Inc.