qemu: support vmOpts.qemu.extraArgs

[girtsf]

Add support for passing additional QEMU CLI arguments via template YAML.

• add ExtraArgs to limatype.QEMUOpts
• append extra args to the generated QEMU command line
• document usage in templates/default.yaml

[AkihiroSuda]

Any reason not to use existing env vars? (QEMU_SYSTEM_AARCH64, QEMU_SYSTEM_X86_64, etc.)

[girtsf]

Mostly because I missed the fact that it gets parsed for shell arguments. :)

Though looking at

lima/pkg/driver/qemu/qemu.go

Line 1101 in 24317e9

logrus.Warnf("Specifying args (%v) via $%s is supported only for debugging!", args, envK)

, looks like it would log a warning when args are present. And it would be nicer to have it in one place in template file.

Feel free to close the PR if you don't think it would be helpful to others.

[jandubois]

Any reason not to use existing env vars? (QEMU_SYSTEM_AARCH64, QEMU_SYSTEM_X86_64, etc.)

I think we used to say that this is just a debugging feature, but it seems like the fact that you can add additional arguments is not documented at all.

If this is supposed to be a regular (driver) feature, then adding it to the template makes sense.

[virtualdxs]

FWIW, I have a use case that this would be incredibly helpful for - I need to add a pflash device for OVMF vars, so that the variables don't get written to the esp of the guest. Right now, the only workaround I'm seeing is to write a wrapper script that I call via QEMU_SYSTEM_X86_64 and QEMU_SYSTEM_AARCH64, but that feels really hacky. Being able to append args via the YAML would eliminate the need for a wrapper script entirely.

[jandubois]

I noticed 2 things reviewing this PR:

1. (this is maybe outside the scope of of the PR) This adds yet another call to limayaml.Convert(). We should call it only once, and then use the returned structure instead of calling multiple times, potentially duplicating warning log entries. There is also some inconsistency if Convert() returning an error show result in a log warning, or error propagation.

2. (this requires some discussion): The extraArgs are appended at the end of the command line. They are not using the appendArgsIfNoConflict() mechanism that is used for other options. Maybe QEMU will always replace earlier values with later values. But will it generate warnings/errors? And we claim that e.g. QEMU_SYSTEM_AARCH64 takes precedence over other options. Should it take precedence over extraArgs as well. Right now the behaviour is a bit muddied, and also undocumented.

Please don't start changing the code for (2) until there has been a discussion and consensus!

[AkihiroSuda]

QEMU_SYSTEM_AARCH64 takes precedence over other options. Should it take precedence over extraArgs as well.

I expect the args to be appended in the following order (w/ appendIfNoConflict):

• QEMU_SYSTEM_${ARCH} env vars
• Internally generated flags
• extraArgs

[virtualdxs]

Re: appendArgsIfNoConflict(): How should it handle the disallowed args (e.g. -drive)? I'd suggest special-casing them and skipping the check rather than disallowing them, to allow for things like -drive if=pflash,format=raw,file=/path/to/edk2-i386-vars.fd to be injected via this mechanism.

[AkihiroSuda]

Re: appendArgsIfNoConflict(): How should it handle the disallowed args (e.g. -drive)? I'd suggest special-casing them and skipping the check rather than disallowing them, to allow for things like -drive if=pflash,format=raw,file=/path/to/edk2-i386-vars.fd to be injected via this mechanism.

For UEFI vars, I'd rather suggest supporting them as a proper feature without needing setting custom args.

[AkihiroSuda]

Looks like we already have the support for efi vars, but not always enabled yet

lima/pkg/driver/qemu/qemu.go

Line 618 in 20dd365

firmwareWithVars := filepath.Join(cfg.InstanceDir, filenames.QemuEfiFullFD)

e3fd241

[virtualdxs]

For UEFI vars, I'd rather suggest supporting them as a proper feature without needing setting custom args.

That would be nice, yes, but in the meantime it would be better to have any way to configure it. In addition, it would be nice to support unknown/unexpected use cases by minimizing restrictions.

[AkihiroSuda]

For UEFI vars, I'd rather suggest supporting them as a proper feature without needing setting custom args.

That would be nice, yes, but in the meantime it would be better to have any way to configure it. In addition, it would be nice to support unknown/unexpected use cases by minimizing restrictions.

Workaround: set QEMU_SYSTEM_${ARCH} to a wrapper script that executes qemu with -drive

[virtualdxs]

Yes, that's the workaround I'm implementing now; it's just very hacky when I'm doing automatically as part of a tool that wraps Lima. I'm struggling to understand, what's the downside of allowing arbitrary -drive arguments? This would allow for more use cases than just EFI vars.

[girtsf]

1. (this is maybe outside the scope of of the PR) This adds yet another call to limayaml.Convert(). We should call it only once, and then use the returned structure instead of calling multiple times, potentially duplicating warning log entries. There is also some inconsistency if Convert() returning an error show result in a log warning, or error propagation.

Maybe we do the parsing in Start and store the parsed result in Config?

lima/pkg/driver/qemu/qemu_driver.go

Line 254 in 20dd365

qCfg := Config{

2. (this requires some discussion): The extraArgs are appended at the end of the command line. They are not using the appendArgsIfNoConflict() mechanism that is used for other options. Maybe QEMU will always replace earlier values with later values. But will it generate warnings/errors? And we claim that e.g. QEMU_SYSTEM_AARCH64 takes precedence over other options. Should it take precedence over extraArgs as well. Right now the behaviour is a bit muddied, and also undocumented.

In my mind extraArgs are an advanced usage case and the user probably understands what is ending up on the qemu command line. In this case, the IfNoConflict part might get in the way.

Would it be useful to be more explicit in the naming that args are appended? appendExtraArgs?