The attribute can be used to disable ROM loading completely
for a device.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
docs/formatdomain.html.in | 3 +++
docs/schemas/domaincommon.rng | 5 +++++
src/conf/device_conf.h | 1 +
src/conf/domain_conf.c | 26 +++++++++++++++++++++++++-
4 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ada0df227f..0afc310e25 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4476,6 +4476,9 @@
virtual function of an sr-iov capable ethernet device (which
has no boot ROMs for the VFs).
<span class="since">Since 0.9.10 (QEMU and KVM only)</span>.
+ The optional <code>enabled</code> attribute can be set to
+ <code>no</code> to disable PCI ROM loading completely for the device.
+ <span class="since">Since 4.3.0 (QEMU and KVM only)</span>.
</dd>
<dt><code>address</code></dt>
<dd>The <code>address</code> element for USB devices has a
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4cab55f05d..3569b92127 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5108,6 +5108,11 @@
<define name="rom">
<element name="rom">
+ <optional>
+ <attribute name="enabled">
+ <ref name="virYesNo"/>
+ </attribute>
+ </optional>
<optional>
<attribute name="bar">
<ref name="virOnOff"/>
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index f87d6f1fc6..a31ce9c376 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -153,6 +153,7 @@ struct _virDomainDeviceInfo {
} master;
/* rombar and romfile are only used for pci hostdev and network
* devices. */
+ int romenabled; /* enum virTristateBool */
int rombar; /* enum virTristateSwitch */
char *romfile;
/* bootIndex is only used for disk, network interface, hostdev
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 35666c1347..3c152441df 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6095,9 +6095,17 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
}
if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM) &&
- (info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
+ (info->romenabled != VIR_TRISTATE_BOOL_ABSENT ||
+ info->rombar != VIR_TRISTATE_SWITCH_ABSENT ||
+ info->romfile)) {
virBufferAddLit(buf, "<rom");
+ if (info->romenabled != VIR_TRISTATE_BOOL_ABSENT) {
+ const char *romenabled = virTristateBoolTypeToString(info->romenabled);
+
+ if (romenabled)
+ virBufferAsprintf(buf, " enabled='%s'", romenabled);
+ }
if (info->rombar != VIR_TRISTATE_SWITCH_ABSENT) {
const char *rombar = virTristateSwitchTypeToString(info->rombar);
@@ -6738,6 +6746,7 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
xmlNodePtr boot = NULL;
xmlNodePtr rom = NULL;
char *type = NULL;
+ char *romenabled = NULL;
char *rombar = NULL;
char *aliasStr = NULL;
int ret = -1;
@@ -6791,6 +6800,12 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
}
if (rom) {
+ if ((romenabled = virXMLPropString(rom, "enabled")) &&
+ ((info->romenabled = virTristateBoolTypeFromString(romenabled)) <= 0)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown rom enabled value '%s'"), romenabled);
+ goto cleanup;
+ }
if ((rombar = virXMLPropString(rom, "bar")) &&
((info->rombar = virTristateSwitchTypeFromString(rombar)) <= 0)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -6798,6 +6813,14 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
goto cleanup;
}
info->romfile = virXMLPropString(rom, "file");
+
+ if (info->romenabled == VIR_TRISTATE_BOOL_NO &&
+ (info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ "%s",
+ _("ROM tuning is not supported when ROM is disabled"));
+ goto cleanup;
+ }
}
if (address &&
@@ -6811,6 +6834,7 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
virDomainDeviceInfoClear(info);
VIR_FREE(type);
VIR_FREE(rombar);
+ VIR_FREE(romenabled);
VIR_FREE(aliasStr);
return ret;
}
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Apr 20, 2018 at 17:44:29 +0200, Andrea Bolognani wrote:
> The attribute can be used to disable ROM loading completely
> for a device.
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> docs/formatdomain.html.in | 3 +++
> docs/schemas/domaincommon.rng | 5 +++++
> src/conf/device_conf.h | 1 +
> src/conf/domain_conf.c | 26 +++++++++++++++++++++++++-
> 4 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index ada0df227f..0afc310e25 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -4476,6 +4476,9 @@
> virtual function of an sr-iov capable ethernet device (which
> has no boot ROMs for the VFs).
> <span class="since">Since 0.9.10 (QEMU and KVM only)</span>.
> + The optional <code>enabled</code> attribute can be set to
> + <code>no</code> to disable PCI ROM loading completely for the device.
> + <span class="since">Since 4.3.0 (QEMU and KVM only)</span>.
Maybe you should mention that any other configration may not be
supported in that case.
[...]
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 35666c1347..3c152441df 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
[...]
> @@ -6798,6 +6813,14 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
> goto cleanup;
> }
> info->romfile = virXMLPropString(rom, "file");
> +
> + if (info->romenabled == VIR_TRISTATE_BOOL_NO &&
> + (info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
I'd explicitly allow empty string in info->romfile, but that would mean
that this needs to be moved to the qemu post-parse callback, since that
is a qemu quirk.
Justification is that, mgmt tools will be able to use enabled='no'
together with the empty file string without having to do any probing
whether that is a valid configuration.
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + "%s",
Above line can be merged into previous one.
> + _("ROM tuning is not supported when ROM is disabled"));
> + goto cleanup;
> + }
> }
>
> if (address &&
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, 2018-04-23 at 08:53 +0200, Peter Krempa wrote:
> > + The optional <code>enabled</code> attribute can be set to
> > + <code>no</code> to disable PCI ROM loading completely for the device.
> > + <span class="since">Since 4.3.0 (QEMU and KVM only)</span>.
>
> Maybe you should mention that any other configration may not be
> supported in that case.
Good idea.
> > @@ -6798,6 +6813,14 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
> > goto cleanup;
> > }
> > info->romfile = virXMLPropString(rom, "file");
> > +
> > + if (info->romenabled == VIR_TRISTATE_BOOL_NO &&
> > + (info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
>
> I'd explicitly allow empty string in info->romfile, but that would mean
> that this needs to be moved to the qemu post-parse callback, since that
> is a qemu quirk.
>
> Justification is that, mgmt tools will be able to use enabled='no'
> together with the empty file string without having to do any probing
> whether that is a valid configuration.
But enabled='no' would be rejected by earlier libvirt releases,
which makes the point about avoiding feature detection moot, no?
I would expect management applications that already use (invalid,
according to the schema, but working fine by all other counts)
file='' to keep using that until they bump their required libvirt
version to 4.3.0, and management applications that didn't already
use the existing kludge to just go straight for enabled='no'.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Apr 23, 2018 at 11:10:01 +0200, Andrea Bolognani wrote:
> On Mon, 2018-04-23 at 08:53 +0200, Peter Krempa wrote:
> > > + The optional <code>enabled</code> attribute can be set to
> > > + <code>no</code> to disable PCI ROM loading completely for the device.
> > > + <span class="since">Since 4.3.0 (QEMU and KVM only)</span>.
> >
> > Maybe you should mention that any other configration may not be
> > supported in that case.
>
> Good idea.
>
> > > @@ -6798,6 +6813,14 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt ATTRIBUTE_UNUSED,
> > > goto cleanup;
> > > }
> > > info->romfile = virXMLPropString(rom, "file");
> > > +
> > > + if (info->romenabled == VIR_TRISTATE_BOOL_NO &&
> > > + (info->rombar != VIR_TRISTATE_SWITCH_ABSENT || info->romfile)) {
> >
> > I'd explicitly allow empty string in info->romfile, but that would mean
> > that this needs to be moved to the qemu post-parse callback, since that
> > is a qemu quirk.
> >
> > Justification is that, mgmt tools will be able to use enabled='no'
> > together with the empty file string without having to do any probing
> > whether that is a valid configuration.
>
> But enabled='no' would be rejected by earlier libvirt releases,
> which makes the point about avoiding feature detection moot, no?
No. Only if they enable schema validation. Since that is an opt-in you
still can define such XML and the option will be ignored.
> I would expect management applications that already use (invalid,
> according to the schema, but working fine by all other counts)
> file='' to keep using that until they bump their required libvirt
> version to 4.3.0, and management applications that didn't already
> use the existing kludge to just go straight for enabled='no'.
I guess that is fair enough.
ACK then.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.