[PATCH v3 2/5] Add support to parse/format/validate virStorageSource type NVRAM

Rohit Kumar rohit.kumar3 at nutanix.com
Wed May 4 16:51:12 UTC 2022


This patch introduces the logic to format and parse remote NVRAM.
This also adds 'type' attribute to nvram element and validates
'file' type NVRAM for unsupported configurations.

Sample XML with new annotation:

<nvram type='network'>
  <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/0'>
    <host name='example.com' port='6000'/>
  </source>
</nvram>

or

<nvram type='file'>
  <source file='/var/lib/libvirt/nvram/guest_VARS.fd'/>
</nvram>

Signed-off-by: Prerna Saxena <prerna.saxena at nutanix.com>
Signed-off-by: Florian Schmidt <flosch at nutanix.com>
Signed-off-by: Rohit Kumar <rohit.kumar3 at nutanix.com>
---
 src/conf/domain_conf.c   | 110 +++++++++++++++++++++++++++++++++------
 src/conf/domain_conf.h   |   1 +
 src/qemu/qemu_domain.c   |   9 ++++
 src/qemu/qemu_firmware.c |   6 +++
 src/qemu/qemu_validate.c |  61 ++++++++++++++++++++++
 5 files changed, 172 insertions(+), 15 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bdd36e777c..0429ff659e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17826,6 +17826,64 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
 }
 
 
+static int
+virDomainNvramDefParseXML(virDomainLoaderDef *loader,
+                          xmlXPathContextPtr ctxt,
+                          virDomainXMLOption *opt,
+                          unsigned int flags)
+{
+    g_autofree char *nvramType = NULL;
+
+    nvramType = virXPathString("string(./os/nvram/@type)", ctxt);
+
+    /* if nvramType==NULL read old-style, else
+     * if there's a type, read new style */
+    if (!nvramType) {
+        loader->newStyleNVRAM = false;
+        loader->nvram->type = VIR_STORAGE_TYPE_FILE;
+        loader->nvram->path = virXPathString("string(./os/nvram[1])", ctxt);
+        return 0;
+    } else {
+        xmlNodePtr source;
+        g_autofree char *srcProtocol = NULL;
+        g_autofree char *srcFile = NULL;
+
+        loader->newStyleNVRAM = true;
+        if ((loader->nvram->type = virStorageTypeFromString(nvramType)) <= 0) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("unknown disk type '%s'"), nvramType);
+            return -1;
+        }
+
+        srcProtocol = virXPathString("string(./os/nvram/source/@protocol)", ctxt);
+        if (srcProtocol && (loader->nvram->type != VIR_STORAGE_TYPE_NETWORK)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Source 'protocol' field is not supported with nvram type '%s'"),
+                           nvramType);
+            return -1;
+        }
+
+        srcFile = virXPathString("string(./os/nvram/source/@file)", ctxt);
+        if (!srcFile && (loader->nvram->type == VIR_STORAGE_TYPE_FILE)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Source 'file' attribute is missing with nvram type '%s'"),
+                           nvramType);
+            return -1;
+        }
+
+        source = virXPathNode("./os/nvram/source[1]", ctxt);
+        if (!source) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("Missing source element with nvram type '%s'"),
+                           nvramType);
+            return -1;
+        }
+        return virDomainStorageSourceParse(source, ctxt, loader->nvram, flags, opt);
+    }
+    return 0;
+}
+
+
 static int
 virDomainSchedulerParseCommonAttrs(xmlNodePtr node,
                                    virProcessSchedPolicy *policy,
@@ -18211,7 +18269,9 @@ virDomainDefParseBootFirmwareOptions(virDomainDef *def,
 
 static int
 virDomainDefParseBootLoaderOptions(virDomainDef *def,
-                                   xmlXPathContextPtr ctxt)
+                                   xmlXPathContextPtr ctxt,
+                                   virDomainXMLOption *xmlopt,
+                                   unsigned int flags)
 {
     xmlNodePtr loader_node = virXPathNode("./os/loader[1]", ctxt);
     const bool fwAutoSelect = def->os.firmware != VIR_DOMAIN_OS_DEF_FIRMWARE_NONE;
@@ -18228,8 +18288,10 @@ virDomainDefParseBootLoaderOptions(virDomainDef *def,
 
     if (virXPathNode("./os/nvram[1]", ctxt)) {
         def->os.loader->nvram = virStorageSourceNew();
-        def->os.loader->nvram->path = virXPathString("string(./os/nvram[1])", ctxt);
-        def->os.loader->nvram->type = VIR_STORAGE_TYPE_FILE;
+
+        if (virDomainNvramDefParseXML(def->os.loader,
+                                      ctxt, xmlopt, flags) < 0)
+            return -1;
     }
     if (!fwAutoSelect)
         def->os.loader->nvramTemplate = virXPathString("string(./os/nvram[1]/@template)", ctxt);
@@ -18284,7 +18346,9 @@ virDomainDefParseBootAcpiOptions(virDomainDef *def,
 
 static int
 virDomainDefParseBootOptions(virDomainDef *def,
-                             xmlXPathContextPtr ctxt)
+                             xmlXPathContextPtr ctxt,
+                             virDomainXMLOption *xmlopt,
+                             unsigned int flags)
 {
     /*
      * Booting options for different OS types....
@@ -18302,7 +18366,7 @@ virDomainDefParseBootOptions(virDomainDef *def,
         if (virDomainDefParseBootFirmwareOptions(def, ctxt) < 0)
             return -1;
 
-        if (virDomainDefParseBootLoaderOptions(def, ctxt) < 0)
+        if (virDomainDefParseBootLoaderOptions(def, ctxt, xmlopt, flags) < 0)
             return -1;
 
         if (virDomainDefParseBootAcpiOptions(def, ctxt) < 0)
@@ -18318,7 +18382,7 @@ virDomainDefParseBootOptions(virDomainDef *def,
     case VIR_DOMAIN_OSTYPE_UML:
         virDomainDefParseBootKernelOptions(def, ctxt);
 
-        if (virDomainDefParseBootLoaderOptions(def, ctxt) < 0)
+        if (virDomainDefParseBootLoaderOptions(def, ctxt, xmlopt, flags) < 0)
             return -1;
 
         break;
@@ -19609,7 +19673,7 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt,
     if (virDomainDefClockParse(def, ctxt) < 0)
         return NULL;
 
-    if (virDomainDefParseBootOptions(def, ctxt) < 0)
+    if (virDomainDefParseBootOptions(def, ctxt, xmlopt, flags) < 0)
         return NULL;
 
     /* analysis of the disk devices */
@@ -26909,14 +26973,16 @@ virDomainHugepagesFormat(virBuffer *buf,
     virBufferAddLit(buf, "</hugepages>\n");
 }
 
-static void
+static int
 virDomainLoaderDefFormat(virBuffer *buf,
-                         virDomainLoaderDef *loader)
+                         virDomainLoaderDef *loader,
+                         virDomainXMLOption *xmlopt,
+                         unsigned int flags)
 {
     g_auto(virBuffer) loaderAttrBuf = VIR_BUFFER_INITIALIZER;
     g_auto(virBuffer) loaderChildBuf = VIR_BUFFER_INITIALIZER;
     g_auto(virBuffer) nvramAttrBuf = VIR_BUFFER_INITIALIZER;
-    g_auto(virBuffer) nvramChildBuf = VIR_BUFFER_INITIALIZER;
+    g_auto(virBuffer) nvramChildBuf = VIR_BUFFER_INIT_CHILD(buf);
 
     if (loader->readonly != VIR_TRISTATE_BOOL_ABSENT)
         virBufferAsprintf(&loaderAttrBuf, " readonly='%s'",
@@ -26935,11 +27001,24 @@ virDomainLoaderDefFormat(virBuffer *buf,
     virXMLFormatElementInternal(buf, "loader", &loaderAttrBuf, &loaderChildBuf, false, false);
 
     virBufferEscapeString(&nvramAttrBuf, " template='%s'", loader->nvramTemplate);
-    if (loader->nvram) {
-        if (loader->nvram->type == VIR_STORAGE_TYPE_FILE)
+    if (loader->nvram || loader->nvramTemplate) {
+        bool childNewline;
+        if (!loader->newStyleNVRAM) {
+            virBufferSetIndent(&nvramChildBuf, 0);
             virBufferEscapeString(&nvramChildBuf, "%s", loader->nvram->path);
+            childNewline = false;
+        } else {
+            virBufferAsprintf(&nvramAttrBuf, " type='%s'", virStorageTypeToString(loader->nvram->type));
+            if (virDomainDiskSourceFormat(&nvramChildBuf, loader->nvram, "source", 0,
+                                          false, flags, false, false, xmlopt) < 0) {
+                return -1;
+            }
+            childNewline = true;
+        }
+        virXMLFormatElementInternal(buf, "nvram", &nvramAttrBuf, &nvramChildBuf, false, childNewline);
     }
-    virXMLFormatElementInternal(buf, "nvram", &nvramAttrBuf, &nvramChildBuf, false, false);
+
+    return 0;
 }
 
 static void
@@ -28140,8 +28219,9 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
     if (def->os.initgroup)
         virBufferAsprintf(buf, "<initgroup>%s</initgroup>\n", def->os.initgroup);
 
-    if (def->os.loader)
-        virDomainLoaderDefFormat(buf, def->os.loader);
+    if (def->os.loader &&
+        virDomainLoaderDefFormat(buf, def->os.loader, xmlopt, flags) < 0)
+        return -1;
     virBufferEscapeString(buf, "<kernel>%s</kernel>\n",
                           def->os.kernel);
     virBufferEscapeString(buf, "<initrd>%s</initrd>\n",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3a3164db9c..6523abaa0c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2236,6 +2236,7 @@ struct _virDomainLoaderDef {
     virDomainLoader type;
     virTristateBool secure;
     virStorageSource *nvram;
+    bool newStyleNVRAM;
     char *nvramTemplate;   /* user override of path to master nvram */
 };
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9dccbbfb1a..afe94eb018 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11283,6 +11283,15 @@ qemuDomainInitializePflashStorageSource(virDomainObj *vm)
         pflash1->format = VIR_STORAGE_FILE_RAW;
         pflash1->nodeformat = g_strdup("libvirt-pflash1-format");
         pflash1->nodestorage = g_strdup("libvirt-pflash1-storage");
+
+        if (!pflash1->privateData &&
+            !(pflash1->privateData = qemuDomainStorageSourcePrivateNew()))
+            return -1;
+
+        if (qemuDomainSecretStorageSourcePrepare(priv, pflash1,
+                                                 pflash1->nodestorage,
+                                                 pflash1->nodeformat) < 0)
+            return -1;
     }
 
     priv->pflash0 = g_steal_pointer(&pflash0);
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index ab5151dc9f..55808efd70 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1375,6 +1375,12 @@ qemuFirmwareFillDomain(virQEMUDriver *driver,
             virFileExists(def->os.loader->nvram->path))
             return 0;
 
+
+        if (!reset_nvram && def->os.loader->nvram &&
+            def->os.loader->nvram->type == VIR_STORAGE_TYPE_NETWORK &&
+            def->os.loader->nvram->path)
+            return 0;
+
         /* ... then we want to consult JSON FW descriptors first,
          * but we don't want to fail if we haven't found a match. */
         needResult = false;
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index b576efe375..28597ac702 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -603,6 +603,64 @@ qemuValidateDomainDefBoot(const virDomainDef *def)
 }
 
 
+static int
+qemuValidateDomainDefNvram(const virDomainDef *def,
+                           virQEMUCaps *qemuCaps)
+{
+    int actualType;
+    if (!def->os.loader || !def->os.loader->nvram)
+        return 0;
+
+    if (qemuDomainValidateStorageSource(def->os.loader->nvram, qemuCaps, false) < 0)
+        return -1;
+
+    actualType = virStorageSourceGetActualType(def->os.loader->nvram);
+
+    switch (actualType) {
+    case VIR_STORAGE_TYPE_FILE:
+        break;
+
+    case VIR_STORAGE_TYPE_NETWORK:
+    case VIR_STORAGE_TYPE_BLOCK:
+    case VIR_STORAGE_TYPE_DIR:
+    case VIR_STORAGE_TYPE_VOLUME:
+    case VIR_STORAGE_TYPE_NVME:
+    case VIR_STORAGE_TYPE_VHOST_USER:
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unsupported nvram disk type %s"),
+                       virStorageTypeToString(actualType));
+        return -1;
+
+    case VIR_STORAGE_TYPE_NONE:
+    case VIR_STORAGE_TYPE_LAST:
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unexpected nvram disk type %s"),
+                       virStorageTypeToString(actualType));
+        return -1;
+    }
+
+    if (def->os.loader->nvram->encryption) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                        _("Encryption is not supported with NVRAM"));
+        return -1;
+    }
+
+    if (def->os.loader->nvram->seclabels) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                        _("Security labels are not supported with NVRAM"));
+        return -1;
+    }
+
+    if (def->os.loader->nvram->auth) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                        _("Auth is not supported with NVRAM"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
 /**
  * qemuValidateDefGetVcpuHotplugGranularity:
  * @def: domain definition
@@ -1177,6 +1235,9 @@ qemuValidateDomainDef(const virDomainDef *def,
     if (qemuValidateDomainDefBoot(def) < 0)
         return -1;
 
+    if (qemuValidateDomainDefNvram(def, qemuCaps) < 0)
+        return -1;
+
     if (qemuValidateDomainVCpuTopology(def, qemuCaps) < 0)
         return -1;
 
-- 
2.25.1



More information about the libvir-list mailing list