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

6 Jun 2017

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

    

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

Pavel Hrdina