kdevops.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
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

      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 NNTP newsgroup(s).