| Message ID | 20230907171655.6996-1-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Change what --nodefaults does and a revert | expand |
On Thu, 7 Sep 2023 18:16:52 +0100, Alexandru Elisei wrote: > The first two patches revert "virtio-net: Don't print the compat warning > for the default device" because using --network mode=none disables the > device and lets the user know that it can use that to disable the default > virtio-net device. I don't think the changes are controversial. > > And the last patch is there to get the conversation going about changing > what --nodefaults does. Details in the patch. > > [...] Applied first two to kvmtool (master), thanks! [1/3] Revert "virtio-net: Don't print the compat warning for the default device" https://git.kernel.org/will/kvmtool/c/4498eb7400c6 [2/3] builtin-run: Document mode=none for -n/--network https://git.kernel.org/will/kvmtool/c/c7b7a542cdcd I'm also not sure about the final RFC patch: [3/3] builtin-run: Have --nodefaults disable the default virtio-net device so it would be great to hear if anybody else has an opinion on that. IIRC, we introduced this for some EFI work, so perhaps those folks might have an opinion? Cheers,
Hi Will, Sorry for the late reply, was on holiday. On Mon, Sep 18, 2023 at 12:05:14PM +0100, Will Deacon wrote: > On Thu, 7 Sep 2023 18:16:52 +0100, Alexandru Elisei wrote: > > The first two patches revert "virtio-net: Don't print the compat warning > > for the default device" because using --network mode=none disables the > > device and lets the user know that it can use that to disable the default > > virtio-net device. I don't think the changes are controversial. > > > > And the last patch is there to get the conversation going about changing > > what --nodefaults does. Details in the patch. > > > > [...] > > Applied first two to kvmtool (master), thanks! > > [1/3] Revert "virtio-net: Don't print the compat warning for the default device" > https://git.kernel.org/will/kvmtool/c/4498eb7400c6 > [2/3] builtin-run: Document mode=none for -n/--network > https://git.kernel.org/will/kvmtool/c/c7b7a542cdcd > > I'm also not sure about the final RFC patch: > > [3/3] builtin-run: Have --nodefaults disable the default virtio-net device > > so it would be great to hear if anybody else has an opinion on that. IIRC, > we introduced this for some EFI work, so perhaps those folks might have > an opinion? It was actually introduced to allow kvmtool to load a kvm-unit-tests test file using --kernel instead of --firmware [1]. The user can specify parameters for a test using two methods: using the kernel command line (for selecting a specific test from a test file with multiple tests, for example) and from an initrd (for environment variables, in a key=value format). --firmware cannot load an initrd, so the only option is to use --kernel. But kvmtool mangles the kernel command line by prepending several kernel options, which breaks kvm-unit-tests' command line parser. Until --defaults there was no way to disable this behaviour. To the point of running kvm-unit-tests, it makes no functional difference if a test is run with --nodefaults --network mode=none or with --nodefaults only. It's just that I think that --nodefaults disabling the default configuration options is slightly more intuitive, but I'm not sure the effort of changing the semantics is worth it (need to inspect all the code that sets the default configuration options, then possibly adding new kvmtool command line parameters to bring back those options if there's no other way of changing them - I suspect this can get rather tedious). [1] https://lore.kernel.org/all/20210923144505.60776-1-alexandru.elisei@arm.com/ Thanks, Alex > > Cheers, > -- > Will > > https://fixes.arm64.dev > https://next.arm64.dev > https://will.arm64.dev
On 11/10/2023 15:56, Alexandru Elisei wrote: > Hi Will, > > Sorry for the late reply, was on holiday. > > On Mon, Sep 18, 2023 at 12:05:14PM +0100, Will Deacon wrote: >> On Thu, 7 Sep 2023 18:16:52 +0100, Alexandru Elisei wrote: >>> The first two patches revert "virtio-net: Don't print the compat warning >>> for the default device" because using --network mode=none disables the >>> device and lets the user know that it can use that to disable the default >>> virtio-net device. I don't think the changes are controversial. >>> >>> And the last patch is there to get the conversation going about changing >>> what --nodefaults does. Details in the patch. >>> >>> [...] >> >> Applied first two to kvmtool (master), thanks! >> >> [1/3] Revert "virtio-net: Don't print the compat warning for the default device" >> https://git.kernel.org/will/kvmtool/c/4498eb7400c6 >> [2/3] builtin-run: Document mode=none for -n/--network >> https://git.kernel.org/will/kvmtool/c/c7b7a542cdcd >> >> I'm also not sure about the final RFC patch: >> >> [3/3] builtin-run: Have --nodefaults disable the default virtio-net device >> >> so it would be great to hear if anybody else has an opinion on that. IIRC, >> we introduced this for some EFI work, so perhaps those folks might have >> an opinion? > > It was actually introduced to allow kvmtool to load a kvm-unit-tests test > file using --kernel instead of --firmware [1]. > > The user can specify parameters for a test using two methods: using the > kernel command line (for selecting a specific test from a test file with > multiple tests, for example) and from an initrd (for environment variables, > in a key=value format). --firmware cannot load an initrd, so the only > option is to use --kernel. But kvmtool mangles the kernel command line by > prepending several kernel options, which breaks kvm-unit-tests' command > line parser. Until --defaults there was no way to disable this behaviour. > > To the point of running kvm-unit-tests, it makes no functional difference > if a test is run with --nodefaults --network mode=none or with --nodefaults > only. It's just that I think that --nodefaults disabling the default > configuration options is slightly more intuitive, but I'm not sure the > effort of changing the semantics is worth it (need to inspect all the code > that sets the default configuration options, then possibly adding new > kvmtool command line parameters to bring back those options if there's no > other way of changing them - I suspect this can get rather tedious). I personally prefer "--nodefaults" doing what it says. i.e., kvmtool not adding *anything* by default on its own. Suzuki > > [1] https://lore.kernel.org/all/20210923144505.60776-1-alexandru.elisei@arm.com/ > > Thanks, > Alex > >> >> Cheers, >> -- >> Will >> >> https://fixes.arm64.dev >> https://next.arm64.dev >> https://will.arm64.dev
The first two patches revert "virtio-net: Don't print the compat warning for the default device" because using --network mode=none disables the device and lets the user know that it can use that to disable the default virtio-net device. I don't think the changes are controversial. And the last patch is there to get the conversation going about changing what --nodefaults does. Details in the patch. Alexandru Elisei (3): Revert "virtio-net: Don't print the compat warning for the default device" builtin-run: Document mode=none for -n/--network builtin-run: Have --nodefaults disable the default virtio-net device builtin-run.c | 6 +++--- virtio/net.c | 11 ++++++----- 2 files changed, 9 insertions(+), 8 deletions(-)