Re: [libvirt RFCv9 03/31] qemu: saveimage: rework save format and read/write to be O_DIRECT friendly

change the saveimage format to: 1) ensure that the header struct fields are packed, so we can be sure no padding will ruin the day 2) finish the libvirt header (header + xml + cookie) with zero padding, in order to ensure that the QEMU VM (QEVM Magic) is aligned. Adapt the read and write of the libvirt header accordingly. Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/qemu/qemu_saveimage.c | 229 ++++++++++++++++++++++---------------- src/qemu/qemu_saveimage.h | 22 ++-- 2 files changed, 143 insertions(+), 108 deletions(-) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 4fd4c5cfcd..7db54f11e1 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -139,12 +139,12 @@ virQEMUSaveDataWrite(virQEMUSaveData *data, int fd, const char *path) { + g_autofree void *base = NULL; + char *buf, *cur; virQEMUSaveHeader *header = &data->header; size_t len; size_t xml_len; size_t cookie_len = 0; - size_t zerosLen = 0; - g_autofree char *zeros = NULL; xml_len = strlen(data->xml) + 1; if (data->cookie) @@ -165,42 +165,148 @@ virQEMUSaveDataWrite(virQEMUSaveData *data, return -1; } } - - zerosLen = header->data_len - len; - zeros = g_new0(char, zerosLen); + buf = virFileDirectBufferNew(&base, sizeof(*header) + header->data_len); + cur = buf; if (data->cookie) header->cookieOffset = xml_len; - if (safewrite(fd, header, sizeof(*header)) != sizeof(*header)) { - virReportSystemError(errno, - _("failed to write header to domain save file '%s'"), - path); - return -1; + memcpy(cur, header, sizeof(*header)); + cur += sizeof(*header); + memcpy(cur, data->xml, xml_len); + cur += xml_len; + if (data->cookie) { + memcpy(cur, data->cookie, cookie_len); + cur += cookie_len; } - if (safewrite(fd, data->xml, xml_len) != xml_len) { + if (virFileDirectWrite(fd, buf, sizeof(*header) + header->data_len) < 0) { virReportSystemError(errno, - _("failed to write domain xml to '%s'"), + _("failed to write libvirt header of domain save file '%s'"), path); return -1; } - if (data->cookie && - safewrite(fd, data->cookie, cookie_len) != cookie_len) { + return 0; +} + +/* virQEMUSaveDataRead: + * + * Reads libvirt's header (including domain XML) from a saved image. + * + * Returns -1 on generic failure, -3 on a corrupted image, or 0 on success. + */ +int +virQEMUSaveDataRead(virQEMUSaveData *data, + int fd, + const char *path) +{ + g_autofree void *base = NULL; + virQEMUSaveHeader *header = &data->header; + size_t xml_len; + size_t cookie_len; + ssize_t rv; + size_t buflen = 1024 * 1024; + void *dst; + char *buf = virFileDirectBufferNew(&base, buflen); + void *src = buf; + + header = &data->header; + rv = virFileDirectReadCopy(fd, &src, buflen, header, sizeof(*header)); + if (rv < 0) { virReportSystemError(errno, - _("failed to write cookie to '%s'"), + _("failed to read libvirt header of domain save file '%s'"), path); return -1; } + if (rv < sizeof(*header)) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("domain save file '%s' libvirt header appears truncated"), + path); + return -3; + } + rv -= sizeof(*header); - if (safewrite(fd, zeros, zerosLen) != zerosLen) { - virReportSystemError(errno, - _("failed to write padding to '%s'"), - path); + 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, + _("domain save file '%s' seems incomplete"), + path); + return -3; + } + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("image magic is incorrect")); + return -1; + } + if (header->version > QEMU_SAVE_VERSION) { + /* convert endianness and try again */ + qemuSaveImageBswapHeader(header); + } + if (header->version > QEMU_SAVE_VERSION) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("image version is not supported (%d > %d)"), + header->version, QEMU_SAVE_VERSION); return -1; } + if (header->cookieOffset) + xml_len = header->cookieOffset; + else + xml_len = header->data_len; + if (xml_len <= 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("invalid xml length: %lu"), xml_len); + return -1; + } + if (header->data_len < xml_len) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("invalid cookieOffset: %u"), header->cookieOffset); + return -1; + } + cookie_len = header->data_len - xml_len; + data->xml = g_new0(char, xml_len); + dst = data->xml; + if (rv > 0) { + rv -= virFileDirectCopyBuf(&src, rv, &dst, &xml_len); + } + if (xml_len > 0) { + rv = virFileDirectReadCopy(fd, &src, buflen, dst, xml_len); + if (rv < 0) { + virReportSystemError(errno, + _("failed to read libvirt xml in domain save file '%s'"), + path); + return -1; + } + if (rv < xml_len) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("domain save file '%s' xml seems incomplete"), + path); + return -3; + } + } + if (cookie_len > 0) { + data->cookie = g_new0(char, cookie_len); + dst = data->cookie; + if (rv > 0) { + rv -= virFileDirectCopyBuf(&src, rv, &dst, &cookie_len); + } + if (cookie_len > 0) { + rv = virFileDirectReadCopy(fd, &src, buflen, dst, cookie_len); + if (rv < 0) { + virReportSystemError(errno, + _("failed to read libvirt cookie in domain save file '%s'"), + path); + return -1; + } + if (rv < cookie_len) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("domain save file '%s' cookie seems incomplete"), + path); + return -3; + } + } + } + /* we should now be aligned and ready to read the QEVM */ return 0; } @@ -444,11 +550,8 @@ qemuSaveImageOpen(virQEMUDriver *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(); @@ -469,89 +572,17 @@ qemuSaveImageOpen(virQEMUDriver *driver, return -1; data = g_new0(virQEMUSaveData, 1); - - header = &data->header; - if (saferead(fd, header, sizeof(*header)) != sizeof(*header)) { - if (unlink_corrupt) { + ret = virQEMUSaveDataRead(data, fd, path); + if (ret < 0) { + if (unlink_corrupt && ret == -3) { if (unlink(path) < 0) { virReportSystemError(errno, _("cannot remove corrupt file: %s"), path); return -1; - } else { - return -3; - } - } - - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to read qemu header")); - 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; - } } - - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("save image is incomplete")); - return -1; - } - - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("image magic is incorrect")); - return -1; - } - - if (header->version > QEMU_SAVE_VERSION) { - /* convert endianness and try again */ - qemuSaveImageBswapHeader(header); - } - - if (header->version > QEMU_SAVE_VERSION) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("image version is not supported (%d > %d)"), - header->version, QEMU_SAVE_VERSION); - return -1; - } - - if (header->data_len <= 0) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("invalid header data length: %d"), header->data_len); - return -1; - } - - if (header->cookieOffset) - xml_len = header->cookieOffset; - else - xml_len = header->data_len; - - cookie_len = header->data_len - xml_len; - - data->xml = g_new0(char, xml_len); - - if (saferead(fd, data->xml, xml_len) != xml_len) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("failed to read domain XML")); - return -1; - } - - if (cookie_len > 0) { - data->cookie = g_new0(char, cookie_len); - - if (saferead(fd, data->cookie, cookie_len) != cookie_len) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("failed to read cookie")); - return -1; } + return ret; } /* Create a domain from this XML */ @@ -601,7 +632,7 @@ qemuSaveImageStartVM(virConnectPtr conn, virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0) goto cleanup; - if ((header->version == 2) && + if ((header->version >= 2) && (header->compressed != QEMU_SAVE_FORMAT_RAW)) { if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed))) goto cleanup; diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 391cd55ed0..58d0949b9c 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -30,20 +30,20 @@ */ #define QEMU_SAVE_MAGIC "LibvirtQemudSave" #define QEMU_SAVE_PARTIAL "LibvirtQemudPart" -#define QEMU_SAVE_VERSION 2 +#define QEMU_SAVE_VERSION 3