opnsense / tools

OPNsense release engineering toolkit
https://opnsense.org/
BSD 2-Clause "Simplified" License
260 stars 187 forks source link

Enable secondary serial console for VM images #367

Closed maurice-w closed 11 months ago

maurice-w commented 11 months ago

Allow initial configuration / interface assignment of VMs in virtualisation environments without a video console.

fichtner commented 11 months ago

I'm a bit in doubt we should do this by default or if this is to be done if the order should be reversed like we have in nano. CC @AdSchellevis

fichtner commented 11 months ago

More context in 5f64cb5 -- the fourth parameter to "vm" script is the configuration being used, also see here:

https://github.com/opnsense/tools/blob/e539e5891a5a3331f661ff951e987f9536d58d3d/composite/factory.sh#L51

AdSchellevis commented 11 months ago

this will likely break ours, so I rather keep it as is for now.

maurice-w commented 11 months ago

@AdSchellevis Understood, I don't want to break anything of course.

@fichtner I did indeed consider using the (well-documented 😉) fourth parameter. But serial_hook is missing touch ${1}/.probe.for.growfs and nano_hook also changes some other settings, which is undesirable (RAM disk for logs and /tmp, disabled RRD).

So would it be preferable to create extra hooks which then could be used as the fourth parameter for VM builds? Like vmserial_hook, vmvideoserial_hook, vmefiserial_hook, ...

fichtner commented 11 months ago

(well-documented 😉)

fair point ;)

We have a few ".local" workarounds. I suppose we could add one for extras.conf.local as well ?

maurice-w commented 11 months ago

To be frank, if no-one else needs this, you don't have to create a workaround just for me. I'll just keep using my own fork of tools then, no worries. Just thought it might be useful for others, but if it isn't, that's perfectly okay!

fichtner commented 11 months ago

Actually there is an extras.conf device hook handler which you could use. And I've added the documentation 01afc21cfa2cb ;)

maurice-w commented 11 months ago

@fichtner, I guess we mean the same thing: Add additional hooks to extras.conf and call these with the fourth parameter.

make vm-qcow2,16G,off,vmserial

Correct? Would you consider merging such additional hooks or should I just do it in my own fork?

fichtner commented 11 months ago

Can you take a look at DEVICE=A10 usage and the respective device/A10.conf ? I think it offer alls the overrides you need, just add a nice device based on the A10 and add your custom hook to access it from the vm script (you can also use the default "vm" designation here.

maurice-w commented 11 months ago

Ah, got it. Hooks in the the device configs augment those in extras.conf? So I'd create a custom device like device/SERIALVM.conf, add my custom vm_hook code there (without the part that's already in extras.conf) and then call:

make vm-qcow2,16G,off DEVICE=SERIALVM

Correct?

fichtner commented 11 months ago

Yes. Device hook is called after generic hook.

maurice-w commented 11 months ago

I ended up creating two devices, AMD64VM.conf (based on A10.conf) and ARM64VM.conf (based on ARM64.conf). They have additional hooks for serial and EFI consoles, which can be invoked by using the fourth parameter when building VM images:

make vm-qcow2,16G,off,serial DEVICE=AMD64VM   # amd64 VM with serial console
make vm-qcow2,16G,off,efi DEVICE=AMD64VM      # amd64 VM with EFI console
make vm-qcow2,16G,off,serial DEVICE=ARM64VM   # aarch64 VM with serial console
make vm-qcow2,16G,off,efi DEVICE=ARM64VM      # aarch64 VM with EFI console

Let me know if you think this is worth another PR. Closing here, thanks for your help!

maurice-w@31d7890 maurice-w@51facab

fichtner commented 11 months ago

@maurice-w too specific for a PR but good we have this "on paper" for others to find later on 👍

maurice-w commented 11 months ago

Problem: The unset PRODUCT_DEVICE line in device files based on A10.conf results in the device hooks getting ignored.

Likely cause: _setup_extras_device() uses PRODUCT_DEVICE to locate the device file: https://github.com/opnsense/tools/blob/f213553e74cbb2a063d4e54d799bc6bda61c7ab2/build/common.sh#L1086-L1090

Unlike when loading the device-specific environment, where PRODUCT_DEVICE_REAL is used: https://github.com/opnsense/tools/blob/f213553e74cbb2a063d4e54d799bc6bda61c7ab2/build/common.sh#L243-L244

Bug or feature? You could remove the unset PRODUCT_DEVICE line from the device file, but this has at least one significant side effect: If you did prefetch the kernel (make prefetch-kernel), this kernel set won't get used because PRODUCT_DEVICE is expected to be in the file name: https://github.com/opnsense/tools/blob/f213553e74cbb2a063d4e54d799bc6bda61c7ab2/build/common.sh#L759

fichtner commented 11 months ago

That’s a bug indeed.

maurice-w commented 11 months ago

Thanks for confirming! So the fix would be using PRODUCT_DEVICE_REAL in _setup_extras_device()? Should I make a PR for this?

fichtner commented 11 months ago

Yep, or repurpose this one. I don’t mind.