From: Luis Chamberlain <mcgrof@kernel.org>
To: Ross Vandegrift <ross@kallisti.us>
Cc: kdevops@lists.linux.dev
Subject: Re: [PATCH v2 1/3] vagrant arm64: arch indep kconfig/ansible options
Date: Tue, 12 Sep 2023 23:07:36 -0700 [thread overview]
Message-ID: <ZQFRqBWxeONYrEhV@bombadil.infradead.org> (raw)
In-Reply-To: <20230913043512.6083-1-ross@kallisti.us>
On Tue, Sep 12, 2023 at 09:35:11PM -0700, Ross Vandegrift wrote:
> Vagrant arm64 will require a handful of new libvirt parameters. Most are
> architecture independent. But they're currently unnecessary because
> libvirt/qemu has defaults are tailored for x86-64.
>
> This adds support for the following, along with reasonable defaults for x86-64:
> - graphics type
> - mouse bus
> - uefi loader & nvram storage
>
> Signed-off-by: Ross Vandegrift <ross@kallisti.us>
> ---
> Thanks for the feedback on the previous version. I opted to keep the
> arch-independant options separate - since I'd already setup so much of it, this
> ended up being less work. As before, everything though make bringup works.
>
> ---
> playbooks/roles/gen_nodes/defaults/main.yml | 4 ++
> .../roles/gen_nodes/templates/Vagrantfile.j2 | 7 +++
> scripts/gen-nodes.Makefile | 15 +++++++
> vagrant/Kconfig | 43 +++++++++++++++++++
> 4 files changed, 69 insertions(+)
>
> diff --git a/playbooks/roles/gen_nodes/defaults/main.yml b/playbooks/roles/gen_nodes/defaults/main.yml
> index c57effe7..d44ba8bb 100644
> --- a/playbooks/roles/gen_nodes/defaults/main.yml
> +++ b/playbooks/roles/gen_nodes/defaults/main.yml
> @@ -60,6 +60,10 @@ libvirt_extra_storage_aio_cache_mode: "none"
> libvirt_extra_storage_nvme_logical_block_size: 512
> libvirt_extra_storage_virtio_logical_block_size: 512
> libvirt_extra_storage_virtio_physical_block_size: 512
> +libvirt_graphics_type: none
This alone should be its own patch as it is a functional change and
it makes the graphics type switch to none as defualt. That is welcomed.
> +libvirt_input_mouse_bus: ps2
> +libvirt_loader: ""
You want a default:
libvirt_loader_enable: False
> +libvirt_nvram_dir: /var/lib/libvirt/qemu/nvram
Then this can be a default Kconfig string. What you do then
is create a Kconfig choise and make the deafult for different
architectures.
But I'll explain more below.
>
> libvirt_largeio_enable: False
> libvirt_largeio_logical_compat: False
> diff --git a/playbooks/roles/gen_nodes/templates/Vagrantfile.j2 b/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
> index 7ed59ff7..0d20ed6d 100644
> --- a/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
> +++ b/playbooks/roles/gen_nodes/templates/Vagrantfile.j2
> @@ -229,6 +229,13 @@ Vagrant.configure("2") do |config|
> libvirt.machine_type = global_data['libvirt_cfg']['machine_type']
> end
> end
> + libvirt.graphics_type = '{{ libvirt_graphics_type }}'
> + libvirt.input :type => "mouse", :bus => "{{ libvirt_input_mouse_bus }}"
This seems fine.
> +{% if libvirt_loader %}
Here you'd use the libvirt_loader_enable instead of libvirt_loader so to
be very precise about a boolean.
> + libvirt.loader = "{{ libvirt_loader }}"
> + libvirt.nvram = "{{ libvirt_nvram_dir }}/%s.fd" % host_name
> +{% endif %}
> +
> # Add an extra spare PCI or PCI-E root bus to be used for extra drives.
> #
> # We use a dedicated PCI or PCI-E root bus to not clash with defaults
> diff --git a/scripts/gen-nodes.Makefile b/scripts/gen-nodes.Makefile
> index 533b19db..9c1f1e09 100644
> --- a/scripts/gen-nodes.Makefile
> +++ b/scripts/gen-nodes.Makefile
> @@ -203,4 +203,19 @@ endif # CONFIG_QEMU_ENABLE_CXL
>
> endif # CONFIG_LIBVIRT_MACHINE_TYPE_Q35
>
> +ifneq (,$(CONFIG_LIBVIRT_GRAPHICS_TYPE))
> +GEN_NODES_EXTRA_ARGS += libvirt_graphics_type='$(subst ",,$(CONFIG_LIBVIRT_GRAPHICS_TYPE))'
> +endif
> +
> +ifeq (y,$(CONFIG_LIBVIRT_INPUT_PS2_MOUSE))
> +GEN_NODES_EXTRA_ARGS += libvirt_input_mouse_bus='ps2'
> +else ifeq (y, $(CONFIG_LIBVIRT_INPUT_VIRTIO_MOUSE))
> +GEN_NODES_EXTRA_ARGS += libvirt_input_mouse_bus='virtio'
> +endif
> +
> +ifneq (,$(CONFIG_LIBVIRT_LOADER))
> +GEN_NODES_EXTRA_ARGS += libvirt_loader='$(subst ",,$(CONFIG_LIBVIRT_LOADER))'
> +GEN_NODES_EXTRA_ARGS += libvirt_nvram_dir='$(subst ",,$(CONFIG_LIBVIRT_NVRAM_DIR))'
> +endif
You replace most of this with just Kconfig language, the only thing
you'd need is a conversion to True for the libvirt_loader_enable.
Then in the future, once we get support to make kconfig spit out
extra_vars.yaml for us from .config input source we can remove those
Makefile hacks. This let's us scale better.
Luis
prev parent reply other threads:[~2023-09-13 6:07 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-13 4:35 [PATCH v2 1/3] vagrant arm64: arch indep kconfig/ansible options Ross Vandegrift
2023-09-13 4:35 ` [PATCH v2 2/3] vagrant arm64: kconfig/ansible to enable arm64 Ross Vandegrift
2023-09-13 6:36 ` Luis Chamberlain
2023-09-13 4:35 ` [PATCH v2 3/3] vagrant arm64: add opensuse arm64 images Ross Vandegrift
2023-09-13 6:45 ` Luis Chamberlain
2023-09-13 6:07 ` Luis Chamberlain [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZQFRqBWxeONYrEhV@bombadil.infradead.org \
--to=mcgrof@kernel.org \
--cc=kdevops@lists.linux.dev \
--cc=ross@kallisti.us \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).