Re: [libvirt RFCv7 22/40] qemu: saveimage: convert qemuSaveImageOpen to use virQEMUSaveFd

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(a)suse.de&gt; --- 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,