On 1/25/25 01:37, Jim Fehlig via Devel wrote:
> Split the reading of libvirt's save image metadata from the opening
> of the fd that will be passed to QEMU. This allows improved error
> handling and provides more flexibility for code reading saved images.
>
> 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_saveimage.c b/src/qemu/qemu_saveimage.c
> index 69617e07eb..76d9e96792 100644
> --- a/src/qemu/qemu_saveimage.c
> +++ b/src/qemu/qemu_saveimage.c
> @@ -522,58 +522,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);
> @@ -671,6 +648,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;
I don't think it's this easy. Previously, the libvirt header was read
and @fd was left at the beginning of QEMU migration stream. Now @fd
points at the beginning of the file (our header).
I wonder whether we can somehow pass @fd from qemuSaveImageGetMetadata()
into here (and possibly reopen it for RW should @open_write be set).
> +
> + if (bypass_cache &&
> + !(*wrapperFd = virFileWrapperFdNew(&fd, path,
> + VIR_FILE_WRAPPER_BYPASS_CACHE)))
> + return -1;
> +
> ret = fd;
> fd = -1;
Michal
