[libvirt] [PATCH v5 1/4] conf : Add loadparm boot option for a boot device

Farhan Ali alifm at linux.vnet.ibm.com
Thu Jun 15 12:28:28 UTC 2017



On 06/14/2017 01:12 PM, John Ferlan wrote:
>
>
> On 06/01/2017 12:36 PM, Farhan Ali wrote:
>> Update the per device boot schema to add an optional loadparm parameter.
>>
>> eg: <boot order='1' loadparm='2'/>
>>
>> Extend the virDomainDeviceInfo to support loadparm option.
>> Modify the appropriate functions to parse loadparm from boot device xml.
>>
>> 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/schemas/domaincommon.rng |  7 +++++
>>  src/conf/device_conf.h        |  1 +
>>  src/conf/domain_conf.c        | 69 +++++++++++++++++++++++++++++++++++++++++--
>>  4 files changed, 82 insertions(+), 4 deletions(-)
>>
>
> I believe it was stated previously that the xml2xml tests should be in
> this patch. So I moved them. The xml2xml test doesn't require all the
> flags that were set and I moved it to be with the other "machine-*" tests.
>
> Reviewed-by: John Ferlan <jferlan at redhat.com>
>
> I will push this (and the subsequent patches) later this week just to be
> sure no one chimes in after my review to note something I missed (or in
> case some of the wording I modified below needs more adjustment).
>
> John
>

Hi John,

The changes look good to me. Thanks for reviewing it.

Thanks
Farhan

>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 07208ee..837be18 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.
>
> s/On/On the/
>
>> +        The optional <code>loadparm</code> attribute is an 8 byte parameter
>
> s/byte parameter/character string/
>
>> +        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.5.0</span>
>> +        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/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 4d9f8d1..ef09860 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -5031,6 +5031,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 c7e20b8..cd35ad2 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);
>>  }
>>
>>
>> @@ -5213,6 +5214,48 @@ virDomainDefValidate(virDomainDefPtr def,
>>      return 0;
>>  }
>
> Now prefer two blank lines between functions.
>
>>
>> +/**
>> + * 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
>> @@ -5222,9 +5265,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);
>> @@ -5650,6 +5698,7 @@ virDomainDeviceBootParseXML(xmlNodePtr node,
>>                              virHashTablePtr bootHash)
>>  {
>>      char *order;
>> +    char *loadparm;
>>      int ret = -1;
>>
>>      if (!(order = virXMLPropString(node, "order"))) {
>> @@ -5678,10 +5727,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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>




More information about the libvir-list mailing list