Re: [PATCH v3 1/3] conf : Add loadparm boot option for a boot device

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

 





On 05/25/2017 07:58 PM, John Ferlan wrote:


On 05/23/2017 09:27 AM, Farhan Ali wrote:
Update the per device boot schema to add an optional loadparm parameter.
Extend the virDomainDeviceInfo to support loadparm option.
Modify the appropriate functions to parse loadparm from boot device xml.


FWIW: I got confused by the time I got to patch 3... the "loadparm" add
added to the -machine qemu command line, but the libvirt XML is added to
the disk device entry.

Perhaps just add a sample here to show where the XML gets added.


Sure, will add an example.

Signed-off-by: Farhan Ali <alifm@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Bjoern Walk <bwalk@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxxxxxxx>
---
 docs/formatdomain.html.in     |  9 ++++--
 docs/news.xml                 | 11 +++++++

Patch ordering - the docs/news.xml should be a single commit and would
be the the last commit.  Still at least you have it there.

Will move it as a separate commit.


 docs/schemas/domaincommon.rng |  7 +++++
 src/conf/device_conf.h        |  1 +
 src/conf/domain_conf.c        | 69 +++++++++++++++++++++++++++++++++++++++++--

When you change/add RNG/XML - there should be adjustments to
qemuxml2xmltest.c and of course xml2xml tests.  Many examples of this to
draw from.


Sure, will add a test case for xml2xml tests.

 5 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3135db4..fd6f666 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3022,8 +3022,13 @@
       <dt><code>boot</code></dt>
       <dd>Specifies that the disk is bootable. The <code>order</code>
         attribute determines the order in which devices will be tried during
-        boot sequence. The per-device <code>boot</code> elements cannot be
-        used together with general boot elements in
+        boot sequence. On S390 architecture only the first boot device is used.
+        The optional <code>loadparm</code> attribute is an 8 byte parameter
+        which can be queried by guests on S390 via sclp or diag 308. Linux
+        guests on S390 can use <code>loadparm</code> to select a boot entry.
+        <span class="since">Since 3.4.0</span>

Since DV is cutting 3.4.0 tomorrow, this probably slides into 3.5.0.
Given when I realized by patch 2, probably no big deal!

Okay

+        The per-device <code>boot</code> elements cannot be used together
+        with general boot elements in
         <a href="#elementsOSBIOS">BIOS bootloader</a> section.
         <span class="since">Since 0.8.8</span>
       </dd>
diff --git a/docs/news.xml b/docs/news.xml
index 52915ee..bd9e43a 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,17 @@
     <section title="New features">
       <change>
         <summary>
+          qemu: Add support for loadparm for a boot device
+        </summary>
+        <description>
+          Add an optional boot parameter 'loadparm' for a boot device.
+          Loadparm is a 8 byte parameter than can be queried by S390 guests

s/a 8/an 8/

s/can be/when present is/ ?

Yes, thanks for catching that.


+          via sclp or diag 308. Linux guests on S390 use it to select a boot
+          entry.
+        </description>
+      </change>
+      <change>
+        <summary>
           Improved streams to efficiently transfer sparseness
         </summary>
         <description>

The rest seems fine -

John

Thanks for reviewing

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f88e84a..c885aec 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5026,6 +5026,13 @@
       <attribute name="order">
         <ref name="positiveInteger"/>
       </attribute>
+      <optional>
+        <attribute name="loadparm">
+          <data type="string">
+            <param name="pattern">[a-zA-Z0-9.\s]{1,8}</param>
+          </data>
+        </attribute>
+      </optional>
       <empty/>
     </element>
   </define>
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index a20de85..48782be 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -167,6 +167,7 @@ struct _virDomainDeviceInfo {
      * assignment, never saved and never reported.
      */
     int pciConnectFlags; /* enum virDomainPCIConnectFlags */
+    char *loadparm;
 };


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8c62673..dbf6108 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3520,6 +3520,7 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
     memset(&info->addr, 0, sizeof(info->addr));
     info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
     VIR_FREE(info->romfile);
+    VIR_FREE(info->loadparm);
 }


@@ -5199,6 +5200,48 @@ virDomainDefValidate(virDomainDefPtr def,
     return 0;
 }

+/**
+ * virDomainDeviceLoadparmIsValid
+ * @loadparm : The string to validate
+ *
+ * The valid set of values for loadparm are [a-zA-Z0-9.]
+ * and blank spaces.
+ * The maximum allowed length is 8 characters.
+ * An empty string is considered invalid
+ */
+static bool
+virDomainDeviceLoadparmIsValid(const char *loadparm)
+{
+    size_t i;
+
+    if (virStringIsEmpty(loadparm)) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("loadparm cannot be an empty string"));
+        return false;
+    }
+
+    if (strlen(loadparm) > 8) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("loadparm '%s' exceeds 8 characters"), loadparm);
+        return false;
+    }
+
+    for (i = 0; i < strlen(loadparm); i++) {
+        uint8_t c = loadparm[i];
+
+        if (('A' <= c && c <= 'Z') || ('0' <= c && c <= '9') ||
+            (c == '.') || (c == ' ')) {
+            continue;
+        } else {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("invalid loadparm char '%c', expecting chars"
+                             " in set of [a-zA-Z0-9.] and blank spaces"), c);
+            return false;
+        }
+    }
+
+    return true;
+}

 /* Generate a string representation of a device address
  * @info address Device address to stringify
@@ -5208,9 +5251,14 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
                           virDomainDeviceInfoPtr info,
                           unsigned int flags)
 {
-    if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex)
-        virBufferAsprintf(buf, "<boot order='%u'/>\n", info->bootIndex);
+    if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex) {
+        virBufferAsprintf(buf, "<boot order='%u'", info->bootIndex);

+        if (info->loadparm)
+            virBufferAsprintf(buf, " loadparm='%s'", info->loadparm);
+
+        virBufferAddLit(buf, "/>\n");
+    }
     if (info->alias &&
         !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
         virBufferAsprintf(buf, "<alias name='%s'/>\n", info->alias);
@@ -5636,6 +5684,7 @@ virDomainDeviceBootParseXML(xmlNodePtr node,
                             virHashTablePtr bootHash)
 {
     char *order;
+    char *loadparm;
     int ret = -1;

     if (!(order = virXMLPropString(node, "order"))) {
@@ -5664,10 +5713,26 @@ virDomainDeviceBootParseXML(xmlNodePtr node,
             goto cleanup;
     }

+    loadparm = virXMLPropString(node, "loadparm");
+    if (loadparm) {
+        if (virStringToUpper(&info->loadparm, loadparm) != 1) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Failed to convert loadparm '%s' to upper case"),
+                           loadparm);
+            goto cleanup;
+        }
+
+        if (!virDomainDeviceLoadparmIsValid(info->loadparm)) {
+            VIR_FREE(info->loadparm);
+            goto cleanup;
+        }
+    }
+
     ret = 0;

  cleanup:
     VIR_FREE(order);
+    VIR_FREE(loadparm);
     return ret;
 }




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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux