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

Re: [libvirt] [PATCH 10/15] qemu: Store save cookie in save images and snapshots



On Mon, Jun 05, 2017 at 11:26:58AM +0200, Jiri Denemark wrote:
> The following patches will add an actual content in the cookie and use
> the data when restoring a domain.
> 
> Signed-off-by: Jiri Denemark <jdenemar redhat com>
> ---
>  docs/formatsnapshot.html.in     |  6 +++
>  docs/schemas/domainsnapshot.rng |  7 ++++
>  src/qemu/qemu_driver.c          | 90 ++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 93 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in
> index c3ab516fa..5e8e21c8a 100644
> --- a/docs/formatsnapshot.html.in
> +++ b/docs/formatsnapshot.html.in
> @@ -235,6 +235,12 @@
>          at the time of the snapshot (<span class="since">since
>          0.9.5</span>).  Readonly.
>        </dd>
> +      <dt><code>cookie</code></dt>
> +      <dd>Save image cookie containing additional data libvirt may need to
> +        properly restore a domain from an active snapshot when such data
> +        cannot be stored directly in the <code>domain</code> to maintain
> +        compatibility with older libvirt or hypervisor. Readonly.
> +      </dd>
>      </dl>
>  
>      <h2><a name="example">Examples</a></h2>
> diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng
> index 4ab1b828f..268088709 100644
> --- a/docs/schemas/domainsnapshot.rng
> +++ b/docs/schemas/domainsnapshot.rng
> @@ -90,6 +90,13 @@
>              </element>
>            </element>
>          </optional>
> +        <optional>
> +          <element name='cookie'>
> +            <zeroOrMore>
> +              <ref name='customElement'/>
> +            </zeroOrMore>
> +          </element>
> +        </optional>
>        </interleave>
>      </element>
>    </define>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 96077df78..10df56e61 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2808,7 +2808,8 @@ struct _virQEMUSaveHeader {
>      uint32_t data_len;
>      uint32_t was_running;
>      uint32_t compressed;
> -    uint32_t unused[15];
> +    uint32_t cookie;

I would suggest cookieOffset.

> +    uint32_t unused[14];
>  };
>  
>  typedef struct _virQEMUSaveData virQEMUSaveData;
> @@ -2816,6 +2817,7 @@ typedef virQEMUSaveData *virQEMUSaveDataPtr;
>  struct _virQEMUSaveData {
>      virQEMUSaveHeader header;
>      char *xml;
> +    char *cookie;
>  };
>  
>  
> @@ -2826,6 +2828,7 @@ bswap_header(virQEMUSaveHeaderPtr hdr)
>      hdr->data_len = bswap_32(hdr->data_len);
>      hdr->was_running = bswap_32(hdr->was_running);
>      hdr->compressed = bswap_32(hdr->compressed);
> +    hdr->cookie = bswap_32(hdr->cookie);
>  }
>  
>  
> @@ -2836,6 +2839,7 @@ virQEMUSaveDataFree(virQEMUSaveDataPtr data)
>          return;
>  
>      VIR_FREE(data->xml);
> +    VIR_FREE(data->cookie);
>      VIR_FREE(data);
>  }
>  
> @@ -2845,8 +2849,10 @@ virQEMUSaveDataFree(virQEMUSaveDataPtr data)
>   */
>  static virQEMUSaveDataPtr
>  virQEMUSaveDataNew(char *domXML,
> +                   qemuDomainSaveCookiePtr cookie,

I would suggest cookieObj.

>                     bool running,
> -                   int compressed)
> +                   int compressed,
> +                   virDomainXMLOptionPtr xmlopt)
>  {
>      virQEMUSaveDataPtr data = NULL;
>      virQEMUSaveHeaderPtr header;
> @@ -2856,6 +2862,11 @@ virQEMUSaveDataNew(char *domXML,
>  
>      VIR_STEAL_PTR(data->xml, domXML);
>  
> +    if (cookie &&
> +        !(data->cookie = virSaveCookieFormat((virObjectPtr) cookie,
> +                                             virDomainXMLOptionGetSaveCookie(xmlopt))))

Here it's kind of confusing,

> +        goto error;
> +
>      header = &data->header;
>      memcpy(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic));
>      header->version = QEMU_SAVE_VERSION;
> @@ -2863,6 +2874,10 @@ virQEMUSaveDataNew(char *domXML,
>      header->compressed = compressed;
>  
>      return data;
> +
> + error:
> +    virQEMUSaveDataFree(data);
> +    return NULL;
>  }
>  
>  
> @@ -2882,9 +2897,15 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data,
>  {
>      virQEMUSaveHeaderPtr header = &data->header;
>      size_t len;
> +    size_t xml_len;
> +    size_t cookie_len = 0;
>      int ret = -1;
>  
> -    len = strlen(data->xml) + 1;
> +    xml_len = strlen(data->xml) + 1;
> +    if (data->cookie)
> +        cookie_len = strlen(data->cookie) + 1;
> +
> +    len = xml_len + cookie_len;
>  
>      if (header->data_len > 0) {
>          if (len > header->data_len) {
> @@ -2893,12 +2914,15 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data,
>              goto cleanup;
>          }
>  
> -        if (VIR_EXPAND_N(data->xml, len, header->data_len - len) < 0)
> +        if (VIR_EXPAND_N(data->xml, xml_len, header->data_len - len) < 0)

I think that we should drop the expanding and just write 0 to the
remaining data part of save image ... [1]

>              goto cleanup;
>      } else {
>          header->data_len = len;
>      }
>  
> +    if (data->cookie)
> +        header->cookie = xml_len;

and this is also kind of confusing.

> +
>      if (safewrite(fd, header, sizeof(*header)) != sizeof(*header)) {
>          virReportSystemError(errno,
>                               _("failed to write header to domain save file '%s'"),
> @@ -2906,13 +2930,21 @@ virQEMUSaveDataWrite(virQEMUSaveDataPtr data,
>          goto cleanup;
>      }
>  
> -    if (safewrite(fd, data->xml, header->data_len) != header->data_len) {
> +    if (safewrite(fd, data->xml, xml_len) != xml_len) {
>          virReportSystemError(errno,
>                               _("failed to write domain xml to '%s'"),
>                               path);
>          goto cleanup;

[1] ... there you write the expanded XML to the save image which will
stop at the end of the our internal data part ... [2]

>      }
>  
> +    if (data->cookie &&
> +        safewrite(fd, data->cookie, cookie_len) != cookie_len) {
> +        virReportSystemError(errno,
> +                             _("failed to write cookie to '%s'"),
> +                             path);
> +        goto cleanup;
> +    }
> +

[2] ... and here you continue writing the cookie data out of the
internal data part.

Pavel

Attachment: signature.asc
Description: Digital signature


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