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@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/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@xxxxxxxxxx>
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
> 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
