[libvirt] [PATCH v3 0/3] Loadparm support
This patch series introduces the support for new s390x 'loadparm' feature. The 'loadparm' can be used to select the boot entry to boot from, for a boot device. Here is a link to the QEMU patches: https://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg00192.html ChangeLog --------- v2 -> v3: - Updated news.xml and formatdomain.html.in with a more architectural description of loadparm (patch 1) v1 -> v2: - Rebased the patch series on the latest master, commit 2f69dd3 virfiletest: include linux/falloc.h Thanks Farhan Ali Farhan Ali (3): conf : Add loadparm boot option for a boot device qemu : Add loadparm to qemu command line string tests : Testcases for loadparm docs/formatdomain.html.in | 9 ++- docs/news.xml | 11 ++++ docs/schemas/domaincommon.rng | 7 +++ src/conf/device_conf.h | 1 + src/conf/domain_conf.c | 69 +++++++++++++++++++++- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 37 ++++++++++++ ...-machine-loadparm-multiple-disks-nets-s390.args | 28 +++++++++ ...v-machine-loadparm-multiple-disks-nets-s390.xml | 43 ++++++++++++++ .../qemuxml2argv-machine-loadparm-net-s390.args | 20 +++++++ .../qemuxml2argv-machine-loadparm-net-s390.xml | 26 ++++++++ ...xml2argv-machine-loadparm-s390-char-invalid.xml | 26 ++++++++ ...uxml2argv-machine-loadparm-s390-len-invalid.xml | 26 ++++++++ .../qemuxml2argv-machine-loadparm-s390.args | 20 +++++++ .../qemuxml2argv-machine-loadparm-s390.xml | 26 ++++++++ tests/qemuxml2argvtest.c | 19 ++++++ 17 files changed, 367 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-char-invalid.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-len-invalid.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.xml -- 1.9.1
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. Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 9 ++++-- docs/news.xml | 11 +++++++ docs/schemas/domaincommon.rng | 7 +++++ src/conf/device_conf.h | 1 + src/conf/domain_conf.c | 69 +++++++++++++++++++++++++++++++++++++++++-- 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> + 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 + 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> 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; } -- 1.9.1
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.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@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.
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.
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!
+ 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/ ?
+ 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
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; }
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@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@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; }
Introduce a new QEMU capability for loadparm and if the capability is supported by QEMU then append the loadparm value to "-machine" string of qemu command line. Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 546dfd7..e3bd445 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -371,6 +371,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "kernel-irqchip.split", "intel-iommu.intremap", "intel-iommu.caching-mode", + "loadparm", ); @@ -3144,6 +3145,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "drive", "throttling.group", QEMU_CAPS_DRIVE_IOTUNE_GROUP }, { "spice", "rendernode", QEMU_CAPS_SPICE_RENDERNODE }, { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP }, + { "machine", "loadparm", QEMU_CAPS_LOADPARM }, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index aa99fda..a18c61c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -409,6 +409,7 @@ typedef enum { QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT, /* -machine kernel_irqchip=split */ QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */ QEMU_CAPS_INTEL_IOMMU_CACHING_MODE, /* intel-iommu.caching-mode */ + QEMU_CAPS_LOADPARM, /* -machine loadparm */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aa66e3d..1291b62 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7226,6 +7226,41 @@ qemuAppendKeyWrapMachineParms(virBuffer *buf, virQEMUCapsPtr qemuCaps, return true; } +static void +qemuAppendLoadparmMachineParm(virBuffer *buf, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i = 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_LOADPARM)) + return; + + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + + if (disk->info.bootIndex == 1) { + if (disk->info.loadparm) + virBufferAsprintf(buf, ",loadparm=%s", disk->info.loadparm); + + return; + } + } + + /* Network boot device*/ + for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + + if (net->info.bootIndex == 1) { + if (net->info.loadparm) + virBufferAsprintf(buf, ",loadparm=%s", net->info.loadparm); + + return; + } + } +} + static int qemuBuildNameCommandLine(virCommandPtr cmd, virQEMUDriverConfigPtr cfg, @@ -7315,6 +7350,8 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-machine"); virBufferAdd(&buf, def->os.machine, -1); + qemuAppendLoadparmMachineParm(&buf, def, qemuCaps); + if (def->virtType == VIR_DOMAIN_VIRT_QEMU) virBufferAddLit(&buf, ",accel=tcg"); else if (def->virtType == VIR_DOMAIN_VIRT_KVM) -- 1.9.1
On 05/23/2017 09:27 AM, Farhan Ali wrote:
Introduce a new QEMU capability for loadparm and if the capability is supported by QEMU then append the loadparm value to "-machine" string of qemu command line.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+)
I was wondering why there weren't any changes to the tests/ qemucapabilitiesdata/*s390*.xml files... Until I realized this is a really new feature for qemu... As in post 2.9 and there is no 2.10 *.replies and *.xml file yet created... The *.xml files add the <flag name=...> entries for whatever qemu versions the machine loadparm=xxx flag can be found in order to define the flag to create the .args. But you have .args in v3, which is confusing (it's late too). See commit '6b5c6314' for a recently added capability for kernel-irqchip which modified a number of tests/qemucapabilitiesdata/* output .xml files. There's also "tests/qemucapsprobe /path/to/binary >caps.replies" which is what's used to generate those .replies files (those could be added as part of this or the xml2xml commit). Sometimes the patch order choice is have patch 1 have all the necessary flag stuff done, patch 2 the xml/rng/docs, patch 3 the qemu, and patch 4 the news.xml.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 546dfd7..e3bd445 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -371,6 +371,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "kernel-irqchip.split", "intel-iommu.intremap", "intel-iommu.caching-mode", + "loadparm", );
@@ -3144,6 +3145,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "drive", "throttling.group", QEMU_CAPS_DRIVE_IOTUNE_GROUP }, { "spice", "rendernode", QEMU_CAPS_SPICE_RENDERNODE }, { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP }, + { "machine", "loadparm", QEMU_CAPS_LOADPARM }, };
static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index aa99fda..a18c61c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -409,6 +409,7 @@ typedef enum { QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT, /* -machine kernel_irqchip=split */ QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */ QEMU_CAPS_INTEL_IOMMU_CACHING_MODE, /* intel-iommu.caching-mode */ + QEMU_CAPS_LOADPARM, /* -machine loadparm */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aa66e3d..1291b62 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7226,6 +7226,41 @@ qemuAppendKeyWrapMachineParms(virBuffer *buf, virQEMUCapsPtr qemuCaps, return true; }
The newer style is two lines between functions
+static void +qemuAppendLoadparmMachineParm(virBuffer *buf, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i = 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_LOADPARM)) + return;
Typically when the flag doesn't exist, we elicit an error message and cause failure. I'm still scratching my head over how this works for test when the flags aren't in the XML file, but I'm way too tired to think to hard about it.
+ + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + + if (disk->info.bootIndex == 1) { + if (disk->info.loadparm)
I think you could combine the if conditions... Unless there's thoughts that other bootIndex's would work... Could end up having arch specific stuff (yuck).
+ virBufferAsprintf(buf, ",loadparm=%s", disk->info.loadparm);> + + return; + } + } + + /* Network boot device*/ ^ Need a space before end of comment
+ for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + + if (net->info.bootIndex == 1) { + if (net->info.loadparm) + virBufferAsprintf(buf, ",loadparm=%s", net->info.loadparm); + + return; + } + } +} +
Two lines between functions
static int qemuBuildNameCommandLine(virCommandPtr cmd, virQEMUDriverConfigPtr cfg, @@ -7315,6 +7350,8 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-machine"); virBufferAdd(&buf, def->os.machine, -1);
+ qemuAppendLoadparmMachineParm(&buf, def, qemuCaps); +
Is this required to be the first parameter to -machine? If not then perhaps it'd be better to place it at the end after the irqchip stuff. Also, you could move the CapsGet into here so you can keep the void designation for the called function. Was there ever thought to adding loadparm to the machine XML? What's the reasoning to not have it there. If it's only valid for bootindex=1, then it's far easier to check if the machine XML has it defined rather than perusing the disk/network lists (which could be lengthy) only to fine none. If the concern is some other arch allowing a different bootindex to have loadparm, then the simple decision there is to have a "loadparm_bootindex=#" value that would match the disk bootindex=# value. Tks - John
if (def->virtType == VIR_DOMAIN_VIRT_QEMU) virBufferAddLit(&buf, ",accel=tcg"); else if (def->virtType == VIR_DOMAIN_VIRT_KVM)
On 05/25/2017 08:34 PM, John Ferlan wrote:
On 05/23/2017 09:27 AM, Farhan Ali wrote:
Introduce a new QEMU capability for loadparm and if the capability is supported by QEMU then append the loadparm value to "-machine" string of qemu command line.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+)
I was wondering why there weren't any changes to the tests/ qemucapabilitiesdata/*s390*.xml files... Until I realized this is a really new feature for qemu... As in post 2.9 and there is no 2.10 *.replies and *.xml file yet created...
The *.xml files add the <flag name=...> entries for whatever qemu versions the machine loadparm=xxx flag can be found in order to define the flag to create the .args. But you have .args in v3, which is confusing (it's late too).
See commit '6b5c6314' for a recently added capability for kernel-irqchip which modified a number of tests/qemucapabilitiesdata/* output .xml files.
There's also "tests/qemucapsprobe /path/to/binary >caps.replies" which is what's used to generate those .replies files (those could be added as part of this or the xml2xml commit).
Will add the relevant files for s390x qemu 2.10
Sometimes the patch order choice is have patch 1 have all the necessary flag stuff done, patch 2 the xml/rng/docs, patch 3 the qemu, and patch 4 the news.xml.
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 546dfd7..e3bd445 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -371,6 +371,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "kernel-irqchip.split", "intel-iommu.intremap", "intel-iommu.caching-mode", + "loadparm", );
@@ -3144,6 +3145,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "drive", "throttling.group", QEMU_CAPS_DRIVE_IOTUNE_GROUP }, { "spice", "rendernode", QEMU_CAPS_SPICE_RENDERNODE }, { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP }, + { "machine", "loadparm", QEMU_CAPS_LOADPARM }, };
static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index aa99fda..a18c61c 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -409,6 +409,7 @@ typedef enum { QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT, /* -machine kernel_irqchip=split */ QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */ QEMU_CAPS_INTEL_IOMMU_CACHING_MODE, /* intel-iommu.caching-mode */ + QEMU_CAPS_LOADPARM, /* -machine loadparm */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aa66e3d..1291b62 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7226,6 +7226,41 @@ qemuAppendKeyWrapMachineParms(virBuffer *buf, virQEMUCapsPtr qemuCaps, return true; }
The newer style is two lines between functions
Sure, wasn't aware of this. Thanks
+static void +qemuAppendLoadparmMachineParm(virBuffer *buf, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) +{ + size_t i = 0; + + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) || + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_LOADPARM)) + return;
Typically when the flag doesn't exist, we elicit an error message and cause failure. I'm still scratching my head over how this works for test when the flags aren't in the XML file, but I'm way too tired to think to hard about it.
+ + for (i = 0; i < def->ndisks; i++) { + virDomainDiskDefPtr disk = def->disks[i]; + + if (disk->info.bootIndex == 1) { + if (disk->info.loadparm)
I think you could combine the if conditions... Unless there's thoughts that other bootIndex's would work... Could end up having arch specific stuff (yuck).
+ virBufferAsprintf(buf, ",loadparm=%s", disk->info.loadparm);> + + return; + } + } + + /* Network boot device*/ ^ Need a space before end of comment
+ for (i = 0; i < def->nnets; i++) { + virDomainNetDefPtr net = def->nets[i]; + + if (net->info.bootIndex == 1) { + if (net->info.loadparm) + virBufferAsprintf(buf, ",loadparm=%s", net->info.loadparm); + + return; + } + } +} +
Two lines between functions
static int qemuBuildNameCommandLine(virCommandPtr cmd, virQEMUDriverConfigPtr cfg, @@ -7315,6 +7350,8 @@ qemuBuildMachineCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, "-machine"); virBufferAdd(&buf, def->os.machine, -1);
+ qemuAppendLoadparmMachineParm(&buf, def, qemuCaps); +
Is this required to be the first parameter to -machine? If not then perhaps it'd be better to place it at the end after the irqchip stuff.
Sure.
Also, you could move the CapsGet into here so you can keep the void designation for the called function.
okay
Was there ever thought to adding loadparm to the machine XML? What's the reasoning to not have it there. If it's only valid for bootindex=1, then it's far easier to check if the machine XML has it defined rather than perusing the disk/network lists (which could be lengthy) only to fine none. If the concern is some other arch allowing a different bootindex to have loadparm, then the simple decision there is to have a "loadparm_bootindex=#" value that would match the disk bootindex=# value.
I am not sure what you mean here? By machine xml do you mean <os> xml?
Tks -
John
if (def->virtType == VIR_DOMAIN_VIRT_QEMU) virBufferAddLit(&buf, ",accel=tcg"); else if (def->virtType == VIR_DOMAIN_VIRT_KVM)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
...
Was there ever thought to adding loadparm to the machine XML? What's the reasoning to not have it there. If it's only valid for bootindex=1, then it's far easier to check if the machine XML has it defined rather than perusing the disk/network lists (which could be lengthy) only to fine none. If the concern is some other arch allowing a different bootindex to have loadparm, then the simple decision there is to have a "loadparm_bootindex=#" value that would match the disk bootindex=# value.
I am not sure what you mean here? By machine xml do you mean <os> xml?
Sorry I was too lazy to go make the cross reference near the end of the day/review. Guess I was thinking more <os> related though... I see in <os> there's a <boot> subelement which has a relationship with the <boot> subelement for <devices>...[<interface>|<disk>]... I think I was just trying to make sure that adding <loadparm> to <devices> would make sense "in the long run" and what other implementations were considered and not taken because of some drawback. Given the description I've read and the implementation that searches either disk or network lists looking for bootIndex = 1, I wonder if the <os> <boot dev='xxx' > should be modified instead. Was that considered - what were the drawbacks? Can bootparm conceptually be added/used for a non boot disk? I'm not requesting one way or another - I'd just like to be sure the question is answered before perhaps someone else asks it now or much later when this isn't so fresh in your mind. John
On 05/26/2017 04:03 PM, John Ferlan wrote:
...
Was there ever thought to adding loadparm to the machine XML? What's the reasoning to not have it there. If it's only valid for bootindex=1, then it's far easier to check if the machine XML has it defined rather than perusing the disk/network lists (which could be lengthy) only to fine none. If the concern is some other arch allowing a different bootindex to have loadparm, then the simple decision there is to have a "loadparm_bootindex=#" value that would match the disk bootindex=# value.
I am not sure what you mean here? By machine xml do you mean <os> xml?
Sorry I was too lazy to go make the cross reference near the end of the day/review. Guess I was thinking more <os> related though...
I see in <os> there's a <boot> subelement which has a relationship with the <boot> subelement for <devices>...[<interface>|<disk>]...
Yes, the <os> has <boot> subelement, but it can be tricky to specify the order of the boot device.
I think I was just trying to make sure that adding <loadparm> to <devices> would make sense "in the long run" and what other implementations were considered and not taken because of some drawback.
The per device boot subelement was added to simplify the boot device order. That's why I thought it made more sense to add it to the per device boot element. Also from a user's perspective it's easier to specify the loadparm when specifying the boot order.
Given the description I've read and the implementation that searches either disk or network lists looking for bootIndex = 1, I wonder if the <os> <boot dev='xxx' > should be modified instead. Was that considered - what were the drawbacks? Can bootparm conceptually be added/used for a non boot disk?
I'm not requesting one way or another - I'd just like to be sure the question is answered before perhaps someone else asks it now or much later when this isn't so fresh in your mind.
John
Add testcases for loadparm Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- ...-machine-loadparm-multiple-disks-nets-s390.args | 28 ++++++++++++++ ...v-machine-loadparm-multiple-disks-nets-s390.xml | 43 ++++++++++++++++++++++ .../qemuxml2argv-machine-loadparm-net-s390.args | 20 ++++++++++ .../qemuxml2argv-machine-loadparm-net-s390.xml | 26 +++++++++++++ ...xml2argv-machine-loadparm-s390-char-invalid.xml | 26 +++++++++++++ ...uxml2argv-machine-loadparm-s390-len-invalid.xml | 26 +++++++++++++ .../qemuxml2argv-machine-loadparm-s390.args | 20 ++++++++++ .../qemuxml2argv-machine-loadparm-s390.xml | 26 +++++++++++++ tests/qemuxml2argvtest.c | 19 ++++++++++ 9 files changed, 234 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-char-invalid.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-len-invalid.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args new file mode 100644 index 0000000..c9e9061 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args @@ -0,0 +1,28 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-machine s390-ccw-virtio,loadparm=SYSTEM1,accel=tcg \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \ +-device virtio-blk-ccw,devno=fe.0.0002,drive=drive-virtio-disk0,\ +id=virtio-disk0,bootindex=1 \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-virtio-disk1 \ +-device virtio-blk-ccw,devno=fe.0.0003,drive=drive-virtio-disk1,\ +id=virtio-disk1,bootindex=3 \ +-device virtio-net-ccw,vlan=0,id=net0,mac=00:11:22:33:44:54,devno=fe.0.0000,\ +bootindex=2 \ +-net user,vlan=0,name=hostnet0 \ +-device virtio-net-ccw,vlan=1,id=net1,mac=00:11:22:33:42:36,devno=fe.0.0004 \ +-net user,vlan=1,name=hostnet1 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml new file mode 100644 index 0000000..df442e9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml @@ -0,0 +1,43 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <boot order='1' loadparm='system1'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x2'/> + </disk> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hdb' bus='virtio'/> + <boot order='3' loadparm='3'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x3'/> + </disk> + <interface type='user'> + <mac address='00:11:22:33:44:54'/> + <model type='virtio'/> + <boot order='2' loadparm='2.lp'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0'/> + </interface> + <interface type='user'> + <mac address='00:11:22:33:42:36'/> + <model type='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x4'/> + </interface> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x1'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args new file mode 100644 index 0000000..4ca2865 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args @@ -0,0 +1,20 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-machine s390-ccw-virtio,loadparm=2,accel=tcg \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-device virtio-net-ccw,vlan=0,id=net0,mac=00:11:22:33:44:54,devno=fe.0.0000,\ +bootindex=1 \ +-net user,vlan=0,name=hostnet0 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml new file mode 100644 index 0000000..ca682af --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <interface type='user'> + <mac address='00:11:22:33:44:54'/> + <model type='virtio'/> + <boot order='1' loadparm='2'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0'/> + </interface> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x1'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-char-invalid.xml b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-char-invalid.xml new file mode 100644 index 0000000..5cca5f9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-char-invalid.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <boot order='1' loadparm='sys1?'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0'/> + </disk> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x1'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-len-invalid.xml b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-len-invalid.xml new file mode 100644 index 0000000..ddc4baa --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-len-invalid.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <boot order='1' loadparm='loadparm1'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0'/> + </disk> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x1'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.args b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.args new file mode 100644 index 0000000..34855e9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.args @@ -0,0 +1,20 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-machine s390-ccw-virtio,loadparm=2,accel=tcg \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \ +-device virtio-blk-ccw,devno=fe.0.0000,drive=drive-virtio-disk0,\ +id=virtio-disk0,bootindex=1 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.xml b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.xml new file mode 100644 index 0000000..96db020 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <boot order='1' loadparm='2'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0'/> + </disk> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x1'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 4269598..690cd55 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2460,6 +2460,25 @@ mymain(void) QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST("machine-loadparm-s390", QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_BOOTINDEX, QEMU_CAPS_LOADPARM); + DO_TEST("machine-loadparm-net-s390", QEMU_CAPS_MACHINE_OPT, + QEMU_CAPS_VIRTIO_CCW, QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_BOOTINDEX, QEMU_CAPS_LOADPARM); + DO_TEST("machine-loadparm-multiple-disks-nets-s390", + QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390, QEMU_CAPS_BOOTINDEX, + QEMU_CAPS_LOADPARM); + DO_TEST_PARSE_ERROR("machine-loadparm-s390-char-invalid", + QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390, QEMU_CAPS_BOOTINDEX, + QEMU_CAPS_LOADPARM); + DO_TEST_PARSE_ERROR("machine-loadparm-s390-len-invalid", + QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_S390, QEMU_CAPS_BOOTINDEX, + QEMU_CAPS_LOADPARM); + DO_TEST("qemu-ns-domain-ns0", NONE); DO_TEST("qemu-ns-domain-commandline", NONE); DO_TEST("qemu-ns-domain-commandline-ns0", NONE); -- 1.9.1
On 05/23/2017 09:27 AM, Farhan Ali wrote:
Add testcases for loadparm
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> --- ...-machine-loadparm-multiple-disks-nets-s390.args | 28 ++++++++++++++ ...v-machine-loadparm-multiple-disks-nets-s390.xml | 43 ++++++++++++++++++++++ .../qemuxml2argv-machine-loadparm-net-s390.args | 20 ++++++++++ .../qemuxml2argv-machine-loadparm-net-s390.xml | 26 +++++++++++++ ...xml2argv-machine-loadparm-s390-char-invalid.xml | 26 +++++++++++++ ...uxml2argv-machine-loadparm-s390-len-invalid.xml | 26 +++++++++++++ .../qemuxml2argv-machine-loadparm-s390.args | 20 ++++++++++ .../qemuxml2argv-machine-loadparm-s390.xml | 26 +++++++++++++ tests/qemuxml2argvtest.c | 19 ++++++++++ 9 files changed, 234 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-char-invalid.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-len-invalid.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.xml
These all seem reasonable - but could be merged with patch2 which had the qemu changes. John
participants (2)
-
Farhan Ali -
John Ferlan