Description of problem: Right now ignition works in CNV via `-fw_cfg`. The same applies to RHCOS. We both append the argument directly to the qemu commandline. It would be great if we could get a supported interface in libvirt so that we can stop doing that, since passing anything directly to qemu is not supported from libvirts perspective. Version-Release number of selected component (if applicable): How reproducible: Use ignition in CNV. You will get domain xmls which contain snippets like this: <domain type='kvm' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'> <devices/> <qemu:commandline> <qemu:arg value='-fw_cfg'/> <qemu:arg value='name=opt/com.name.domain.your.example,string=hello'/> </qemu:commandline> </domain> Steps to Reproduce: 1. 2. 3. Actual results: This puts us in the unfortunate situation where the preferred way on how to boostrap openshift nodes via the openshift installer is not officially supported in CNV. Expected results: Provide an officially supported way to use ignition. Additional info:
I've started discussion on the list: https://www.redhat.com/archives/libvir-list/2020-May/msg00954.html
There is also bug created bug Daniel we deferred. See bug 1422831.
Setting sev to high as this is a commonly request ed feature However, to be fair, we also shoul dinvestigate if there are other ways to provide ignition configs to a VM - not using fw_Cfg
(In reply to Fabian Deutsch from comment #3) > However, to be fair, we also should investigate if there are other ways to > provide ignition configs to a VM - not using fw_Cfg There are OEM strings (which are already implemented) as mentioned by Dan on the list: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html They are used like this: <domain type='qemu'> <sysinfo type='smbios'> <oemStrings> <entry>Hello</entry> <entry>World</entry> <entry>This is, more tricky value=escaped</entry> </oemStrings> </sysinfo> <os> <smbios mode='sysinfo'/> </os> </domain> and this is how they appear in the guest: # dmidecode -u --oem-string count 3 # dmidecode -u --oem-string 1 Hello # dmidecode -u --oem-string 2 World # dmidecode -u --oem-string 3 This is, more tricky value=escaped I've discovered some sysfs attributes Per Dan suggestion it's probably better to put an URL into the string where the file can be downloaded from rather than the whole file itself.
First attempt here: https://www.redhat.com/archives/libvir-list/2020-June/msg00139.html
Second attempt here: https://www.redhat.com/archives/libvir-list/2020-June/msg00167.html
*** Bug 1422831 has been marked as a duplicate of this bug. ***
Third attempt: https://www.redhat.com/archives/libvir-list/2020-June/msg00382.html
I've just merged patches upstream: 2072c39541 news: Document -fw_cfg 14c32cd10f qemu: Generate command line for -fw_cfg d024a7da7a secdrivers: Relabel firmware config files 9ce32b0935 qemu: Introduce fw_cfg capability b5f8f04989 qemu: Validate firmware blob configuration 3dda889a44 conf: Add firmware blob configuration v6.4.0-77-g2072c39541
I've identified a memleak in the referenced commits. Patch posed here: https://www.redhat.com/archives/libvir-list/2020-June/msg00610.html
Pushed upstream as: d659cd341f virsysinfo: Don't leak fw_cfg v6.4.0-121-gd659cd341f
Test Version: libvirt-6.6.0-2.virtcov.el8.x86_64 qemu-kvm-5.0.0-2.module+el8.3.0+7379+0505d6ca.x86_64 SC1: [Positive test] Define and start guest with “fwcfg” type in sysinfo field 1. Prepare a /tmp/provision.ign file and write contents to it. # ll -Z /tmp/provision.ign -rw-r--r--. 1 root root unconfined_u:object_r:user_tmp_t:s0 22 Jul 21 22:24 /tmp/provision.ign # echo "This is a new feature" >/tmp/provision.ign 2. Prepare a guest xml with the following snippet, define and start the guest. … <sysinfo type='fwcfg'> <entry name='opt/com.example/name'>example value</entry> <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> </sysinfo> ... # virsh define lmn.xml Domain lmn defined from lmn.xml # virsh start lmn Domain lmn started 3. Check the dumpxml and qemu command and the seclabels of the provision.ing file. # virsh dumpxml lmn | grep fwcfg -A 3 <sysinfo type='fwcfg'> <entry name='opt/com.example/name'>example value</entry> <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> </sysinfo> # ps aux | grep lmn | grep fw_cfg … -fw_cfg name=opt/com.example/name,string=example value -fw_cfg name=opt/com.coreos/config,file=/tmp/provision.ign … # ll -Z /tmp/provision.ign -rw-r--r--. 1 qemu qemu system_u:object_r:virt_content_t:s0 22 Jul 21 22:24 /tmp/provision.ign 4. Check in the guest. # cat /sys/firmware/qemu_fw_cfg/by_name/opt/com.example/name/raw example value # cat /sys/firmware/qemu_fw_cfg/by_name/opt/com.coreos/config/raw This is a new feature SC2: [Negative test] Edit the guest with invalid options 1. Define the guest with missing name attribute: ...<entry>example value</entry>... # virsh define lmn.xml error: Failed to define domain from lmn.xml error: XML error: Firmware entry is missing 'name' attribute 2. Define the guest with missing value or file attribute: # virsh define lmn.xml ...<entry name='opt/com.example/name'/>... or ...<entry name='opt/com.coreos/config'/>... error: Failed to define domain from lmn.xml error: XML error: Firmware entry must have either value or 'file' attribute 3. Define the guest without prefix “/opt”: ...<entry name='tmp/com.example/name'>example value</entry>... # virsh define lmn.xml error: Failed to define domain from lmn.xml error: unsupported configuration: Invalid firmware name 4. Define the guest with an existing name. ...<entry name='opt/org.qemu/'>example value</entry>... or <entry name='opt/ovmf/'>example value</entry> # virsh define lmn.xml error: Failed to define domain from lmn.xml error: unsupported configuration: That firmware name is reserved
Move this bug to be verified according to comment 15
Hi Michal, I've found this RHBZ coming from another topic (but 1688978). Some observations and questions: (1) In the mailing list thread linked in comment 1, Daniel made the following remark: https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html > I think we should be documenting that its usage is strongly > discouraged though, in favour of OEM strings. (I'd like to thank Daniel for remembering this.) Can you please point out to me where this guidance was implemented in the libvirt documentation / patch series that was ultimately merged? Because what I can see now is the exact opposite of the above (valid) recommendation: (1a) There's a sentence in the docs, from commit 3dda889a4426 ("conf: Add firmware blob configuration", 2020-06-10), that goes > In case of QEMU, these then appear under domain's sysfs, under > /sys/firmware/qemu_fw_cfg. It advertises an abuse of the firmware config facility (namely, the Linux kernel's FW_CFG_SYSFS driver, which -- as a mitigating factor at least -- defaults to "n"). (1b) The same commit provides the following example as well, twice: <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> It's another abuse (with a widely known / highly visible project name). In reality it's something we should actively discourage (and not name as an example) -- in favor of the OEM strings SMBIOS table. (2) I think there's a typo (in the same commit) that persists after the RST conversion too (as of 9514e24984ee), namely: > <smbios type='fwcfg'> > <entry name='opt/com.example/name'>example value</entry> > <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> > </smbios> > The smbios element can have multiple entry child elements [...] Here the outer element's name should be "sysinfo", not "smbios" (three instances).
Further, from commit 2072c395419b ("news: Document -fw_cfg", 2020-06-10): > + * Allow firmware blobs configuration > + > + QEMU offers a way to tweak how firmware configures itself > + and/or provide new configuration blobs. [...] The first half is valid. The second half ("provide new configuration blobs"), which seems to be offered as something unrelated to firmware, should be removed, or redirected to the OEM Strings SMBIOS table. Should I report a new bug, for cleaning up the docs? Thanks.
(In reply to Laszlo Ersek from comment #17) > Hi Michal, > > I've found this RHBZ coming from another topic (but 1688978). Some > observations and questions: > > (1) In the mailing list thread linked in comment 1, Daniel made the > following remark: > > https://www.redhat.com/archives/libvir-list/2020-May/msg00957.html > > > I think we should be documenting that its usage is strongly > > discouraged though, in favour of OEM strings. > > (I'd like to thank Daniel for remembering this.) > > Can you please point out to me where this guidance was implemented in > the libvirt documentation / patch series that was ultimately merged? I don't think that we've documented it in the end. > > Because what I can see now is the exact opposite of the above (valid) > recommendation: > > (1a) There's a sentence in the docs, from commit 3dda889a4426 ("conf: > Add firmware blob configuration", 2020-06-10), that goes > > > In case of QEMU, these then appear under domain's sysfs, under > > /sys/firmware/qemu_fw_cfg. > > It advertises an abuse of the firmware config facility (namely, the > Linux kernel's FW_CFG_SYSFS driver, which -- as a mitigating factor at > least -- defaults to "n"). Okay, I can document that kernel needs to have FW_CFG_SYSFS enabled. > > (1b) The same commit provides the following example as well, twice: > > <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> > > It's another abuse (with a widely known / highly visible project name). Which is exactly why we added the support. If a widely known project has to use unsupported feature then libvirt should do something about it. > > In reality it's something we should actively discourage (and not name as > an example) -- in favor of the OEM strings SMBIOS table. I don't recall all the details (and I agree that we should be advocating for OEM strings), but there were some problems with OEM strings - something about passing binary data. > > > (2) I think there's a typo (in the same commit) that persists after the > RST conversion too (as of 9514e24984ee), namely: > > > <smbios type='fwcfg'> > > <entry name='opt/com.example/name'>example value</entry> > > <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> > > </smbios> > > > The smbios element can have multiple entry child elements [...] > > Here the outer element's name should be "sysinfo", not "smbios" (three > instances). Correct.
(In reply to Laszlo Ersek from comment #18) > > Should I report a new bug, for cleaning up the docs? > I don't think it's necessary. I will write the patches and when sending them I will CC you.
(In reply to Michal Privoznik from comment #19) > (In reply to Laszlo Ersek from comment #17) > > > > (1b) The same commit provides the following example as well, twice: > > > > <entry name='opt/com.coreos/config' file='/tmp/provision.ign'/> > > > > It's another abuse (with a widely known / highly visible project name). > > Which is exactly why we added the support. If a widely known project has to > use unsupported feature then libvirt should do something about it. > > > > > In reality it's something we should actively discourage (and not name as > > an example) -- in favor of the OEM strings SMBIOS table. > > I don't recall all the details (and I agree that we should be advocating for > OEM strings), but there were some problems with OEM strings - something > about passing binary data. > Ignition is switching to just use a block device, probably phasing out fw_cfg eventually https://github.com/coreos/ignition/issues/928#issuecomment-640695503
As promised in comment 20 I've sent the patch: https://www.redhat.com/archives/libvir-list/2020-September/msg00344.html
(In reply to Michal Privoznik from comment #22) > As promised in comment 20 I've sent the patch: > > https://www.redhat.com/archives/libvir-list/2020-September/msg00344.html Thank you!
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (virt:8.3 bug fix and enhancement update), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2020:5137