[libvirt] [PATCH v2 04/15] virDomainLoaderDefParseXML: Allow loader path to be NULL

Michal Privoznik posted 15 patches 6 years, 11 months ago
[libvirt] [PATCH v2 04/15] virDomainLoaderDefParseXML: Allow loader path to be NULL
Posted by Michal Privoznik 6 years, 11 months ago
Except not really. At least for now.

In the future, the firmware will be selected automagically.
Therefore, it makes no sense to require the pathname of a
specific firmware binary in the domain XML. But since it is not
implemented do not really allow the path to be NULL. Only move
code around to prepare it for further expansion.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 docs/schemas/domaincommon.rng |  4 +++-
 src/conf/domain_conf.c        | 29 +++++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 80f9f84f70..c8d63c4912 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -283,7 +283,9 @@
                 </choice>
               </attribute>
             </optional>
-            <ref name="absFilePath"/>
+            <optional>
+              <ref name="absFilePath"/>
+            </optional>
           </element>
         </optional>
         <optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eb660f5764..053b2cb210 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6590,6 +6590,22 @@ virDomainDefMemtuneValidate(const virDomainDef *def)
 }
 
 
+static int
+virDomainDefOSValidate(const virDomainDef *def)
+{
+    if (!def->os.loader)
+        return 0;
+
+    if (!def->os.loader->path) {
+        virReportError(VIR_ERR_XML_DETAIL, "%s",
+                       _("no loader path specified"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 virDomainDefValidateInternal(const virDomainDef *def)
 {
@@ -6628,6 +6644,9 @@ virDomainDefValidateInternal(const virDomainDef *def)
     if (virDomainDefMemtuneValidate(def) < 0)
         return -1;
 
+    if (virDomainDefOSValidate(def) < 0)
+        return -1;
+
     return 0;
 }
 
@@ -18246,6 +18265,9 @@ virDomainLoaderDefParseXML(xmlNodePtr node,
     type_str = virXMLPropString(node, "type");
     loader->path = (char *) xmlNodeGetContent(node);
 
+    if (STREQ_NULLABLE(loader->path, ""))
+        VIR_FREE(loader->path);
+
     if (readonly_str &&
         (loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= 0) {
         virReportError(VIR_ERR_XML_DETAIL,
@@ -26989,9 +27011,12 @@ virDomainLoaderDefFormat(virBufferPtr buf,
     if (loader->secure)
         virBufferAsprintf(buf, " secure='%s'", secure);
 
-    virBufferAsprintf(buf, " type='%s'>", type);
+    virBufferAsprintf(buf, " type='%s'", type);
 
-    virBufferEscapeString(buf, "%s</loader>\n", loader->path);
+    if (loader->path)
+        virBufferEscapeString(buf, ">%s</loader>\n", loader->path);
+    else
+        virBufferAddLit(buf, "/>\n");
     if (loader->nvram || loader->templt) {
         virBufferAddLit(buf, "<nvram");
         virBufferEscapeString(buf, " template='%s'", loader->templt);
-- 
2.19.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/15] virDomainLoaderDefParseXML: Allow loader path to be NULL
Posted by Daniel P. Berrangé 6 years, 11 months ago
On Thu, Mar 07, 2019 at 10:29:14AM +0100, Michal Privoznik wrote:
> Except not really. At least for now.
> 
> In the future, the firmware will be selected automagically.
> Therefore, it makes no sense to require the pathname of a
> specific firmware binary in the domain XML. But since it is not
> implemented do not really allow the path to be NULL. Only move
> code around to prepare it for further expansion.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  docs/schemas/domaincommon.rng |  4 +++-
>  src/conf/domain_conf.c        | 29 +++++++++++++++++++++++++++--
>  2 files changed, 30 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 04/15] virDomainLoaderDefParseXML: Allow loader path to be NULL
Posted by Laszlo Ersek 6 years, 11 months ago
On 03/07/19 10:29, Michal Privoznik wrote:
> Except not really. At least for now.
> 
> In the future, the firmware will be selected automagically.
> Therefore, it makes no sense to require the pathname of a
> specific firmware binary in the domain XML. But since it is not
> implemented do not really allow the path to be NULL. Only move
> code around to prepare it for further expansion.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  docs/schemas/domaincommon.rng |  4 +++-
>  src/conf/domain_conf.c        | 29 +++++++++++++++++++++++++++--
>  2 files changed, 30 insertions(+), 3 deletions(-)

The commit message looks good. Thanks!
Laszlo

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list