On 5/14/22 5:52 PM, Claudio Fontana wrote:
> 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@xxxxxxx>
> ---
> 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
Introducing this incompatibility is not necessary, I'll respin momentarily a version that should allow
old images to work with libvirt after this commit,
and new images to also work in older libvirt.
C
