From 996dfd6322782e72583384381ffebf91ef1d3e27 Mon Sep 17 00:00:00 2001

Message-Id: <996dfd6322782e72583384381ffebf91ef1d3e27@dist-git>

From: Peter Krempa <pkrempa@redhat.com>

Date: Thu, 14 Dec 2017 14:31:51 +0100

Subject: [PATCH] conf: Add infrastructure for disk source private data XML

VM drivers may need to store additional private data to the status XML

so that it can be restored after libvirtd restart. Since not everything

is needed add a callback infrastructure, where VM drivers can add only

stuff they need.

Note that the private data is formatted as a <privateData> sub-element

of the <disk> or <backingStore> <source> sub-element. This is done since

storing it out of band (in the VM private data) would require a complex

matching process to allow to put the data into correct place.

(cherry picked from commit aed3d038a6565f3b0e916bdb004ca90b302b200d)

 Conflicts:

	tests/qemublocktest.c: This file does not exist downstream yet.

https://bugzilla.redhat.com/show_bug.cgi?id=1523261

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/conf/domain_conf.c      | 136 ++++++++++++++++++++++++++++++++++----------

 src/conf/domain_conf.h      |  17 +++++-

 src/conf/snapshot_conf.c    |  18 +++---

 src/network/bridge_driver.c |   2 +-

 src/qemu/qemu_domain.c      |   2 +-

 tests/virstoragetest.c      |   2 +-

 6 files changed, 134 insertions(+), 43 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index 2d54037b71..ad9c16a1e0 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -8550,11 +8550,43 @@ virDomainDiskSourceEncryptionParse(xmlNodePtr node,

 }

+static int

+virDomainDiskSourcePrivateDataParse(xmlXPathContextPtr ctxt,

+                                    virStorageSourcePtr src,

+                                    unsigned int flags,

+                                    virDomainXMLOptionPtr xmlopt)

+{

+    xmlNodePtr saveNode = ctxt->node;

+    xmlNodePtr node;

+    int ret = -1;

+

+    if (!(flags & VIR_DOMAIN_DEF_PARSE_STATUS) ||

+        !xmlopt || !xmlopt->privateData.storageParse)

+        return 0;

+

+    if (!(node = virXPathNode("./privateData", ctxt)))

+        return 0;

+

+    ctxt->node = node;

+

+    if (xmlopt->privateData.storageParse(ctxt, src) < 0)

+        goto cleanup;

+

+    ret = 0;

+

+ cleanup:

+    ctxt->node = saveNode;

+

+    return ret;

+}

+

+

 int

 virDomainDiskSourceParse(xmlNodePtr node,

                          xmlXPathContextPtr ctxt,

                          virStorageSourcePtr src,

-                         unsigned int flags)

+                         unsigned int flags,

+                         virDomainXMLOptionPtr xmlopt)

 {

     int ret = -1;

     xmlNodePtr saveNode = ctxt->node;

@@ -8593,6 +8625,9 @@ virDomainDiskSourceParse(xmlNodePtr node,

     if (virDomainDiskSourceEncryptionParse(node, &src->encryption) < 0)

         goto cleanup;

+    if (virDomainDiskSourcePrivateDataParse(ctxt, src, flags, xmlopt) < 0)

+        goto cleanup;

+

     /* People sometimes pass a bogus '' source path when they mean to omit the

      * source element completely (e.g. CDROM without media). This is just a

      * little compatibility check to help those broken apps */

@@ -8610,7 +8645,8 @@ virDomainDiskSourceParse(xmlNodePtr node,

 static int

 virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,

                                virStorageSourcePtr src,

-                               unsigned int flags)

+                               unsigned int flags,

+                               virDomainXMLOptionPtr xmlopt)

 {

     virStorageSourcePtr backingStore = NULL;

     xmlNodePtr save_ctxt = ctxt->node;

@@ -8668,8 +8704,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,

         goto cleanup;

     }

-    if (virDomainDiskSourceParse(source, ctxt, backingStore, flags) < 0 ||

-        virDomainDiskBackingStoreParse(ctxt, backingStore, flags) < 0)

+    if (virDomainDiskSourceParse(source, ctxt, backingStore, flags, xmlopt) < 0 ||

+        virDomainDiskBackingStoreParse(ctxt, backingStore, flags, xmlopt) < 0)

         goto cleanup;

     VIR_STEAL_PTR(src->backingStore, backingStore);

@@ -8771,7 +8807,8 @@ static int

 virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,

                             xmlNodePtr cur,

                             xmlXPathContextPtr ctxt,

-                            unsigned int flags)

+                            unsigned int flags,

+                            virDomainXMLOptionPtr xmlopt)

 {

     xmlNodePtr mirrorNode;

     char *mirrorFormat = NULL;

@@ -8809,7 +8846,8 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def,

             goto cleanup;

         }

-        if (virDomainDiskSourceParse(mirrorNode, ctxt, def->mirror, flags) < 0)

+        if (virDomainDiskSourceParse(mirrorNode, ctxt, def->mirror,

+                                     flags, xmlopt) < 0)

             goto cleanup;

     } else {

         /* For back-compat reasons, we handle a file name

@@ -9235,7 +9273,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,

         if (!source && virXMLNodeNameEqual(cur, "source")) {

             sourceNode = cur;

-            if (virDomainDiskSourceParse(cur, ctxt, def->src, flags) < 0)

+            if (virDomainDiskSourceParse(cur, ctxt, def->src, flags, xmlopt) < 0)

                 goto error;

             /* If we've already found an <auth> as a child of <disk> and

@@ -9316,7 +9354,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,

         } else if (!def->mirror &&

                    virXMLNodeNameEqual(cur, "mirror") &&

                    !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {

-            if (virDomainDiskDefMirrorParse(def, cur, ctxt, flags) < 0)

+            if (virDomainDiskDefMirrorParse(def, cur, ctxt, flags, xmlopt) < 0)

                 goto error;

         } else if (!authdef &&

                    virXMLNodeNameEqual(cur, "auth")) {

@@ -9587,7 +9625,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,

     product = NULL;

     if (!(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) {

-        if (virDomainDiskBackingStoreParse(ctxt, def->src, flags) < 0)

+        if (virDomainDiskBackingStoreParse(ctxt, def->src, flags, xmlopt) < 0)

             goto error;

     }

@@ -22349,12 +22387,43 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,

 }

+static int

+virDomainDiskSourceFormatPrivateData(virBufferPtr buf,

+                                     virStorageSourcePtr src,

+                                     unsigned int flags,

+                                     virDomainXMLOptionPtr xmlopt)

+{

+    virBuffer childBuf = VIR_BUFFER_INITIALIZER;

+    int ret = -1;

+

+    if (!(flags & VIR_DOMAIN_DEF_FORMAT_STATUS) ||

+        !xmlopt || !xmlopt->privateData.storageFormat)

+        return 0;

+

+    virBufferSetChildIndent(&childBuf, buf);

+

+    if (xmlopt->privateData.storageFormat(src, &childBuf) < 0)

+        goto cleanup;

+

+    if (virXMLFormatElement(buf, "privateData", NULL, &childBuf) < 0)

+        goto cleanup;

+

+    ret = 0;

+

+ cleanup:

+    virBufferFreeAndReset(&childBuf);

+

+    return ret;

+}

+

+

 static int

 virDomainDiskSourceFormatInternal(virBufferPtr buf,

                                   virStorageSourcePtr src,

                                   int policy,

                                   unsigned int flags,

-                                  bool skipSeclabels)

+                                  bool skipSeclabels,

+                                  virDomainXMLOptionPtr xmlopt)

 {

     const char *startupPolicy = NULL;

     virBuffer attrBuf = VIR_BUFFER_INITIALIZER;

@@ -22436,6 +22505,9 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,

             virStorageEncryptionFormat(&childBuf, src->encryption) < 0)

             return -1;

+        if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0)

+            return -1;

+

         if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0)

             goto error;

     }

@@ -22453,15 +22525,18 @@ int

 virDomainDiskSourceFormat(virBufferPtr buf,

                           virStorageSourcePtr src,

                           int policy,

-                          unsigned int flags)

+                          unsigned int flags,

+                          virDomainXMLOptionPtr xmlopt)

 {

-    return virDomainDiskSourceFormatInternal(buf, src, policy, flags, false);

+    return virDomainDiskSourceFormatInternal(buf, src, policy, flags, false, xmlopt);

 }

 static int

 virDomainDiskBackingStoreFormat(virBufferPtr buf,

-                                virStorageSourcePtr backingStore)

+                                virStorageSourcePtr backingStore,

+                                virDomainXMLOptionPtr xmlopt,

+                                unsigned int flags)

 {

     const char *format;

@@ -22490,9 +22565,9 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,

     virBufferAsprintf(buf, "<format type='%s'/>\n", format);

     /* We currently don't output seclabels for backing chain element */

-    if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, 0, true) < 0 ||

-        virDomainDiskBackingStoreFormat(buf,

-                                        backingStore->backingStore) < 0)

+    if (virDomainDiskSourceFormatInternal(buf, backingStore, 0, flags, true, xmlopt) < 0 ||

+        virDomainDiskBackingStoreFormat(buf, backingStore->backingStore,

+                                        xmlopt, flags) < 0)

         return -1;

     virBufferAdjustIndent(buf, -2);

@@ -22510,7 +22585,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,

 static int

 virDomainDiskDefFormat(virBufferPtr buf,

                        virDomainDiskDefPtr def,

-                       unsigned int flags)

+                       unsigned int flags,

+                       virDomainXMLOptionPtr xmlopt)

 {

     const char *type = virStorageTypeToString(def->src->type);

     const char *device = virDomainDiskDeviceTypeToString(def->device);

@@ -22623,13 +22699,14 @@ virDomainDiskDefFormat(virBufferPtr buf,

     }

     if (virDomainDiskSourceFormat(buf, def->src, def->startupPolicy,

-                                  flags) < 0)

+                                  flags, xmlopt) < 0)

         return -1;

     /* Don't format backingStore to inactive XMLs until the code for

      * persistent storage of backing chains is ready. */

     if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&

-        virDomainDiskBackingStoreFormat(buf, def->src->backingStore) < 0)

+        virDomainDiskBackingStoreFormat(buf, def->src->backingStore,

+                                        xmlopt, flags) < 0)

         return -1;

     virBufferEscapeString(buf, "<backenddomain name='%s'/>\n", def->domain_name);

@@ -22666,7 +22743,7 @@ virDomainDiskDefFormat(virBufferPtr buf,

         virBufferAddLit(buf, ">\n");

         virBufferAdjustIndent(buf, 2);

         virBufferEscapeString(buf, "<format type='%s'/>\n", formatStr);

-        if (virDomainDiskSourceFormat(buf, def->mirror, 0, 0) < 0)

+        if (virDomainDiskSourceFormat(buf, def->mirror, 0, 0, xmlopt) < 0)

             return -1;

         virBufferAdjustIndent(buf, -2);

         virBufferAddLit(buf, "</mirror>\n");

@@ -25897,7 +25974,8 @@ int

 virDomainDefFormatInternal(virDomainDefPtr def,

                            virCapsPtr caps,

                            unsigned int flags,

-                           virBufferPtr buf)

+                           virBufferPtr buf,

+                           virDomainXMLOptionPtr xmlopt)

 {

     unsigned char *uuid;

     char uuidstr[VIR_UUID_STRING_BUFLEN];

@@ -25952,10 +26030,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,

          * but no leading indentation before the starting element.

          * Thankfully, libxml maps what looks like globals into

          * thread-local uses, so we are thread-safe.  */

-        xmlIndentTreeOutput = 1;

-        xmlbuf = xmlBufferCreate();

-        if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata,

-                        virBufferGetIndent(buf, false) / 2, 1) < 0) {

+            xmlIndentTreeOutput = 1;

+            xmlbuf = xmlBufferCreate();

+            if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata,

+                            virBufferGetIndent(buf, false) / 2, 1) < 0) {

             xmlBufferFree(xmlbuf);

             xmlIndentTreeOutput = oldIndentTreeOutput;

             goto error;

@@ -26528,7 +26606,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,

                           def->emulator);

     for (n = 0; n < def->ndisks; n++)

-        if (virDomainDiskDefFormat(buf, def->disks[n], flags) < 0)

+        if (virDomainDiskDefFormat(buf, def->disks[n], flags, xmlopt) < 0)

             goto error;

     for (n = 0; n < def->ncontrollers; n++)

@@ -26715,7 +26793,7 @@ virDomainDefFormat(virDomainDefPtr def, virCapsPtr caps, unsigned int flags)

     virBuffer buf = VIR_BUFFER_INITIALIZER;

     virCheckFlags(VIR_DOMAIN_DEF_FORMAT_COMMON_FLAGS, NULL);

-    if (virDomainDefFormatInternal(def, caps, flags, &buf) < 0)

+    if (virDomainDefFormatInternal(def, caps, flags, &buf, NULL) < 0)

         return NULL;

     return virBufferContentAndReset(&buf;;

@@ -26750,7 +26828,7 @@ virDomainObjFormat(virDomainXMLOptionPtr xmlopt,

         xmlopt->privateData.format(&buf, obj) < 0)

         goto error;

-    if (virDomainDefFormatInternal(obj->def, caps, flags, &buf) < 0)

+    if (virDomainDefFormatInternal(obj->def, caps, flags, &buf, xmlopt) < 0)

         goto error;

     virBufferAdjustIndent(&buf, -2);

@@ -27704,7 +27782,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src,

     switch ((virDomainDeviceType) src->type) {

     case VIR_DOMAIN_DEVICE_DISK:

-        rc = virDomainDiskDefFormat(&buf, src->data.disk, flags);

+        rc = virDomainDiskDefFormat(&buf, src->data.disk, flags, xmlopt);

         break;

     case VIR_DOMAIN_DEVICE_LEASE:

         rc = virDomainLeaseDefFormat(&buf, src->data.lease);

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h

index 698d3280fe..5e67c9a9c1 100644

--- a/src/conf/domain_conf.h

+++ b/src/conf/domain_conf.h

@@ -2626,6 +2626,12 @@ typedef int (*virDomainXMLPrivateDataParseFunc)(xmlXPathContextPtr,

                                                 virDomainObjPtr,

                                                 virDomainDefParserConfigPtr);

+typedef int (*virDomainXMLPrivateDataStorageSourceParseFunc)(xmlXPathContextPtr ctxt,

+                                                             virStorageSourcePtr src);

+typedef int (*virDomainXMLPrivateDataStorageSourceFormatFunc)(virStorageSourcePtr src,

+                                                              virBufferPtr buf);

+

+

 typedef struct _virDomainXMLPrivateDataCallbacks virDomainXMLPrivateDataCallbacks;

 typedef virDomainXMLPrivateDataCallbacks *virDomainXMLPrivateDataCallbacksPtr;

 struct _virDomainXMLPrivateDataCallbacks {

@@ -2639,6 +2645,8 @@ struct _virDomainXMLPrivateDataCallbacks {

     virDomainXMLPrivateDataNewFunc    chrSourceNew;

     virDomainXMLPrivateDataFormatFunc format;

     virDomainXMLPrivateDataParseFunc  parse;

+    virDomainXMLPrivateDataStorageSourceParseFunc storageParse;

+    virDomainXMLPrivateDataStorageSourceFormatFunc storageFormat;

 };

 typedef bool (*virDomainABIStabilityDomain)(const virDomainDef *src,

@@ -2963,12 +2971,14 @@ char *virDomainObjFormat(virDomainXMLOptionPtr xmlopt,

 int virDomainDefFormatInternal(virDomainDefPtr def,

                                virCapsPtr caps,

                                unsigned int flags,

-                               virBufferPtr buf);

+                               virBufferPtr buf,

+                               virDomainXMLOptionPtr xmlopt);

 int virDomainDiskSourceFormat(virBufferPtr buf,

                               virStorageSourcePtr src,

                               int policy,

-                              unsigned int flags);

+                              unsigned int flags,

+                              virDomainXMLOptionPtr xmlopt);

 int virDomainNetDefFormat(virBufferPtr buf,

                           virDomainNetDefPtr def,

@@ -3017,7 +3027,8 @@ virDomainDiskRemoveByName(virDomainDefPtr def, const char *name);

 int virDomainDiskSourceParse(xmlNodePtr node,

                              xmlXPathContextPtr ctxt,

                              virStorageSourcePtr src,

-                             unsigned int flags);

+                             unsigned int flags,

+                             virDomainXMLOptionPtr xmlopt);

 int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net);

 virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device);

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c

index f0e852c92b..d7b086242b 100644

--- a/src/conf/snapshot_conf.c

+++ b/src/conf/snapshot_conf.c

@@ -110,7 +110,8 @@ static int

 virDomainSnapshotDiskDefParseXML(xmlNodePtr node,

                                  xmlXPathContextPtr ctxt,

                                  virDomainSnapshotDiskDefPtr def,

-                                 unsigned int flags)

+                                 unsigned int flags,

+                                 virDomainXMLOptionPtr xmlopt)

 {

     int ret = -1;

     char *snapshot = NULL;

@@ -155,7 +156,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,

     }

     if ((cur = virXPathNode("./source", ctxt)) &&

-        virDomainDiskSourceParse(cur, ctxt, def->src, flags) < 0)

+        virDomainDiskSourceParse(cur, ctxt, def->src, flags, xmlopt) < 0)

         goto cleanup;

     if ((driver = virXPathString("string(./driver/@type)", ctxt))) {

@@ -348,8 +349,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,

             goto cleanup;

         def->ndisks = n;

         for (i = 0; i < def->ndisks; i++) {

-            if (virDomainSnapshotDiskDefParseXML(nodes[i], ctxt,

-                                                 &def->disks[i], flags) < 0)

+            if (virDomainSnapshotDiskDefParseXML(nodes[i], ctxt, &def->disks[i],

+                                                 flags, xmlopt) < 0)

                 goto cleanup;

         }

         VIR_FREE(nodes);

@@ -663,7 +664,8 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def,

 static void

 virDomainSnapshotDiskDefFormat(virBufferPtr buf,

-                               virDomainSnapshotDiskDefPtr disk)

+                               virDomainSnapshotDiskDefPtr disk,

+                               virDomainXMLOptionPtr xmlopt)

 {

     int type = disk->src->type;

@@ -686,7 +688,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,

     if (disk->src->format > 0)

         virBufferEscapeString(buf, "<driver type='%s'/>\n",

                               virStorageFileFormatTypeToString(disk->src->format));

-    virDomainDiskSourceFormat(buf, disk->src, 0, 0);

+    virDomainDiskSourceFormat(buf, disk->src, 0, 0, xmlopt);

     virBufferAdjustIndent(buf, -2);

     virBufferAddLit(buf, "</disk>\n");

@@ -740,13 +742,13 @@ virDomainSnapshotDefFormat(const char *domain_uuid,

         virBufferAddLit(&buf, "<disks>\n");

         virBufferAdjustIndent(&buf, 2);

         for (i = 0; i < def->ndisks; i++)

-            virDomainSnapshotDiskDefFormat(&buf, &def->disks[i]);

+            virDomainSnapshotDiskDefFormat(&buf, &def->disks[i], xmlopt);

         virBufferAdjustIndent(&buf, -2);

         virBufferAddLit(&buf, "</disks>\n");

     }

     if (def->dom) {

-        if (virDomainDefFormatInternal(def->dom, caps, flags, &buf) < 0)

+        if (virDomainDefFormatInternal(def->dom, caps, flags, &buf, xmlopt) < 0)

             goto error;

     } else if (domain_uuid) {

         virBufferAddLit(&buf, "<domain>\n");

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c

index fcaa66df91..334da7a85d 100644

--- a/src/network/bridge_driver.c

+++ b/src/network/bridge_driver.c

@@ -233,7 +233,7 @@ networkRunHook(virNetworkObjPtr obj,

             goto cleanup;

         if (virNetworkDefFormatBuf(&buf, def, 0) < 0)

             goto cleanup;

-        if (dom && virDomainDefFormatInternal(dom, NULL, 0, &buf) < 0)

+        if (dom && virDomainDefFormatInternal(dom, NULL, 0, &buf, NULL) < 0)

             goto cleanup;

         virBufferAdjustIndent(&buf, -2);

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c

index 9dd9db049b..625bcb20d5 100644

--- a/src/qemu/qemu_domain.c

+++ b/src/qemu/qemu_domain.c

@@ -5245,7 +5245,7 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,

  format:

     ret = virDomainDefFormatInternal(def, caps,

                                      virDomainDefFormatConvertXMLFlags(flags),

-                                     buf);

+                                     buf, driver->xmlopt);

  cleanup:

     virDomainDefFree(copy);

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c

index e1d8751728..bdd0829c21 100644

--- a/tests/virstoragetest.c

+++ b/tests/virstoragetest.c

@@ -693,7 +693,7 @@ testBackingParse(const void *args)

         goto cleanup;

     }

-    if (virDomainDiskSourceFormat(&buf, src, 0, 0) < 0 ||

+    if (virDomainDiskSourceFormat(&buf, src, 0, 0, NULL) < 0 ||

         !(xml = virBufferContentAndReset(&buf))) {

         fprintf(stderr, "failed to format disk source xml\n");

         goto cleanup;

-- 

2.15.1