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@suse.de>
---
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
G_STATIC_ASSERT(sizeof(QEMU_SAVE_MAGIC) == sizeof(QEMU_SAVE_PARTIAL));
typedef struct _virQEMUSaveHeader virQEMUSaveHeader;
struct _virQEMUSaveHeader {
- char magic[sizeof(QEMU_SAVE_MAGIC)-1];
- uint32_t version;
- uint32_t data_len;
- uint32_t was_running;
- uint32_t compressed;
- uint32_t cookieOffset;
- uint32_t unused[14];
-};
+ char magic[sizeof(QEMU_SAVE_MAGIC)-1]; /* 16 bytes */
+ uint32_t version; /* 4 bytes */
+ uint32_t data_len; /* 4 bytes */
+ uint32_t was_running; /* 4 bytes */
+ uint32_t compressed; /* 4 bytes */
+ uint32_t cookieOffset; /* 4 bytes */
+ uint32_t unused[14]; /* 56 bytes */
+} ATTRIBUTE_PACKED; /* = 92 bytes */
typedef struct _virQEMUSaveData virQEMUSaveData;
@@ -103,6 +103,10 @@ int
virQEMUSaveDataWrite(virQEMUSaveData *data,
int fd,
const char *path);
+int
+virQEMUSaveDataRead(virQEMUSaveData *data,
+ int fd,
+ const char *path);
virQEMUSaveData *
virQEMUSaveDataNew(char *domXML,
--
2.35.3
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@suse.de>
> ---
> 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
© 2016 - 2026 Red Hat, Inc.