docs/formatdomain.rst | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
Even though this was brought up in upstream discussion [1] it
missed my patches: users should prefer <oemStrings/> over fwcfg.
The reason is that fwcfg is considered somewhat internal to QEMU
and it has limited number of slots and neither of these applies
to <oemStrings/>.
While I'm at it, I'm fixing the example too (because it contains
incorrect element name) and clarifying sysfs/ exposure.
1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html
Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
docs/formatdomain.rst | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 1979dfb8d3..821ffe8d60 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -509,18 +509,22 @@ layout of sub-elements, with supported values of:
Some hypervisors provide unified way to tweak how firmware configures itself,
or may contain tables to be installed for the guest OS, for instance boot
order, ACPI, SMBIOS, etc. It even allows users to define their own config
- blobs. In case of QEMU, these then appear under domain's sysfs, under
+ blobs. In case of QEMU, these then appear under domain's sysfs (if the guest
+ kernel has FW_CFG_SYSFS config option enabled), under
``/sys/firmware/qemu_fw_cfg``. Note, that these values apply regardless the
<smbios/> mode under <os/>. :since:`Since 6.5.0`
+ **Please note that because of limited number of data slots use of fwcfg is
+ strongly discouraged and <oemStrings/> should be used instead**.
+
::
- <smbios type='fwcfg'>
+ <sysinfo type='fwcfg'>
<entry name='opt/com.example/name'>example value</entry>
- <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/>
- </smbios>
+ <entry name='opt/com.example/config' file='/tmp/provision.ign'/>
+ </sysinfo>
- The ``smbios`` element can have multiple ``entry`` child elements. Each
+ The ``sysinfo`` element can have multiple ``entry`` child elements. Each
element then has mandatory ``name`` attribute, which defines the name of the
blob and must begin with ``"opt/"`` and to avoid clashing with other names is
advised to be in form ``"opt/$RFQDN/$name"`` where ``$RFQDN`` is a reverse
--
2.26.2
On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: >Even though this was brought up in upstream discussion [1] it >missed my patches: users should prefer <oemStrings/> over fwcfg. >The reason is that fwcfg is considered somewhat internal to QEMU >and it has limited number of slots and neither of these applies >to <oemStrings/>. > >While I'm at it, I'm fixing the example too (because it contains >incorrect element name) and clarifying sysfs/ exposure. > >1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html > >Reported-by: Laszlo Ersek <lersek@redhat.com> >Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >--- > docs/formatdomain.rst | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > >diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst >index 1979dfb8d3..821ffe8d60 100644 >--- a/docs/formatdomain.rst >+++ b/docs/formatdomain.rst >@@ -509,18 +509,22 @@ layout of sub-elements, with supported values of: > Some hypervisors provide unified way to tweak how firmware configures itself, > or may contain tables to be installed for the guest OS, for instance boot > order, ACPI, SMBIOS, etc. It even allows users to define their own config >- blobs. In case of QEMU, these then appear under domain's sysfs, under >+ blobs. In case of QEMU, these then appear under domain's sysfs (if the guest >+ kernel has FW_CFG_SYSFS config option enabled), under > ``/sys/firmware/qemu_fw_cfg``. Note, that these values apply regardless the > <smbios/> mode under <os/>. :since:`Since 6.5.0` > >+ **Please note that because of limited number of data slots use of fwcfg is >+ strongly discouraged and <oemStrings/> should be used instead**. >+ It's nice that you say that, but people who would like to use fw_cfg for passing in a huge blob, which is saved in a file, will read this, go to <oemStrings/> and see that there is no way to pass a file as an input. Should that be dealt with somehow? Or would that be discouraged as well?
On 9/7/20 3:57 PM, Martin Kletzander wrote: > On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: >> Even though this was brought up in upstream discussion [1] it >> missed my patches: users should prefer <oemStrings/> over fwcfg. >> The reason is that fwcfg is considered somewhat internal to QEMU >> and it has limited number of slots and neither of these applies >> to <oemStrings/>. >> >> While I'm at it, I'm fixing the example too (because it contains >> incorrect element name) and clarifying sysfs/ exposure. >> >> 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >> >> Reported-by: Laszlo Ersek <lersek@redhat.com> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> --- >> docs/formatdomain.rst | 14 +++++++++----- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> > It's nice that you say that, but people who would like to use fw_cfg for > passing > in a huge blob, which is saved in a file, will read this, go to > <oemStrings/> > and see that there is no way to pass a file as an input. Should that be > dealt > with somehow? Or would that be discouraged as well? Unfortunately, QEMU doesn't allow reading OEM strings from a file (at least quick glance over hw/smbios/smbios.c doesn't show any signs it's allowed). Michal
On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: > On 9/7/20 3:57 PM, Martin Kletzander wrote: > > On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: > > > Even though this was brought up in upstream discussion [1] it > > > missed my patches: users should prefer <oemStrings/> over fwcfg. > > > The reason is that fwcfg is considered somewhat internal to QEMU > > > and it has limited number of slots and neither of these applies > > > to <oemStrings/>. > > > > > > While I'm at it, I'm fixing the example too (because it contains > > > incorrect element name) and clarifying sysfs/ exposure. > > > > > > 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html > > > > > > Reported-by: Laszlo Ersek <lersek@redhat.com> > > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > > > --- > > > docs/formatdomain.rst | 14 +++++++++----- > > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > > > > It's nice that you say that, but people who would like to use fw_cfg for > > passing > > in a huge blob, which is saved in a file, will read this, go to > > <oemStrings/> > > and see that there is no way to pass a file as an input. Should that be > > dealt > > with somehow? Or would that be discouraged as well? > > Unfortunately, QEMU doesn't allow reading OEM strings from a file (at least > quick glance over hw/smbios/smbios.c doesn't show any signs it's allowed). Indeed, that is an RFE I've never got around to Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Sep 07, 2020 at 03:38:23PM +0100, Daniel P. Berrangé wrote: >On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: >> On 9/7/20 3:57 PM, Martin Kletzander wrote: >> > On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: >> > > Even though this was brought up in upstream discussion [1] it >> > > missed my patches: users should prefer <oemStrings/> over fwcfg. >> > > The reason is that fwcfg is considered somewhat internal to QEMU >> > > and it has limited number of slots and neither of these applies >> > > to <oemStrings/>. >> > > >> > > While I'm at it, I'm fixing the example too (because it contains >> > > incorrect element name) and clarifying sysfs/ exposure. >> > > >> > > 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >> > > >> > > Reported-by: Laszlo Ersek <lersek@redhat.com> >> > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> > > --- >> > > docs/formatdomain.rst | 14 +++++++++----- >> > > 1 file changed, 9 insertions(+), 5 deletions(-) >> > > >> >> >> > It's nice that you say that, but people who would like to use fw_cfg for >> > passing >> > in a huge blob, which is saved in a file, will read this, go to >> > <oemStrings/> >> > and see that there is no way to pass a file as an input. Should that be >> > dealt >> > with somehow? Or would that be discouraged as well? >> >> Unfortunately, QEMU doesn't allow reading OEM strings from a file (at least >> quick glance over hw/smbios/smbios.c doesn't show any signs it's allowed). > >Indeed, that is an RFE I've never got around to > Do you mean RFE for QEMU or libvirt? Are there any limitations for the oemStrings? Libvirt could read the file and pass the data to QEMU as it is not something that is supposed to be writeable or shareable. I understand it could be the first time we do something like this, but it might not be that bad of a precedence. Just so we are on the same page, I'm not against this patch, I just want to save our users the frustration that I, myself, experienced so many times. There's something deep hurting when you finally find exactly what you're looking for, only to learn that it is deprecated with another option which does not provide the same functionality. Sure, you can have a start hook that reads a file and changes the data, etc. But you get the point. At least we could mention that? >Regards, >Daniel >-- >|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| >|: https://libvirt.org -o- https://fstop138.berrange.com :| >|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Sep 08, 2020 at 08:37:03AM +0200, Martin Kletzander wrote: > On Mon, Sep 07, 2020 at 03:38:23PM +0100, Daniel P. Berrangé wrote: > > On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: > > > On 9/7/20 3:57 PM, Martin Kletzander wrote: > > > > On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: > > > > > Even though this was brought up in upstream discussion [1] it > > > > > missed my patches: users should prefer <oemStrings/> over fwcfg. > > > > > The reason is that fwcfg is considered somewhat internal to QEMU > > > > > and it has limited number of slots and neither of these applies > > > > > to <oemStrings/>. > > > > > > > > > > While I'm at it, I'm fixing the example too (because it contains > > > > > incorrect element name) and clarifying sysfs/ exposure. > > > > > > > > > > 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html > > > > > > > > > > Reported-by: Laszlo Ersek <lersek@redhat.com> > > > > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > > > > > --- > > > > > docs/formatdomain.rst | 14 +++++++++----- > > > > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > > > > > > > > > > > > It's nice that you say that, but people who would like to use fw_cfg for > > > > passing > > > > in a huge blob, which is saved in a file, will read this, go to > > > > <oemStrings/> > > > > and see that there is no way to pass a file as an input. Should that be > > > > dealt > > > > with somehow? Or would that be discouraged as well? > > > > > > Unfortunately, QEMU doesn't allow reading OEM strings from a file (at least > > > quick glance over hw/smbios/smbios.c doesn't show any signs it's allowed). > > > > Indeed, that is an RFE I've never got around to > > > > Do you mean RFE for QEMU or libvirt? Both. > Are there any limitations for the oemStrings? Libvirt could read the file and > pass the data to QEMU as it is not something that is supposed to be writeable or > shareable. I understand it could be the first time we do something like this, > but it might not be that bad of a precedence. The problem is the size of the command line you end up with. You need to have QMEU support for reading from the file. > Just so we are on the same page, I'm not against this patch, I just want to save > our users the frustration that I, myself, experienced so many times. There's > something deep hurting when you finally find exactly what you're looking for, > only to learn that it is deprecated with another option which does not provide > the same functionality. Sure, you can have a start hook that reads a file and > changes the data, etc. But you get the point. At least we could mention that? Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, Sep 08, 2020 at 08:03:46AM +0100, Daniel P. Berrangé wrote: >On Tue, Sep 08, 2020 at 08:37:03AM +0200, Martin Kletzander wrote: >> On Mon, Sep 07, 2020 at 03:38:23PM +0100, Daniel P. Berrangé wrote: >> > On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: >> > > On 9/7/20 3:57 PM, Martin Kletzander wrote: >> > > > On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: >> > > > > Even though this was brought up in upstream discussion [1] it >> > > > > missed my patches: users should prefer <oemStrings/> over fwcfg. >> > > > > The reason is that fwcfg is considered somewhat internal to QEMU >> > > > > and it has limited number of slots and neither of these applies >> > > > > to <oemStrings/>. >> > > > > >> > > > > While I'm at it, I'm fixing the example too (because it contains >> > > > > incorrect element name) and clarifying sysfs/ exposure. >> > > > > >> > > > > 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >> > > > > >> > > > > Reported-by: Laszlo Ersek <lersek@redhat.com> >> > > > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> > > > > --- >> > > > > docs/formatdomain.rst | 14 +++++++++----- >> > > > > 1 file changed, 9 insertions(+), 5 deletions(-) >> > > > > >> > > >> > > >> > > > It's nice that you say that, but people who would like to use fw_cfg for >> > > > passing >> > > > in a huge blob, which is saved in a file, will read this, go to >> > > > <oemStrings/> >> > > > and see that there is no way to pass a file as an input. Should that be >> > > > dealt >> > > > with somehow? Or would that be discouraged as well? >> > > >> > > Unfortunately, QEMU doesn't allow reading OEM strings from a file (at least >> > > quick glance over hw/smbios/smbios.c doesn't show any signs it's allowed). >> > >> > Indeed, that is an RFE I've never got around to >> > >> >> Do you mean RFE for QEMU or libvirt? > >Both. > >> Are there any limitations for the oemStrings? Libvirt could read the file and >> pass the data to QEMU as it is not something that is supposed to be writeable or >> shareable. I understand it could be the first time we do something like this, >> but it might not be that bad of a precedence. > >The problem is the size of the command line you end up with. Good point. >You need to have QEMU support for reading from the file. > Or from an FD, ideally. >> Just so we are on the same page, I'm not against this patch, I just want to save >> our users the frustration that I, myself, experienced so many times. There's >> something deep hurting when you finally find exactly what you're looking for, >> only to learn that it is deprecated with another option which does not provide >> the same functionality. Sure, you can have a start hook that reads a file and >> changes the data, etc. But you get the point. At least we could mention that? > > >Regards, >Daniel >-- >|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| >|: https://libvirt.org -o- https://fstop138.berrange.com :| >|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 09/08/20 08:37, Martin Kletzander wrote: > On Mon, Sep 07, 2020 at 03:38:23PM +0100, Daniel P. Berrangé wrote: >> On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: >>> On 9/7/20 3:57 PM, Martin Kletzander wrote: >>> > On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: >>> > > Even though this was brought up in upstream discussion [1] it >>> > > missed my patches: users should prefer <oemStrings/> over fwcfg. >>> > > The reason is that fwcfg is considered somewhat internal to QEMU >>> > > and it has limited number of slots and neither of these applies >>> > > to <oemStrings/>. >>> > > >>> > > While I'm at it, I'm fixing the example too (because it contains >>> > > incorrect element name) and clarifying sysfs/ exposure. >>> > > >>> > > 1: >>> https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >>> > > >>> > > Reported-by: Laszlo Ersek <lersek@redhat.com> >>> > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >>> > > --- >>> > > docs/formatdomain.rst | 14 +++++++++----- >>> > > 1 file changed, 9 insertions(+), 5 deletions(-) >>> > > >>> >>> >>> > It's nice that you say that, but people who would like to use >>> fw_cfg for >>> > passing >>> > in a huge blob, which is saved in a file, will read this, go to >>> > <oemStrings/> >>> > and see that there is no way to pass a file as an input. Should >>> that be >>> > dealt >>> > with somehow? Or would that be discouraged as well? >>> >>> Unfortunately, QEMU doesn't allow reading OEM strings from a file (at >>> least >>> quick glance over hw/smbios/smbios.c doesn't show any signs it's >>> allowed). >> >> Indeed, that is an RFE I've never got around to >> > > Do you mean RFE for QEMU or libvirt? > > Are there any limitations for the oemStrings? Libvirt could read the > file and > pass the data to QEMU as it is not something that is supposed to be > writeable or > shareable. I understand it could be the first time we do something like > this, > but it might not be that bad of a precedence. > > Just so we are on the same page, I'm not against this patch, I just want > to save > our users the frustration that I, myself, experienced so many times. > There's > something deep hurting when you finally find exactly what you're looking > for, > only to learn that it is deprecated with another option which does not > provide > the same functionality. This description is not entirely accurate. I would put it like this: you stumble upon a mis-use of a feature that was originally meant for something else. It seems that the original consumers of the feature have always been unhappy about the mis-use (it presents a risk for the subsystem they are responsible for), in spite of the mis-use possibly scratching your itch too. That shouldn't hurt your feelings -- the point is that it's not "deprecation"; the original thing has never been condoned for this particular creative kind of abuse. Instead, the feature that could scratch your itch has never existed. Thanks, Laszlo > Sure, you can have a start hook that reads a > file and > changes the data, etc. But you get the point. At least we could > mention that? > >> Regards, >> Daniel >> -- >> |: https://berrange.com     -o-   >> https://www.flickr.com/photos/dberrange :| >> |: https://libvirt.org        -o-           >> https://fstop138.berrange.com :| >> |: https://entangle-photo.org   -o-   >> https://www.instagram.com/dberrange :|
On Tue, Sep 08, 2020 at 10:22:34AM +0200, Laszlo Ersek wrote: >On 09/08/20 08:37, Martin Kletzander wrote: >> On Mon, Sep 07, 2020 at 03:38:23PM +0100, Daniel P. Berrangé wrote: >>> On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: >>>> On 9/7/20 3:57 PM, Martin Kletzander wrote: >>>> > On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: >>>> > > Even though this was brought up in upstream discussion [1] it >>>> > > missed my patches: users should prefer <oemStrings/> over fwcfg. >>>> > > The reason is that fwcfg is considered somewhat internal to QEMU >>>> > > and it has limited number of slots and neither of these applies >>>> > > to <oemStrings/>. >>>> > > >>>> > > While I'm at it, I'm fixing the example too (because it contains >>>> > > incorrect element name) and clarifying sysfs/ exposure. >>>> > > >>>> > > 1: >>>> https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >>>> > > >>>> > > Reported-by: Laszlo Ersek <lersek@redhat.com> >>>> > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >>>> > > --- >>>> > > docs/formatdomain.rst | 14 +++++++++----- >>>> > > 1 file changed, 9 insertions(+), 5 deletions(-) >>>> > > >>>> >>>> >>>> > It's nice that you say that, but people who would like to use >>>> fw_cfg for >>>> > passing >>>> > in a huge blob, which is saved in a file, will read this, go to >>>> > <oemStrings/> >>>> > and see that there is no way to pass a file as an input. Should >>>> that be >>>> > dealt >>>> > with somehow? Or would that be discouraged as well? >>>> >>>> Unfortunately, QEMU doesn't allow reading OEM strings from a file (at >>>> least >>>> quick glance over hw/smbios/smbios.c doesn't show any signs it's >>>> allowed). >>> >>> Indeed, that is an RFE I've never got around to >>> >> >> Do you mean RFE for QEMU or libvirt? >> >> Are there any limitations for the oemStrings? Libvirt could read the >> file and >> pass the data to QEMU as it is not something that is supposed to be >> writeable or >> shareable. I understand it could be the first time we do something like >> this, >> but it might not be that bad of a precedence. >> >> Just so we are on the same page, I'm not against this patch, I just want >> to save >> our users the frustration that I, myself, experienced so many times. >> There's >> something deep hurting when you finally find exactly what you're looking >> for, >> only to learn that it is deprecated with another option which does not >> provide >> the same functionality. > >This description is not entirely accurate. I would put it like this: you >stumble upon a mis-use of a feature that was originally meant for >something else. It seems that the original consumers of the feature have >always been unhappy about the mis-use (it presents a risk for the >subsystem they are responsible for), in spite of the mis-use possibly >scratching your itch too. > That is *also* true, it's just a view from the other perspective. >That shouldn't hurt your feelings -- the point is that it's not >"deprecation"; the original thing has never been condoned for this >particular creative kind of abuse. Instead, the feature that could >scratch your itch has never existed. > This works for empathic readers only ;-) >Thanks, >Laszlo > > >> Sure, you can have a start hook that reads a >> file and >> changes the data, etc. But you get the point. At least we could >> mention that? >> >>> Regards, >>> Daniel >>> -- >>> |: https://berrange.com -o- >>> https://www.flickr.com/photos/dberrange :| >>> |: https://libvirt.org -o- >>> https://fstop138.berrange.com :| >>> |: https://entangle-photo.org -o- >>> https://www.instagram.com/dberrange :| >
On Tue, Sep 08, 2020 at 10:22:34AM +0200, Laszlo Ersek wrote: > On 09/08/20 08:37, Martin Kletzander wrote: > > On Mon, Sep 07, 2020 at 03:38:23PM +0100, Daniel P. Berrangé wrote: > >> On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: > >>> On 9/7/20 3:57 PM, Martin Kletzander wrote: > >>> > On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: > >>> > > Even though this was brought up in upstream discussion [1] it > >>> > > missed my patches: users should prefer <oemStrings/> over fwcfg. > >>> > > The reason is that fwcfg is considered somewhat internal to QEMU > >>> > > and it has limited number of slots and neither of these applies > >>> > > to <oemStrings/>. > >>> > > > >>> > > While I'm at it, I'm fixing the example too (because it contains > >>> > > incorrect element name) and clarifying sysfs/ exposure. > >>> > > > >>> > > 1: > >>> https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html > >>> > > > >>> > > Reported-by: Laszlo Ersek <lersek@redhat.com> > >>> > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > >>> > > --- > >>> > > docs/formatdomain.rst | 14 +++++++++----- > >>> > > 1 file changed, 9 insertions(+), 5 deletions(-) > >>> > > > >>> > >>> > >>> > It's nice that you say that, but people who would like to use > >>> fw_cfg for > >>> > passing > >>> > in a huge blob, which is saved in a file, will read this, go to > >>> > <oemStrings/> > >>> > and see that there is no way to pass a file as an input. Should > >>> that be > >>> > dealt > >>> > with somehow? Or would that be discouraged as well? > >>> > >>> Unfortunately, QEMU doesn't allow reading OEM strings from a file (at > >>> least > >>> quick glance over hw/smbios/smbios.c doesn't show any signs it's > >>> allowed). > >> > >> Indeed, that is an RFE I've never got around to > >> > > > > Do you mean RFE for QEMU or libvirt? > > > > Are there any limitations for the oemStrings? Libvirt could read the > > file and > > pass the data to QEMU as it is not something that is supposed to be > > writeable or > > shareable. I understand it could be the first time we do something like > > this, > > but it might not be that bad of a precedence. > > > > Just so we are on the same page, I'm not against this patch, I just want > > to save > > our users the frustration that I, myself, experienced so many times. > > There's > > something deep hurting when you finally find exactly what you're looking > > for, > > only to learn that it is deprecated with another option which does not > > provide > > the same functionality. > > This description is not entirely accurate. I would put it like this: you > stumble upon a mis-use of a feature that was originally meant for > something else. It seems that the original consumers of the feature have > always been unhappy about the mis-use (it presents a risk for the > subsystem they are responsible for), in spite of the mis-use possibly > scratching your itch too. > > That shouldn't hurt your feelings -- the point is that it's not > "deprecation"; the original thing has never been condoned for this > particular creative kind of abuse. Instead, the feature that could > scratch your itch has never existed. I understand why you wish that was the case, but I don't think that is the reality today. The QEMU manual page has explicitly documented -fw_cfg, even going as far as illustrating its usage for passing application specific data to the guest: [quote] Add named fw_cfg entry with contents from string str. The terminating NUL character of the contents of str will not be included as part of the fw_cfg item data. To insert contents with embedded NUL characters, you have to use the file parameter. The fw_cfg entries are passed by QEMU through to the guest. Example: -fw_cfg name=opt/com.mycompany/blob,file=./my_blob.bin creates an fw_cfg entry named opt/com.mycompany/blob with contents from ./my_blob.bin. [/quote] Similarly the -help output merely lists -fw_cfg as a "debug/expert" option, and doesn't give any indication it is not for application usage. IOW, saying that this is not for general application usage is directly contradicting QEMU's own documentation that exists today. If you want to change such that this option is marked not for application usage, then that is certainly a deprecation of an existing documented feature in QEMU. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 09/08/20 11:05, Daniel P. Berrangé wrote: > On Tue, Sep 08, 2020 at 10:22:34AM +0200, Laszlo Ersek wrote: >> On 09/08/20 08:37, Martin Kletzander wrote: >>> On Mon, Sep 07, 2020 at 03:38:23PM +0100, Daniel P. Berrangé wrote: >>>> On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: >>>>> On 9/7/20 3:57 PM, Martin Kletzander wrote: >>>>>> On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: >>>>>>> Even though this was brought up in upstream discussion [1] it >>>>>>> missed my patches: users should prefer <oemStrings/> over fwcfg. >>>>>>> The reason is that fwcfg is considered somewhat internal to QEMU >>>>>>> and it has limited number of slots and neither of these applies >>>>>>> to <oemStrings/>. >>>>>>> >>>>>>> While I'm at it, I'm fixing the example too (because it contains >>>>>>> incorrect element name) and clarifying sysfs/ exposure. >>>>>>> >>>>>>> 1: >>>>> https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >>>>>>> >>>>>>> Reported-by: Laszlo Ersek <lersek@redhat.com> >>>>>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >>>>>>> --- >>>>>>> docs/formatdomain.rst | 14 +++++++++----- >>>>>>> 1 file changed, 9 insertions(+), 5 deletions(-) >>>>>>> >>>>> >>>>> >>>>>> It's nice that you say that, but people who would like to use >>>>> fw_cfg for >>>>>> passing >>>>>> in a huge blob, which is saved in a file, will read this, go to >>>>>> <oemStrings/> >>>>>> and see that there is no way to pass a file as an input. Should >>>>> that be >>>>>> dealt >>>>>> with somehow? Or would that be discouraged as well? >>>>> >>>>> Unfortunately, QEMU doesn't allow reading OEM strings from a file (at >>>>> least >>>>> quick glance over hw/smbios/smbios.c doesn't show any signs it's >>>>> allowed). >>>> >>>> Indeed, that is an RFE I've never got around to >>>> >>> >>> Do you mean RFE for QEMU or libvirt? >>> >>> Are there any limitations for the oemStrings? Libvirt could read the >>> file and >>> pass the data to QEMU as it is not something that is supposed to be >>> writeable or >>> shareable. I understand it could be the first time we do something like >>> this, >>> but it might not be that bad of a precedence. >>> >>> Just so we are on the same page, I'm not against this patch, I just want >>> to save >>> our users the frustration that I, myself, experienced so many times. >>> There's >>> something deep hurting when you finally find exactly what you're looking >>> for, >>> only to learn that it is deprecated with another option which does not >>> provide >>> the same functionality. >> >> This description is not entirely accurate. I would put it like this: you >> stumble upon a mis-use of a feature that was originally meant for >> something else. It seems that the original consumers of the feature have >> always been unhappy about the mis-use (it presents a risk for the >> subsystem they are responsible for), in spite of the mis-use possibly >> scratching your itch too. >> >> That shouldn't hurt your feelings -- the point is that it's not >> "deprecation"; the original thing has never been condoned for this >> particular creative kind of abuse. Instead, the feature that could >> scratch your itch has never existed. > > I understand why you wish that was the case, but I don't think that > is the reality today. > > The QEMU manual page has explicitly documented -fw_cfg, even going > as far as illustrating its usage for passing application specific > data to the guest: > > [quote] > Add named fw_cfg entry with contents from string str. > > The terminating NUL character of the contents of str will > not be included as part of the fw_cfg item data. To insert > contents with embedded NUL characters, you have to use the > file parameter. > > The fw_cfg entries are passed by QEMU through to the guest. > > Example: > -fw_cfg name=opt/com.mycompany/blob,file=./my_blob.bin > > creates an fw_cfg entry named opt/com.mycompany/blob with > contents from ./my_blob.bin. > [/quote] > > Similarly the -help output merely lists -fw_cfg as a "debug/expert" > option, and doesn't give any indication it is not for application > usage. > > IOW, saying that this is not for general application usage is > directly contradicting QEMU's own documentation that exists today. > > If you want to change such that this option is marked not for > application usage, then that is certainly a deprecation of an > existing documented feature in QEMU. ... fair enough I guess. Laszlo
On 09/07/20 16:38, Daniel P. Berrangé wrote: > On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: >> On 9/7/20 3:57 PM, Martin Kletzander wrote: >>> On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: >>>> Even though this was brought up in upstream discussion [1] it >>>> missed my patches: users should prefer <oemStrings/> over fwcfg. >>>> The reason is that fwcfg is considered somewhat internal to QEMU >>>> and it has limited number of slots and neither of these applies >>>> to <oemStrings/>. >>>> >>>> While I'm at it, I'm fixing the example too (because it contains >>>> incorrect element name) and clarifying sysfs/ exposure. >>>> >>>> 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >>>> >>>> Reported-by: Laszlo Ersek <lersek@redhat.com> >>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >>>> --- >>>> docs/formatdomain.rst | 14 +++++++++----- >>>> 1 file changed, 9 insertions(+), 5 deletions(-) >>>> >> >> >>> It's nice that you say that, but people who would like to use fw_cfg for >>> passing >>> in a huge blob, which is saved in a file, will read this, go to >>> <oemStrings/> >>> and see that there is no way to pass a file as an input. Should that be >>> dealt >>> with somehow? Or would that be discouraged as well? >> >> Unfortunately, QEMU doesn't allow reading OEM strings from a file (at least >> quick glance over hw/smbios/smbios.c doesn't show any signs it's allowed). > > Indeed, that is an RFE I've never got around to Yes, I remember that -- I remember that we discussed this, and also that you didn't have time for it. Which is entirely justifiable; other stuff has been (and continues to be) more important. For reference, here's the launchpad ticket that tracks the RFE for QEMU: https://bugs.launchpad.net/qemu/+bug/1826200 Thanks, Laszlo
On Tue, Sep 08, 2020 at 10:17:08AM +0200, Laszlo Ersek wrote: > On 09/07/20 16:38, Daniel P. Berrangé wrote: > > On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: > >> On 9/7/20 3:57 PM, Martin Kletzander wrote: > >>> On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: > >>>> Even though this was brought up in upstream discussion [1] it > >>>> missed my patches: users should prefer <oemStrings/> over fwcfg. > >>>> The reason is that fwcfg is considered somewhat internal to QEMU > >>>> and it has limited number of slots and neither of these applies > >>>> to <oemStrings/>. > >>>> > >>>> While I'm at it, I'm fixing the example too (because it contains > >>>> incorrect element name) and clarifying sysfs/ exposure. > >>>> > >>>> 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html > >>>> > >>>> Reported-by: Laszlo Ersek <lersek@redhat.com> > >>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > >>>> --- > >>>> docs/formatdomain.rst | 14 +++++++++----- > >>>> 1 file changed, 9 insertions(+), 5 deletions(-) > >>>> > >> > >> > >>> It's nice that you say that, but people who would like to use fw_cfg for > >>> passing > >>> in a huge blob, which is saved in a file, will read this, go to > >>> <oemStrings/> > >>> and see that there is no way to pass a file as an input. Should that be > >>> dealt > >>> with somehow? Or would that be discouraged as well? > >> > >> Unfortunately, QEMU doesn't allow reading OEM strings from a file (at least > >> quick glance over hw/smbios/smbios.c doesn't show any signs it's allowed). > > > > Indeed, that is an RFE I've never got around to > > Yes, I remember that -- I remember that we discussed this, and also that > you didn't have time for it. Which is entirely justifiable; other stuff > has been (and continues to be) more important. > > For reference, here's the launchpad ticket that tracks the RFE for QEMU: > > https://bugs.launchpad.net/qemu/+bug/1826200 I spent some time today (more than expected in fact!) implementing this.... https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg03023.html I hit a possible EDK2 bug when SMBIOS 3.0 where it hangs when the size exceeds about 128 KB, but I don't intend to investigate it further. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 09/08/20 19:26, Daniel P. Berrangé wrote: > On Tue, Sep 08, 2020 at 10:17:08AM +0200, Laszlo Ersek wrote: >> On 09/07/20 16:38, Daniel P. Berrangé wrote: >>> On Mon, Sep 07, 2020 at 04:20:02PM +0200, Michal Privoznik wrote: >>>> On 9/7/20 3:57 PM, Martin Kletzander wrote: >>>>> On Mon, Sep 07, 2020 at 03:48:16PM +0200, Michal Privoznik wrote: >>>>>> Even though this was brought up in upstream discussion [1] it >>>>>> missed my patches: users should prefer <oemStrings/> over fwcfg. >>>>>> The reason is that fwcfg is considered somewhat internal to QEMU >>>>>> and it has limited number of slots and neither of these applies >>>>>> to <oemStrings/>. >>>>>> >>>>>> While I'm at it, I'm fixing the example too (because it contains >>>>>> incorrect element name) and clarifying sysfs/ exposure. >>>>>> >>>>>> 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >>>>>> >>>>>> Reported-by: Laszlo Ersek <lersek@redhat.com> >>>>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >>>>>> --- >>>>>> docs/formatdomain.rst | 14 +++++++++----- >>>>>> 1 file changed, 9 insertions(+), 5 deletions(-) >>>>>> >>>> >>>> >>>>> It's nice that you say that, but people who would like to use fw_cfg for >>>>> passing >>>>> in a huge blob, which is saved in a file, will read this, go to >>>>> <oemStrings/> >>>>> and see that there is no way to pass a file as an input. Should that be >>>>> dealt >>>>> with somehow? Or would that be discouraged as well? >>>> >>>> Unfortunately, QEMU doesn't allow reading OEM strings from a file (at least >>>> quick glance over hw/smbios/smbios.c doesn't show any signs it's allowed). >>> >>> Indeed, that is an RFE I've never got around to >> >> Yes, I remember that -- I remember that we discussed this, and also that >> you didn't have time for it. Which is entirely justifiable; other stuff >> has been (and continues to be) more important. >> >> For reference, here's the launchpad ticket that tracks the RFE for QEMU: >> >> https://bugs.launchpad.net/qemu/+bug/1826200 > > I spent some time today (more than expected in fact!) implementing > this.... > > https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg03023.html That was quick! Thank you! > I hit a possible EDK2 bug when SMBIOS 3.0 where it hangs when the > size exceeds about 128 KB, but I don't intend to investigate it > further. I'll look into it. Thanks! Laszlo
On 09/07/20 15:48, Michal Privoznik wrote: > Even though this was brought up in upstream discussion [1] it > missed my patches: users should prefer <oemStrings/> over fwcfg. > The reason is that fwcfg is considered somewhat internal to QEMU > and it has limited number of slots and neither of these applies > to <oemStrings/>. > > While I'm at it, I'm fixing the example too (because it contains > incorrect element name) and clarifying sysfs/ exposure. > > 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html > > Reported-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > --- > docs/formatdomain.rst | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > index 1979dfb8d3..821ffe8d60 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst > @@ -509,18 +509,22 @@ layout of sub-elements, with supported values of: > Some hypervisors provide unified way to tweak how firmware configures itself, > or may contain tables to be installed for the guest OS, for instance boot > order, ACPI, SMBIOS, etc. It even allows users to define their own config > - blobs. In case of QEMU, these then appear under domain's sysfs, under > + blobs. In case of QEMU, these then appear under domain's sysfs (if the guest > + kernel has FW_CFG_SYSFS config option enabled), under > ``/sys/firmware/qemu_fw_cfg``. Note, that these values apply regardless the > <smbios/> mode under <os/>. :since:`Since 6.5.0` > > + **Please note that because of limited number of data slots use of fwcfg is > + strongly discouraged and <oemStrings/> should be used instead**. please replace: strongly discouraged with: strongly discouraged for configuring any guest-side component other than the firmware ( Consider for example the following feature: https://bugzilla.tianocore.org/show_bug.cgi?id=2681 Namely, the following QEMU switches: -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn] -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn] alter the behavior of OVMF and ArmVirtQemu. These flags are meant to be stable. They do not need dedicated QEMU or libvirtd enablement. They influence firmware behavior. So <sysinfo type='fwcfg'> is perfectly fine (even ideal!) for tweaking them, through the domain XML. What's not fine is configuring any random guest payload via <sysinfo type='fwcfg'>. The point is that people who parse new fw_cfg files in edk2 such as "opt/org.tianocore/IPv6PXESupport" are conscious of the slot count in QEMU. They *can* bump the "x-file-slots" property in QEMU, for new machine types, they just need to be aware of the property. ) > + > :: > > - <smbios type='fwcfg'> > + <sysinfo type='fwcfg'> > <entry name='opt/com.example/name'>example value</entry> I suggest (according to the above): - name: opt/org.tianocore/IPv4PXESupport - value: n > - <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> > - </smbios> > + <entry name='opt/com.example/config' file='/tmp/provision.ign'/> We have a functional -- working, stable -- example for name+file as well: - name: etc/edk2/https/cacerts - file: /etc/pki/ca-trust/extracted/edk2/cacerts.bin (This is documented in "OvmfPkg/README" in edk2, but it's not really relevant here.) > + </sysinfo> > > - The ``smbios`` element can have multiple ``entry`` child elements. Each > + The ``sysinfo`` element can have multiple ``entry`` child elements. Each > element then has mandatory ``name`` attribute, which defines the name of the > blob and must begin with ``"opt/"`` and to avoid clashing with other names is > advised to be in form ``"opt/$RFQDN/$name"`` where ``$RFQDN`` is a reverse > It's hard to express this cleanly. - The opt/RFQDN notation is a mitigation for users that are hell-bent on using fw-cfg files of their own purposes (not heeding our advice about not using fw-cfg for such purposes at all). So the idea is, "if you ignore our request, then (a) be prepared to run out of slots, and (b) *at least* use a name pattern (opt/RFQDN) that minimizes conflicts with other, similar-minded users / projects" - For "officially supported" knobs that the firmware looks at, it's fine to use any names -- they avoid conflicts with the above "rogue" files. Examples: - opt/ovmf/ -- reserved for historical reasons - opt/org.tianocore/ -- should never conflict due to RFQDN - etc/edk2/https/... -- should never conflict due to being outside of opt/ So I guess the short rule is, "Feel free to refer to any fw_cfg file name that your firmware officially supports. When defining other fw_cfg file names (i.e., for your own purposes), then prepare for breakage in the long-term, and then at least use the opt/RFQDN/ name pattern". Thank you, Laszlo
On Tue, Sep 08, 2020 at 11:02:10AM +0200, Laszlo Ersek wrote: > On 09/07/20 15:48, Michal Privoznik wrote: > > Even though this was brought up in upstream discussion [1] it > > missed my patches: users should prefer <oemStrings/> over fwcfg. > > The reason is that fwcfg is considered somewhat internal to QEMU > > and it has limited number of slots and neither of these applies > > to <oemStrings/>. > > > > While I'm at it, I'm fixing the example too (because it contains > > incorrect element name) and clarifying sysfs/ exposure. > > > > 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html > > > > Reported-by: Laszlo Ersek <lersek@redhat.com> > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > > --- > > docs/formatdomain.rst | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > > index 1979dfb8d3..821ffe8d60 100644 > > --- a/docs/formatdomain.rst > > +++ b/docs/formatdomain.rst > > @@ -509,18 +509,22 @@ layout of sub-elements, with supported values of: > > Some hypervisors provide unified way to tweak how firmware configures itself, > > or may contain tables to be installed for the guest OS, for instance boot > > order, ACPI, SMBIOS, etc. It even allows users to define their own config > > - blobs. In case of QEMU, these then appear under domain's sysfs, under > > + blobs. In case of QEMU, these then appear under domain's sysfs (if the guest > > + kernel has FW_CFG_SYSFS config option enabled), under > > ``/sys/firmware/qemu_fw_cfg``. Note, that these values apply regardless the > > <smbios/> mode under <os/>. :since:`Since 6.5.0` > > > > + **Please note that because of limited number of data slots use of fwcfg is > > + strongly discouraged and <oemStrings/> should be used instead**. > > please replace: > > strongly discouraged > > with: > > strongly discouraged for configuring any guest-side component other > than the firmware > > ( > > Consider for example the following feature: > > https://bugzilla.tianocore.org/show_bug.cgi?id=2681 > > Namely, the following QEMU switches: > > -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn] > -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn] > > alter the behavior of OVMF and ArmVirtQemu. These flags are meant to be > stable. They do not need dedicated QEMU or libvirtd enablement. They > influence firmware behavior. So <sysinfo type='fwcfg'> is perfectly fine > (even ideal!) for tweaking them, through the domain XML. What's not fine > is configuring any random guest payload via <sysinfo type='fwcfg'>. > > The point is that people who parse new fw_cfg files in edk2 such as > "opt/org.tianocore/IPv6PXESupport" are conscious of the slot count in > QEMU. They *can* bump the "x-file-slots" property in QEMU, for new > machine types, they just need to be aware of the property. I'd suggest that QEMU always sets x-file-slots to be 10 entries larger than the number of officially known slots. That leads some scope for app usage without risk of hitting the limits. > > - <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> > > - </smbios> > > + <entry name='opt/com.example/config' file='/tmp/provision.ign'/> > > We have a functional -- working, stable -- example for name+file as > well: > > - name: etc/edk2/https/cacerts > - file: /etc/pki/ca-trust/extracted/edk2/cacerts.bin I don't think we should document that in libvirt, since it is something libvirt will set silently behind the scenes. The ignition example is a acceptable real world example of expected usage in libvirt. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 09/08/20 11:12, Daniel P. Berrangé wrote: > On Tue, Sep 08, 2020 at 11:02:10AM +0200, Laszlo Ersek wrote: >> On 09/07/20 15:48, Michal Privoznik wrote: >>> Even though this was brought up in upstream discussion [1] it >>> missed my patches: users should prefer <oemStrings/> over fwcfg. >>> The reason is that fwcfg is considered somewhat internal to QEMU >>> and it has limited number of slots and neither of these applies >>> to <oemStrings/>. >>> >>> While I'm at it, I'm fixing the example too (because it contains >>> incorrect element name) and clarifying sysfs/ exposure. >>> >>> 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >>> >>> Reported-by: Laszlo Ersek <lersek@redhat.com> >>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >>> --- >>> docs/formatdomain.rst | 14 +++++++++----- >>> 1 file changed, 9 insertions(+), 5 deletions(-) >>> >>> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst >>> index 1979dfb8d3..821ffe8d60 100644 >>> --- a/docs/formatdomain.rst >>> +++ b/docs/formatdomain.rst >>> @@ -509,18 +509,22 @@ layout of sub-elements, with supported values of: >>> Some hypervisors provide unified way to tweak how firmware configures itself, >>> or may contain tables to be installed for the guest OS, for instance boot >>> order, ACPI, SMBIOS, etc. It even allows users to define their own config >>> - blobs. In case of QEMU, these then appear under domain's sysfs, under >>> + blobs. In case of QEMU, these then appear under domain's sysfs (if the guest >>> + kernel has FW_CFG_SYSFS config option enabled), under >>> ``/sys/firmware/qemu_fw_cfg``. Note, that these values apply regardless the >>> <smbios/> mode under <os/>. :since:`Since 6.5.0` >>> >>> + **Please note that because of limited number of data slots use of fwcfg is >>> + strongly discouraged and <oemStrings/> should be used instead**. >> >> please replace: >> >> strongly discouraged >> >> with: >> >> strongly discouraged for configuring any guest-side component other >> than the firmware >> >> ( >> >> Consider for example the following feature: >> >> https://bugzilla.tianocore.org/show_bug.cgi?id=2681 >> >> Namely, the following QEMU switches: >> >> -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn] >> -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn] >> >> alter the behavior of OVMF and ArmVirtQemu. These flags are meant to be >> stable. They do not need dedicated QEMU or libvirtd enablement. They >> influence firmware behavior. So <sysinfo type='fwcfg'> is perfectly fine >> (even ideal!) for tweaking them, through the domain XML. What's not fine >> is configuring any random guest payload via <sysinfo type='fwcfg'>. >> >> The point is that people who parse new fw_cfg files in edk2 such as >> "opt/org.tianocore/IPv6PXESupport" are conscious of the slot count in >> QEMU. They *can* bump the "x-file-slots" property in QEMU, for new >> machine types, they just need to be aware of the property. > > I'd suggest that QEMU always sets x-file-slots to be 10 entries larger > than the number of officially known slots. That leads some scope for > app usage without risk of hitting the limits. Right, at least I believe we've tried maintaining a "safety margin". The "officially known slots" is sort of a fuzzy count one can deduce from grepping the QEMU, SeaBIOS and edk2 source trees. So when we'd like to introduce a new file, we redo those greps, and compare with the current x-file-slots value (for the latest machine types, that is). > > >>> - <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> >>> - </smbios> >>> + <entry name='opt/com.example/config' file='/tmp/provision.ign'/> >> >> We have a functional -- working, stable -- example for name+file as >> well: >> >> - name: etc/edk2/https/cacerts >> - file: /etc/pki/ca-trust/extracted/edk2/cacerts.bin > > I don't think we should document that in libvirt, since it is something > libvirt will set silently behind the scenes. The ignition example is a > acceptable real world example of expected usage in libvirt. OK. I wasn't sure if specific libvirt enablement was going to occur for the CA certs passing. Thanks, Laszlo
On 9/8/20 11:02 AM, Laszlo Ersek wrote: > On 09/07/20 15:48, Michal Privoznik wrote: >> Even though this was brought up in upstream discussion [1] it >> missed my patches: users should prefer <oemStrings/> over fwcfg. >> The reason is that fwcfg is considered somewhat internal to QEMU >> and it has limited number of slots and neither of these applies >> to <oemStrings/>. >> >> While I'm at it, I'm fixing the example too (because it contains >> incorrect element name) and clarifying sysfs/ exposure. >> >> 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >> >> Reported-by: Laszlo Ersek <lersek@redhat.com> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> --- >> docs/formatdomain.rst | 14 +++++++++----- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst >> index 1979dfb8d3..821ffe8d60 100644 >> --- a/docs/formatdomain.rst >> +++ b/docs/formatdomain.rst >> @@ -509,18 +509,22 @@ layout of sub-elements, with supported values of: >> Some hypervisors provide unified way to tweak how firmware configures itself, >> or may contain tables to be installed for the guest OS, for instance boot >> order, ACPI, SMBIOS, etc. It even allows users to define their own config >> - blobs. In case of QEMU, these then appear under domain's sysfs, under >> + blobs. In case of QEMU, these then appear under domain's sysfs (if the guest >> + kernel has FW_CFG_SYSFS config option enabled), under >> ``/sys/firmware/qemu_fw_cfg``. Note, that these values apply regardless the >> <smbios/> mode under <os/>. :since:`Since 6.5.0` >> >> + **Please note that because of limited number of data slots use of fwcfg is >> + strongly discouraged and <oemStrings/> should be used instead**. > > please replace: > > strongly discouraged > > with: > > strongly discouraged for configuring any guest-side component other > than the firmware Actually, we have a check that forbids anything else than "opt/" prefix and also "opt/ovmf/" and "opt/org.qemu/" are forbidden: https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_validate.c#L780 Looks like "opt/org.tianocore" slipped through. I think the reasoning is that this is too low level and allows changing values of knobs that are exposed elsewhere (e.g. boot order). It really should be just to append new values, not change existing ones. > > ( > > Consider for example the following feature: > > https://bugzilla.tianocore.org/show_bug.cgi?id=2681 > > Namely, the following QEMU switches: > > -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn] > -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn] > > alter the behavior of OVMF and ArmVirtQemu. These flags are meant to be > stable. They do not need dedicated QEMU or libvirtd enablement. They > influence firmware behavior. So <sysinfo type='fwcfg'> is perfectly fine > (even ideal!) for tweaking them, through the domain XML. What's not fine > is configuring any random guest payload via <sysinfo type='fwcfg'>. > > The point is that people who parse new fw_cfg files in edk2 such as > "opt/org.tianocore/IPv6PXESupport" are conscious of the slot count in > QEMU. They *can* bump the "x-file-slots" property in QEMU, for new > machine types, they just need to be aware of the property. > > ) > >> + >> :: >> >> - <smbios type='fwcfg'> >> + <sysinfo type='fwcfg'> >> <entry name='opt/com.example/name'>example value</entry> > > I suggest (according to the above): > > - name: opt/org.tianocore/IPv4PXESupport > - value: n > >> - <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> >> - </smbios> >> + <entry name='opt/com.example/config' file='/tmp/provision.ign'/> > > We have a functional -- working, stable -- example for name+file as > well: > > - name: etc/edk2/https/cacerts > - file: /etc/pki/ca-trust/extracted/edk2/cacerts.bin > > (This is documented in "OvmfPkg/README" in edk2, but it's not really > relevant here.) > >> + </sysinfo> >> >> - The ``smbios`` element can have multiple ``entry`` child elements. Each >> + The ``sysinfo`` element can have multiple ``entry`` child elements. Each >> element then has mandatory ``name`` attribute, which defines the name of the >> blob and must begin with ``"opt/"`` and to avoid clashing with other names is >> advised to be in form ``"opt/$RFQDN/$name"`` where ``$RFQDN`` is a reverse >> > > It's hard to express this cleanly. > > - The opt/RFQDN notation is a mitigation for users that are hell-bent on > using fw-cfg files of their own purposes (not heeding our advice about > not using fw-cfg for such purposes at all). So the idea is, "if you > ignore our request, then (a) be prepared to run out of slots, and (b) > *at least* use a name pattern (opt/RFQDN) that minimizes conflicts > with other, similar-minded users / projects" > > - For "officially supported" knobs that the firmware looks at, it's fine > to use any names -- they avoid conflicts with the above "rogue" files. > Examples: > > - opt/ovmf/ -- reserved for historical reasons > > - opt/org.tianocore/ -- should never conflict due to RFQDN > > - etc/edk2/https/... -- should never conflict due to being outside of > opt/ > > So I guess the short rule is, "Feel free to refer to any fw_cfg file > name that your firmware officially supports. When defining other fw_cfg > file names (i.e., for your own purposes), then prepare for breakage in > the long-term, and then at least use the opt/RFQDN/ name pattern". I think in the light of what I wrote above this makes more sense, doesn't it? Michal
On 09/08/20 12:05, Michal Privoznik wrote: > On 9/8/20 11:02 AM, Laszlo Ersek wrote: >> On 09/07/20 15:48, Michal Privoznik wrote: >>> Even though this was brought up in upstream discussion [1] it >>> missed my patches: users should prefer <oemStrings/> over fwcfg. >>> The reason is that fwcfg is considered somewhat internal to QEMU >>> and it has limited number of slots and neither of these applies >>> to <oemStrings/>. >>> >>> While I'm at it, I'm fixing the example too (because it contains >>> incorrect element name) and clarifying sysfs/ exposure. >>> >>> 1: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html >>> >>> Reported-by: Laszlo Ersek <lersek@redhat.com> >>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >>> --- >>> docs/formatdomain.rst | 14 +++++++++----- >>> 1 file changed, 9 insertions(+), 5 deletions(-) >>> >>> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst >>> index 1979dfb8d3..821ffe8d60 100644 >>> --- a/docs/formatdomain.rst >>> +++ b/docs/formatdomain.rst >>> @@ -509,18 +509,22 @@ layout of sub-elements, with supported values of: >>> Some hypervisors provide unified way to tweak how firmware >>> configures itself, >>> or may contain tables to be installed for the guest OS, for >>> instance boot >>> order, ACPI, SMBIOS, etc. It even allows users to define their >>> own config >>> - blobs. In case of QEMU, these then appear under domain's sysfs, >>> under >>> + blobs. In case of QEMU, these then appear under domain's sysfs >>> (if the guest >>> + kernel has FW_CFG_SYSFS config option enabled), under >>> ``/sys/firmware/qemu_fw_cfg``. Note, that these values apply >>> regardless the >>> <smbios/> mode under <os/>. :since:`Since 6.5.0` >>> >>> + **Please note that because of limited number of data slots use of >>> fwcfg is >>> + strongly discouraged and <oemStrings/> should be used instead**. >> >> please replace: >> >> strongly discouraged >> >> with: >> >> strongly discouraged for configuring any guest-side component other >> than the firmware > > Actually, we have a check that forbids anything else than "opt/" prefix > and also "opt/ovmf/" and "opt/org.qemu/" are forbidden: > > https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_validate.c#L780 > > > Looks like "opt/org.tianocore" slipped through. I think the reasoning is > that this is too low level and allows changing values of knobs that are > exposed elsewhere (e.g. boot order). It really should be just to append > new values, not change existing ones. > >> >> ( >> >> Consider for example the following feature: >> >> https://bugzilla.tianocore.org/show_bug.cgi?id=2681 >> >> Namely, the following QEMU switches: >> >> -fw_cfg name=opt/org.tianocore/IPv4PXESupport,string=[yn] >> -fw_cfg name=opt/org.tianocore/IPv6PXESupport,string=[yn] >> >> alter the behavior of OVMF and ArmVirtQemu. These flags are meant to be >> stable. They do not need dedicated QEMU or libvirtd enablement. They >> influence firmware behavior. So <sysinfo type='fwcfg'> is perfectly fine >> (even ideal!) for tweaking them, through the domain XML. What's not fine >> is configuring any random guest payload via <sysinfo type='fwcfg'>. >> >> The point is that people who parse new fw_cfg files in edk2 such as >> "opt/org.tianocore/IPv6PXESupport" are conscious of the slot count in >> QEMU. They *can* bump the "x-file-slots" property in QEMU, for new >> machine types, they just need to be aware of the property. >> >> ) >> >>> + >>> :: >>> >>> - <smbios type='fwcfg'> >>> + <sysinfo type='fwcfg'> >>> <entry name='opt/com.example/name'>example value</entry> >> >> I suggest (according to the above): >> >> - name: opt/org.tianocore/IPv4PXESupport >> - value: n >> >>> - <entry name='opt/com.coreos/config' >>> file='/tmp/provision.ign'/> >>> - </smbios> >>> + <entry name='opt/com.example/config' >>> file='/tmp/provision.ign'/> >> >> We have a functional -- working, stable -- example for name+file as >> well: >> >> - name: etc/edk2/https/cacerts >> - file: /etc/pki/ca-trust/extracted/edk2/cacerts.bin >> >> (This is documented in "OvmfPkg/README" in edk2, but it's not really >> relevant here.) >> >>> + </sysinfo> >>> >>> - The ``smbios`` element can have multiple ``entry`` child >>> elements. Each >>> + The ``sysinfo`` element can have multiple ``entry`` child >>> elements. Each >>> element then has mandatory ``name`` attribute, which defines the >>> name of the >>> blob and must begin with ``"opt/"`` and to avoid clashing with >>> other names is >>> advised to be in form ``"opt/$RFQDN/$name"`` where ``$RFQDN`` is >>> a reverse >>> >> >> It's hard to express this cleanly. >> >> - The opt/RFQDN notation is a mitigation for users that are hell-bent on >> using fw-cfg files of their own purposes (not heeding our advice about >> not using fw-cfg for such purposes at all). So the idea is, "if you >> ignore our request, then (a) be prepared to run out of slots, and (b) >> *at least* use a name pattern (opt/RFQDN) that minimizes conflicts >> with other, similar-minded users / projects" >> >> - For "officially supported" knobs that the firmware looks at, it's fine >> to use any names -- they avoid conflicts with the above "rogue" files. >> Examples: >> >> - opt/ovmf/ -- reserved for historical reasons >> >> - opt/org.tianocore/ -- should never conflict due to RFQDN >> >> - etc/edk2/https/... -- should never conflict due to being outside of >> opt/ >> >> So I guess the short rule is, "Feel free to refer to any fw_cfg file >> name that your firmware officially supports. When defining other fw_cfg >> file names (i.e., for your own purposes), then prepare for breakage in >> the long-term, and then at least use the opt/RFQDN/ name pattern". > > I think in the light of what I wrote above this makes more sense, > doesn't it? Are you saying that the current (upstream) language applies only to the case when users define their own new fw_cfg file names? Hm. That would certainly be consistent with my initial confusion upon seeing the subject of this patch "Discourage users from using fwcfg" -- "why the blanket discouragement, since there are valid uses"? But this would explain it, yes. The docs say, Some hypervisors provide unified way to tweak how firmware configures itself, or may contain tables to be installed for the guest OS, for instance boot order, ACPI, SMBIOS, etc. It even allows users to define their own config blobs. In case of QEMU, these then appear [...] So I guess for me what's missing is a hard paragraph break just before "It even allows". That would clarify that the domain XML snippet belongs to "defin[ing] their own config blobs". With that in mind, let me re-review your patch... Yes, I think your patch is fine then. Please consider breaking the sentence "It even allows..." to a new paragraph. You could even keep the "com.coreos" example as-is. Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo
On 9/8/20 1:45 PM, Laszlo Ersek wrote: > Reviewed-by: Laszlo Ersek<lersek@redhat.com> Thank you, pushed. Michal
© 2016 - 2024 Red Hat, Inc.