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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux