[libvirt] [PATCH v3 1/3] conf : Add loadparm boot option for a boot device
Farhan Ali
alifm at linux.vnet.ibm.com
Fri May 26 14:19:09 UTC 2017
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 at linux.vnet.ibm.com>
>> Reviewed-by: Bjoern Walk <bwalk at linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
>> Reviewed-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
>> ---
>> 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;
>> }
>>
>>
>
More information about the libvir-list
mailing list