All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Enable aarch64 with guestfs
@ 2024-03-05 16:36 Chuck Lever
  2024-03-05 16:36 ` [PATCH 1/4] guestfs: Specify host architecture to virt-builder Chuck Lever
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Chuck Lever @ 2024-03-05 16:36 UTC (permalink / raw)
  To: kdevops

Hi-

These patches add aarch64 platform support when running kdevops
with guestfs. I decided not to add aarch64 support for vagrant
since vagrant seems to be going away soon.

I've tested only on Fedora 39 (with Fedora 38 guests), but it
seems robust enough for broader testing and review.

---

Chuck Lever (4):
      guestfs: Specify host architecture to virt-builder
      guestfs: Enable destruction of guests with NVRAM
      gen_nodes: Instructions for adding a new guestfs architecture
      libvirt: Support aarch64 guests


 kconfigs/Kconfig.libvirt                      |   8 +-
 playbooks/roles/gen_nodes/templates/README.md |  65 ++++++
 .../gen_nodes/templates/guestfs_virt.j2.xml   | 215 ++++++++++++++++++
 scripts/bringup_guestfs.sh                    |   2 +-
 scripts/destroy_guestfs.sh                    |   2 +-
 scripts/gen-nodes.Makefile                    |   5 +
 workflows/linux/Kconfig                       |   3 +-
 7 files changed, 295 insertions(+), 5 deletions(-)
 create mode 100644 playbooks/roles/gen_nodes/templates/README.md
 create mode 100644 playbooks/roles/gen_nodes/templates/guestfs_virt.j2.xml

--
Chuck Lever


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/4] guestfs: Specify host architecture to virt-builder
  2024-03-05 16:36 [PATCH 0/4] Enable aarch64 with guestfs Chuck Lever
@ 2024-03-05 16:36 ` Chuck Lever
  2024-03-05 20:11   ` Luis Chamberlain
  2024-03-05 16:36 ` [PATCH 2/4] guestfs: Enable destruction of guests with NVRAM Chuck Lever
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2024-03-05 16:36 UTC (permalink / raw)
  To: kdevops

From: Chuck Lever <chuck.lever@oracle.com>

According to documentation, the default architecture for
virt-builder isn't the same as the host's platform architecture.
To support platforms other than x86, specify the host's
architecture explicitly on the command line.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 scripts/bringup_guestfs.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
index 34ad48cbe81f..51b5a07218ac 100755
--- a/scripts/bringup_guestfs.sh
+++ b/scripts/bringup_guestfs.sh
@@ -67,7 +67,7 @@ _EOT
 	fi
 
 	echo "Generating new base image for ${OS_VERSION}"
-	virt-builder ${OS_VERSION} -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
+	virt-builder ${OS_VERSION} --arch `uname -m` -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
 fi
 
 # FIXME: is there a yaml equivalent of jq?



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/4] guestfs: Enable destruction of guests with NVRAM
  2024-03-05 16:36 [PATCH 0/4] Enable aarch64 with guestfs Chuck Lever
  2024-03-05 16:36 ` [PATCH 1/4] guestfs: Specify host architecture to virt-builder Chuck Lever
@ 2024-03-05 16:36 ` Chuck Lever
  2024-03-05 20:23   ` Luis Chamberlain
  2024-03-05 16:36 ` [PATCH 3/4] gen_nodes: Instructions for adding a new guestfs architecture Chuck Lever
  2024-03-05 16:36 ` [PATCH 4/4] libvirt: Support aarch64 guests Chuck Lever
  3 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2024-03-05 16:36 UTC (permalink / raw)
  To: kdevops

From: Chuck Lever <chuck.lever@oracle.com>

The default guest configuration on ARM systems includes a virtual
NVRAM device. However, "virsh undefine" won't touch such guests
without an explicit command-line option that enables destruction of
that device and its contents.

Note that virsh(1) says:

> --nvram and --keep-nvram specify accordingly to delete or keep
> nvram (/domain/os/nvram/) file.

Although adding this option to "virsh undefine" prevents an error
and allows the "undefine" to continue to completion, it does not
seem to actually delete the virtual NVRAM device's backing file on
my system.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 scripts/destroy_guestfs.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/destroy_guestfs.sh b/scripts/destroy_guestfs.sh
index 125890dc34dc..9c627f231cc7 100755
--- a/scripts/destroy_guestfs.sh
+++ b/scripts/destroy_guestfs.sh
@@ -19,7 +19,7 @@ if [ -f "$GUESTFSDIR/kdevops_nodes.yaml" ]; then
 			if [ "$domstate" = 'running' ]; then
 				virsh destroy $name
 			fi
-			virsh undefine $name
+			virsh undefine --nvram $name
 		fi
 		rm -rf "$GUESTFSDIR/$name"
 		rm -rf "$STORAGEDIR/$name"



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/4] gen_nodes: Instructions for adding a new guestfs architecture
  2024-03-05 16:36 [PATCH 0/4] Enable aarch64 with guestfs Chuck Lever
  2024-03-05 16:36 ` [PATCH 1/4] guestfs: Specify host architecture to virt-builder Chuck Lever
  2024-03-05 16:36 ` [PATCH 2/4] guestfs: Enable destruction of guests with NVRAM Chuck Lever
@ 2024-03-05 16:36 ` Chuck Lever
  2024-03-05 16:36 ` [PATCH 4/4] libvirt: Support aarch64 guests Chuck Lever
  3 siblings, 0 replies; 14+ messages in thread
From: Chuck Lever @ 2024-03-05 16:36 UTC (permalink / raw)
  To: kdevops

From: Chuck Lever <chuck.lever@oracle.com>

Write down what I did to build a new guestfs_splat.j2.xml file.
These notes guide the addition of support for a new guest type.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 playbooks/roles/gen_nodes/templates/README.md |   65 +++++++++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 playbooks/roles/gen_nodes/templates/README.md

diff --git a/playbooks/roles/gen_nodes/templates/README.md b/playbooks/roles/gen_nodes/templates/README.md
new file mode 100644
index 000000000000..67c91368654d
--- /dev/null
+++ b/playbooks/roles/gen_nodes/templates/README.md
@@ -0,0 +1,65 @@
+Constructing Node XML Files
+===========================
+
+Here are some basic recipes for constructing a guestfs_nnnn.j2.xml
+file. This will be necessary only when bringing up a previously
+unsupported guest ISA for use as a target guest.
+
+There are already a few guestfs_nnnn.j2.xml files in this directory
+to review for guidance.
+
+Requirements
+------------
+
+These recipes assume you have already installed the virt-* tools
+on your host.
+
+Build a virtual machine image
+-----------------------------
+
+Use virt-builder to download an build a sample disk image for the
+new guest. The following example builds a guest image with the same
+ISA as the host.
+
+  $ virt-builder fedora-38 --arch `uname -m` --size 20G --format raw
+
+Provision a virtual machine
+---------------------------
+
+Use virt-install to start up a guest on the disk image you built.
+
+  $ virt-install --disk path=./fedora-38.img --osinfo detect=on,require=off \
+        --install no_install=yes --memory=8000
+
+Extract node XML
+----------------
+
+Extract the guest's machine description into a file.
+
+  $ virsh dumpxml xxx > guestfs_nnnn.xml
+  $ virsh destroy xxx
+
+
+Hand-edit XML
+-------------
+
+kdevops wants a jinja2 file that can be used to substitute configured
+values into the XML. So:
+
+  $ cp guestfs_nnnn.xml guestfs_nnnn.j2.xml
+  $ edit guestfs_q35.j2.xml guestfs_nnnn.j2.xml
+
+Find instances of "{{" and copy those lines, as appropriate, to the
+new XML file.
+
+Test the new file with "make && make bringup". Adjust the .j2.xml
+file as needed.
+
+When you are satisfied with guestfs_nnnn.j2.xml, delete guestfs_nnnn.xml,
+then commit the guestfs_nnnn.j2.xml file to the kdevops repo.
+
+
+License
+-------
+
+copyleft-next-0.3.1



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/4] libvirt: Support aarch64 guests
  2024-03-05 16:36 [PATCH 0/4] Enable aarch64 with guestfs Chuck Lever
                   ` (2 preceding siblings ...)
  2024-03-05 16:36 ` [PATCH 3/4] gen_nodes: Instructions for adding a new guestfs architecture Chuck Lever
@ 2024-03-05 16:36 ` Chuck Lever
  2024-03-05 20:40   ` Luis Chamberlain
  3 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2024-03-05 16:36 UTC (permalink / raw)
  To: kdevops

From: Chuck Lever <chuck.lever@oracle.com>

I've added aarch64 support only under guestfs, since I figure
Vagrant support in kdevops is not long for this world.

- Add a .j2.xml file for building aarch64 nodes
- Remove some hard dependencies on the q35 guest definition

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 kconfigs/Kconfig.libvirt                           |    8 +
 .../roles/gen_nodes/templates/guestfs_virt.j2.xml  |  215 ++++++++++++++++++++
 scripts/gen-nodes.Makefile                         |    5 
 workflows/linux/Kconfig                            |    3 
 4 files changed, 228 insertions(+), 3 deletions(-)
 create mode 100644 playbooks/roles/gen_nodes/templates/guestfs_virt.j2.xml

diff --git a/kconfigs/Kconfig.libvirt b/kconfigs/Kconfig.libvirt
index fa39120450fd..6d51a1c26604 100644
--- a/kconfigs/Kconfig.libvirt
+++ b/kconfigs/Kconfig.libvirt
@@ -466,7 +466,8 @@ endif # HAVE_LIBVIRT_PCIE_PASSTHROUGH
 
 choice
 	prompt "Machine type to use"
-	default LIBVIRT_MACHINE_TYPE_Q35
+	default LIBVIRT_MACHINE_TYPE_Q35 if TARGET_ARCH_X86_64
+	default LIBVIRT_MACHINE_TYPE_VIRT if TARGET_ARCH_ARM64
 
 config LIBVIRT_MACHINE_TYPE_DEFAULT
 	bool "Use the default machine type"
@@ -487,6 +488,11 @@ config LIBVIRT_MACHINE_TYPE_Q35
 	  Use q35 for the machine type. This will be required for things like
 	  CXL or PCIe passthrough.
 
+config LIBVIRT_MACHINE_TYPE_VIRT
+	bool "virt"
+	help
+	  Use virt for the machine type. This is the default on aarch64 hosts.
+
 endchoice
 
 config LIBVIRT_HOST_PASSTHROUGH
diff --git a/playbooks/roles/gen_nodes/templates/guestfs_virt.j2.xml b/playbooks/roles/gen_nodes/templates/guestfs_virt.j2.xml
new file mode 100644
index 000000000000..9a7f004dcc1c
--- /dev/null
+++ b/playbooks/roles/gen_nodes/templates/guestfs_virt.j2.xml
@@ -0,0 +1,215 @@
+<domain type='kvm' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
+  <name>{{ hostname }}</name>
+  <memory unit='MiB'>{{ libvirt_mem_mb }}</memory>
+  <currentMemory unit='MiB'>{{ libvirt_mem_mb }}</currentMemory>
+  <vcpu placement='static'>{{ libvirt_vcpus_count }}</vcpu>
+  <os firmware='efi'>
+    <type arch='aarch64' machine='virt-8.1'>hvm</type>
+    <firmware>
+      <feature enabled='no' name='enrolled-keys'/>
+      <feature enabled='no' name='secure-boot'/>
+    </firmware>
+    <loader readonly='yes' type='pflash' format='qcow2'>/usr/share/edk2/aarch64/QEMU_EFI-silent-pflash.qcow2</loader>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <gic version='3'/>
+  </features>
+  <cpu mode='{{ 'host-passthrough' if libvirt_host_passthrough else 'host-model' }}' migratable='off'/>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>{{ qemu_bin_path }}</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='raw'/>
+      <source file='{{ kdevops_storage_pool_path }}/guestfs/{{ hostname }}/root.raw' index='1'/>
+      <backingStore/>
+      <target dev='vda' bus='virtio'/>
+      <alias name='virtio-disk0'/>
+      <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/>
+    </disk>
+    <controller type='usb' index='0' model='qemu-xhci' ports='15'>
+      <alias name='usb'/>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/>
+    </controller>
+    <controller type='pci' index='0' model='pcie-root'>
+      <alias name='pcie.0'/>
+    </controller>
+    <controller type='pci' index='1' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='1' port='0x8'/>
+      <alias name='pci.1'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/>
+    </controller>
+    <controller type='pci' index='2' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='2' port='0x9'/>
+      <alias name='pci.2'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='3' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='3' port='0xa'/>
+      <alias name='pci.3'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='4' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='4' port='0xb'/>
+      <alias name='pci.4'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x3'/>
+    </controller>
+    <controller type='pci' index='5' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='5' port='0xc'/>
+      <alias name='pci.5'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x4'/>
+    </controller>
+    <controller type='pci' index='6' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='6' port='0xd'/>
+      <alias name='pci.6'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x5'/>
+    </controller>
+    <controller type='pci' index='7' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='7' port='0xe'/>
+      <alias name='pci.7'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x6'/>
+    </controller>
+    <controller type='pci' index='8' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='8' port='0xf'/>
+      <alias name='pci.8'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x7'/>
+    </controller>
+    <controller type='pci' index='9' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='9' port='0x10'/>
+      <alias name='pci.9'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0' multifunction='on'/>
+    </controller>
+    <controller type='pci' index='10' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='10' port='0x11'/>
+      <alias name='pci.10'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/>
+    </controller>
+    <controller type='pci' index='11' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='11' port='0x12'/>
+      <alias name='pci.11'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x2'/>
+    </controller>
+    <controller type='pci' index='12' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='12' port='0x13'/>
+      <alias name='pci.12'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x3'/>
+    </controller>
+    <controller type='pci' index='13' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='13' port='0x14'/>
+      <alias name='pci.13'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x4'/>
+    </controller>
+    <controller type='pci' index='14' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='14' port='0x15'/>
+      <alias name='pci.14'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x5'/>
+    </controller>
+    <controller type='virtio-serial' index='0'>
+      <alias name='virtio-serial0'/>
+      <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/>
+    </controller>
+    <interface type='bridge'>
+      <source bridge='{{ libvirt_session_public_network_dev }}'/>
+      <target dev='tap0'/>
+      <model type='virtio'/>
+      <alias name='net0'/>
+      <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
+    </interface>
+    <serial type='pty'>
+      <source path='/dev/pts/2'/>
+      <target type='system-serial' port='0'>
+        <model name='pl011'/>
+      </target>
+      <alias name='serial0'/>
+      <log file='{{ guestfs_path }}/{{ hostname }}/console.log' append='on'/>
+    </serial>
+    <console type='pty' tty='/dev/pts/2'>
+      <source path='/dev/pts/1'/>
+      <target type='serial' port='0'/>
+      <alias name='serial0'/>
+    </console>
+    <channel type='unix'>
+      <source mode='bind'/>
+      <target type='virtio' name='org.qemu.guest_agent.0' state='connected'/>
+      <alias name='channel0'/>
+      <address type='virtio-serial' controller='0' bus='0' port='1'/>
+    </channel>
+    <tpm model='tpm-tis'>
+      <backend type='emulator' version='2.0'/>
+      <alias name='tpm0'/>
+    </tpm>
+    <audio id='1' type='none'/>
+    <memballoon model='virtio'>
+      <alias name='balloon0'/>
+      <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/>
+    </memballoon>
+    <rng model='virtio'>
+      <backend model='random'>/dev/urandom</backend>
+      <alias name='rng0'/>
+      <address type='pci' domain='0x0000' bus='0x06' slot='0x00' function='0x0'/>
+    </rng>
+  </devices>
+  <qemu:commandline>
+    <qemu:arg value='-global'/>
+    <qemu:arg value='ICH9-LPC.disable_s3=0'/>
+    <qemu:arg value='-global'/>
+    <qemu:arg value='ICH9-LPC.disable_s4=0'/>
+    <qemu:arg value='-device'/>
+    <qemu:arg value='pxb-pcie,id=pcie.1,bus_nr=32,bus=pcie.0,addr=0x8'/>
+{% if libvirt_extra_storage_drive_ide %}
+{% for n in range(0,4) %}
+    <qemu:arg value='-drive'/>
+    <qemu:arg value='file={{ kdevops_storage_pool_path }}/guestfs/{{ hostname }}/extra{{ n }}.{{ libvirt_extra_drive_format }},format={{ libvirt_extra_drive_format }},aio={{ libvirt_extra_storage_aio_mode }},cache={{ libvirt_extra_storage_aio_cache_mode }},if=none,id=drv{{ n }}'/>
+    <qemu:arg value='-device'/>
+    <qemu:arg value="ide-hd,drive=drv{{ n }},bus=ide.{{ n }},serial=kdevops{{ n }}"/>
+{% endfor %}
+{% elif libvirt_extra_storage_drive_virtio %}
+{% for n in range(0,4) %}
+    <qemu:arg value='-device'/>
+    <qemu:arg value='pcie-root-port,id=pcie-port-for-virtio-{{ n }},multifunction=on,bus=pcie.1,addr=0x{{ n }},chassis=5{{ n }}'/>
+    <qemu:arg value="-object"/>
+    <qemu:arg value="iothread,id=kdevops-virtio-iothread-{{ n }}"/>
+    <qemu:arg value="-drive"/>
+    <qemu:arg value="file={{ kdevops_storage_pool_path }}/guestfs/{{ hostname }}/extra{{ n }}.{{ libvirt_extra_drive_format }},format={{ libvirt_extra_drive_format }},if=none,aio={{ libvirt_extra_storage_aio_mode }},cache={{ libvirt_extra_storage_aio_cache_mode }},id=drv{{ n }}"/>
+    <qemu:arg value="-device"/>
+    <qemu:arg value="virtio-blk-pci,scsi=off,drive=drv{{ n }},id=virtio-drv{{ n }},serial=kdevops{{ n }},bus=pcie-port-for-virtio-{{ n }},addr=0x0,iothread=kdevops-virtio-iothread-{{ n }},logical_block_size={{ libvirt_extra_storage_virtio_logical_block_size }},physical_block_size={{ libvirt_extra_storage_virtio_physical_block_size }}"/>
+{% endfor %}
+{% elif libvirt_extra_storage_drive_nvme  %}
+{% for n in range(0,4) %}
+    <qemu:arg value='-device'/>
+    <qemu:arg value='pcie-root-port,id=pcie-port-for-nvme-{{ n }},multifunction=on,bus=pcie.1,addr=0x{{ n }},chassis=5{{ n }}'/>
+    <qemu:arg value='-drive'/>
+    <qemu:arg value='file={{ kdevops_storage_pool_path }}/guestfs/{{ hostname }}/extra{{ n }}.{{ libvirt_extra_drive_format }},format={{ libvirt_extra_drive_format }},if=none,id=drv{{ n }}'/>
+    <qemu:arg value='-device'/>
+    <qemu:arg value='nvme,id=nvme{{ n }},serial=kdevops{{ n }},bus=pcie-port-for-nvme-{{ n }},addr=0x0'/>
+    <qemu:arg value='-device'/>
+    <qemu:arg value='nvme-ns,drive=drv{{ n }},bus=nvme{{ n }},nsid=1,logical_block_size=512,physical_block_size=512'/>
+{% endfor %}
+{% endif %}
+{% if bootlinux_9p %}
+    <qemu:arg value='-device'/>
+    <qemu:arg value='{{ bootlinux_9p_driver }},fsdev={{ bootlinux_9p_fsdev }},mount_tag={{ bootlinux_9p_mount_tag }},bus=pcie.0,addr=0x10'/>
+    <qemu:arg value='-fsdev'/>
+    <qemu:arg value='local,id={{ bootlinux_9p_fsdev }},path={{ bootlinux_9p_host_path }},security_model={{ bootlinux_9p_security_model }}'/>
+{% endif %}
+  </qemu:commandline>
+</domain>
+
diff --git a/scripts/gen-nodes.Makefile b/scripts/gen-nodes.Makefile
index 657f64496309..ce6b794f1fb1 100644
--- a/scripts/gen-nodes.Makefile
+++ b/scripts/gen-nodes.Makefile
@@ -216,4 +216,9 @@ endif # CONFIG_QEMU_ENABLE_CXL
 
 endif # CONFIG_LIBVIRT_MACHINE_TYPE_Q35
 
+ifeq (y,$(CONFIG_LIBVIRT_MACHINE_TYPE_VIRT))
+GEN_NODES_EXTRA_ARGS += libvirt_override_machine_type='True'
+GEN_NODES_EXTRA_ARGS += libvirt_machine_type='virt'
+endif # CONFIG_LIBVIRT_MACHINE_TYPE_VIRT
+
 ANSIBLE_EXTRA_ARGS += $(GEN_NODES_EXTRA_ARGS)
diff --git a/workflows/linux/Kconfig b/workflows/linux/Kconfig
index d4dd3abe953b..41ca740dcca0 100644
--- a/workflows/linux/Kconfig
+++ b/workflows/linux/Kconfig
@@ -29,8 +29,7 @@ endif # HAVE_SUPPORTS_PURE_IOMAP
 config BOOTLINUX_9P
 	bool "Use 9p to build Linux"
 	depends on LIBVIRT
-	depends on LIBVIRT_MACHINE_TYPE_Q35
-	default LIBVIRT_MACHINE_TYPE_Q35
+	default LIBVIRT
 	help
 	  This will let you choose use 9p to build Linux. What this does is
 	  use your localhost to git clone Linux under the assumption your



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] guestfs: Specify host architecture to virt-builder
  2024-03-05 16:36 ` [PATCH 1/4] guestfs: Specify host architecture to virt-builder Chuck Lever
@ 2024-03-05 20:11   ` Luis Chamberlain
  2024-03-05 20:22     ` Chuck Lever
  0 siblings, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2024-03-05 20:11 UTC (permalink / raw)
  To: Chuck Lever; +Cc: kdevops

On Tue, Mar 05, 2024 at 11:36:20AM -0500, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> According to documentation, the default architecture for
> virt-builder isn't the same as the host's platform architecture.
> To support platforms other than x86, specify the host's
> architecture explicitly on the command line.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  scripts/bringup_guestfs.sh |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
> index 34ad48cbe81f..51b5a07218ac 100755
> --- a/scripts/bringup_guestfs.sh
> +++ b/scripts/bringup_guestfs.sh
> @@ -67,7 +67,7 @@ _EOT
>  	fi
>  
>  	echo "Generating new base image for ${OS_VERSION}"
> -	virt-builder ${OS_VERSION} -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
> +	virt-builder ${OS_VERSION} --arch `uname -m` -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile

Our current kconfig allows target architecture to be different, the
above seems to assume it matches the host arch so there is that
disrepancy in this patch. It would break the flexibility we are
providing.

Although your commit 44f7e0515dac3c0e ("Kconfig: Remove unused Kconfig
option") removed TARGET_ARCH, here is an example where that could have
been used.

config TARGET_ARCH
	string
	default "x86_64" if TARGET_ARCH_X86_64
	default "ppc64le" if TARGET_ARCH_PPC64LE

Then the above would use ${CONFIG_TARGET_ARCH}

A futher consideration is to add a new kconfig option which is disabled
by default (the default for all kconfig options are to be disabled by
default) which would *not* make it possible to override the target arch,
only if that bool would be enabled  would we allow the user to override
it. We want to discourage this as the performance would suck and the
results may vary as well.

Also, since we're in hopes of making libguest scalable and nicer than
vagrant, I look at bringup_guestfs.sh and start thinking a lot of that
stuff might be nicer in a playbook. Not a requirement now, but long term
I think we should consider striving for it.

There's a libvirt ansible module too:

https://docs.ansible.com/ansible/latest/collections/community/libvirt/virt_module.html

  Luis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] guestfs: Specify host architecture to virt-builder
  2024-03-05 20:11   ` Luis Chamberlain
@ 2024-03-05 20:22     ` Chuck Lever
  2024-03-05 20:29       ` Luis Chamberlain
  0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2024-03-05 20:22 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Chuck Lever, kdevops

On Tue, Mar 05, 2024 at 12:11:03PM -0800, Luis Chamberlain wrote:
> On Tue, Mar 05, 2024 at 11:36:20AM -0500, Chuck Lever wrote:
> > From: Chuck Lever <chuck.lever@oracle.com>
> > 
> > According to documentation, the default architecture for
> > virt-builder isn't the same as the host's platform architecture.
> > To support platforms other than x86, specify the host's
> > architecture explicitly on the command line.
> > 
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> >  scripts/bringup_guestfs.sh |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
> > index 34ad48cbe81f..51b5a07218ac 100755
> > --- a/scripts/bringup_guestfs.sh
> > +++ b/scripts/bringup_guestfs.sh
> > @@ -67,7 +67,7 @@ _EOT
> >  	fi
> >  
> >  	echo "Generating new base image for ${OS_VERSION}"
> > -	virt-builder ${OS_VERSION} -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
> > +	virt-builder ${OS_VERSION} --arch `uname -m` -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
> 
> Our current kconfig allows target architecture to be different, the
> above seems to assume it matches the host arch so there is that
> disrepancy in this patch. It would break the flexibility we are
> providing.
> 
> Although your commit 44f7e0515dac3c0e ("Kconfig: Remove unused Kconfig
> option") removed TARGET_ARCH, here is an example where that could have
> been used.
> 
> config TARGET_ARCH
> 	string
> 	default "x86_64" if TARGET_ARCH_X86_64
> 	default "ppc64le" if TARGET_ARCH_PPC64LE
> 
> Then the above would use ${CONFIG_TARGET_ARCH}
> 
> A futher consideration is to add a new kconfig option which is disabled
> by default (the default for all kconfig options are to be disabled by
> default) which would *not* make it possible to override the target arch,
> only if that bool would be enabled  would we allow the user to override
> it. We want to discourage this as the performance would suck and the
> results may vary as well.

Then I'm a bit confused. You said before that the TARGET_ARCHITECTURE
setting is supposed to select the set of tests to run, not the ISA.

That's why I got rid of TARGET_ARCH -- the guest's ISA not related to
the CONFIG_TARGET_ARCHITECTURE setting.

I can make virt-builder's --arch option take a Kconfig setting
instead of just running `uname -m`. The default value for that
setting should then be the host's ISA. But do we want a string, or
should it be a menu of limited choices (like the guest memory size
and CPU count settings)?

-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] guestfs: Enable destruction of guests with NVRAM
  2024-03-05 16:36 ` [PATCH 2/4] guestfs: Enable destruction of guests with NVRAM Chuck Lever
@ 2024-03-05 20:23   ` Luis Chamberlain
  2024-03-05 20:40     ` Chuck Lever
  0 siblings, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2024-03-05 20:23 UTC (permalink / raw)
  To: Chuck Lever; +Cc: kdevops

On Tue, Mar 05, 2024 at 11:36:26AM -0500, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> The default guest configuration on ARM systems includes a virtual
> NVRAM device. However, "virsh undefine" won't touch such guests
> without an explicit command-line option that enables destruction of
> that device and its contents.
> 
> Note that virsh(1) says:
> 
> > --nvram and --keep-nvram specify accordingly to delete or keep
> > nvram (/domain/os/nvram/) file.
> 
> Although adding this option to "virsh undefine" prevents an error
> and allows the "undefine" to continue to completion, it does not
> seem to actually delete the virtual NVRAM device's backing file on
> my system.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  scripts/destroy_guestfs.sh |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/destroy_guestfs.sh b/scripts/destroy_guestfs.sh
> index 125890dc34dc..9c627f231cc7 100755
> --- a/scripts/destroy_guestfs.sh
> +++ b/scripts/destroy_guestfs.sh
> @@ -19,7 +19,7 @@ if [ -f "$GUESTFSDIR/kdevops_nodes.yaml" ]; then
>  			if [ "$domstate" = 'running' ]; then
>  				virsh destroy $name
>  			fi
> -			virsh undefine $name
> +			virsh undefine --nvram $name

Do we want to add:

UNDEFINE_EXTRA=""
UNDEFINE_EXTRA_CMD="true"
if [[ "$CONFIG_LIBVIRT_MACHINE_TYPE_VIRT" == "y" ]]; then
	UNDEFINE_EXTRA="--nvram"
	UNDEFINE_EXTRA_CMD="rm -f nvram/foo"
fi

virsh undefine $UNDEFINE_EXTRA $name
$UNDEFINE_EXTRA_CMD

So we should nuke the nvram file, that can be done by augmenting the
KDEVOPS_MRPROPER += with the nvram file on the Makefile for libguest.

 Luis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] guestfs: Specify host architecture to virt-builder
  2024-03-05 20:22     ` Chuck Lever
@ 2024-03-05 20:29       ` Luis Chamberlain
  2024-03-05 20:47         ` Chuck Lever
  0 siblings, 1 reply; 14+ messages in thread
From: Luis Chamberlain @ 2024-03-05 20:29 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Chuck Lever, kdevops

On Tue, Mar 05, 2024 at 03:22:35PM -0500, Chuck Lever wrote:
> On Tue, Mar 05, 2024 at 12:11:03PM -0800, Luis Chamberlain wrote:
> > On Tue, Mar 05, 2024 at 11:36:20AM -0500, Chuck Lever wrote:
> > > From: Chuck Lever <chuck.lever@oracle.com>
> > > 
> > > According to documentation, the default architecture for
> > > virt-builder isn't the same as the host's platform architecture.
> > > To support platforms other than x86, specify the host's
> > > architecture explicitly on the command line.
> > > 
> > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > ---
> > >  scripts/bringup_guestfs.sh |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
> > > index 34ad48cbe81f..51b5a07218ac 100755
> > > --- a/scripts/bringup_guestfs.sh
> > > +++ b/scripts/bringup_guestfs.sh
> > > @@ -67,7 +67,7 @@ _EOT
> > >  	fi
> > >  
> > >  	echo "Generating new base image for ${OS_VERSION}"
> > > -	virt-builder ${OS_VERSION} -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
> > > +	virt-builder ${OS_VERSION} --arch `uname -m` -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
> > 
> > Our current kconfig allows target architecture to be different, the
> > above seems to assume it matches the host arch so there is that
> > disrepancy in this patch. It would break the flexibility we are
> > providing.
> > 
> > Although your commit 44f7e0515dac3c0e ("Kconfig: Remove unused Kconfig
> > option") removed TARGET_ARCH, here is an example where that could have
> > been used.
> > 
> > config TARGET_ARCH
> > 	string
> > 	default "x86_64" if TARGET_ARCH_X86_64
> > 	default "ppc64le" if TARGET_ARCH_PPC64LE
> > 
> > Then the above would use ${CONFIG_TARGET_ARCH}
> > 
> > A futher consideration is to add a new kconfig option which is disabled
> > by default (the default for all kconfig options are to be disabled by
> > default) which would *not* make it possible to override the target arch,
> > only if that bool would be enabled  would we allow the user to override
> > it. We want to discourage this as the performance would suck and the
> > results may vary as well.
> 
> Then I'm a bit confused. You said before that the TARGET_ARCHITECTURE
> setting is supposed to select the set of tests to run, not the ISA.

Sure, but when using virtualization it makes sense to match these since
if you break away from it your host CPU will only be able to emulate and
that's slow.

For cloud solutions the target arch can be anything and that should just
work fine.

> That's why I got rid of TARGET_ARCH -- the guest's ISA not related to
> the CONFIG_TARGET_ARCHITECTURE setting.

I see.

> I can make virt-builder's --arch option take a Kconfig setting
> instead of just running `uname -m`. The default value for that
> setting should then be the host's ISA.

Yeah I think that makes sense for non-cloud solutions, and the user
may only want to override if they insist (to emulate say on a high end
system).

> But do we want a string, or
> should it be a menu of limited choices (like the guest memory size
> and CPU count settings)?

I think by default we give them the right answer, and only if they
enable a bool "Override guest arch" or something will they be able
to modify it.

  Luis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/4] libvirt: Support aarch64 guests
  2024-03-05 16:36 ` [PATCH 4/4] libvirt: Support aarch64 guests Chuck Lever
@ 2024-03-05 20:40   ` Luis Chamberlain
  0 siblings, 0 replies; 14+ messages in thread
From: Luis Chamberlain @ 2024-03-05 20:40 UTC (permalink / raw)
  To: Chuck Lever; +Cc: kdevops

Thanks for doing this!!

Yes I think we should ignore vagrant, let it die. In fact I'm starting
to test libguest to the point I'd like to get a sense when we are ready
to flip the switch to default it over vagrant.

On Tue, Mar 05, 2024 at 11:36:39AM -0500, Chuck Lever wrote:
> diff --git a/kconfigs/Kconfig.libvirt b/kconfigs/Kconfig.libvirt
> index fa39120450fd..6d51a1c26604 100644
> --- a/kconfigs/Kconfig.libvirt
> +++ b/kconfigs/Kconfig.libvirt
> @@ -466,7 +466,8 @@ endif # HAVE_LIBVIRT_PCIE_PASSTHROUGH
>  
>  choice
>  	prompt "Machine type to use"
> -	default LIBVIRT_MACHINE_TYPE_Q35
> +	default LIBVIRT_MACHINE_TYPE_Q35 if TARGET_ARCH_X86_64
> +	default LIBVIRT_MACHINE_TYPE_VIRT if TARGET_ARCH_ARM64
>  
>  config LIBVIRT_MACHINE_TYPE_DEFAULT
>  	bool "Use the default machine type"
> @@ -487,6 +488,11 @@ config LIBVIRT_MACHINE_TYPE_Q35
>  	  Use q35 for the machine type. This will be required for things like
>  	  CXL or PCIe passthrough.
>  
> +config LIBVIRT_MACHINE_TYPE_VIRT
> +	bool "virt"

Should this depends on TARGET_ARCH_ARM64 ?

> diff --git a/playbooks/roles/gen_nodes/templates/guestfs_virt.j2.xml b/playbooks/roles/gen_nodes/templates/guestfs_virt.j2.xml
> new file mode 100644
> index 000000000000..9a7f004dcc1c
> --- /dev/null
> +++ b/playbooks/roles/gen_nodes/templates/guestfs_virt.j2.xml
> @@ -0,0 +1,215 @@
> +<domain type='kvm' xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'>
> +  <name>{{ hostname }}</name>
> +  <memory unit='MiB'>{{ libvirt_mem_mb }}</memory>
> +  <currentMemory unit='MiB'>{{ libvirt_mem_mb }}</currentMemory>
> +  <vcpu placement='static'>{{ libvirt_vcpus_count }}</vcpu>
> +  <os firmware='efi'>
> +    <type arch='aarch64' machine='virt-8.1'>hvm</type>
> +    <firmware>
> +      <feature enabled='no' name='enrolled-keys'/>
> +      <feature enabled='no' name='secure-boot'/>
> +    </firmware>
> +    <loader readonly='yes' type='pflash' format='qcow2'>/usr/share/edk2/aarch64/QEMU_EFI-silent-pflash.qcow2</loader>
> +    <boot dev='hd'/>
> +  </os>
> +  <features>
> +    <acpi/>
> +    <gic version='3'/>
> +  </features>
> +  <cpu mode='{{ 'host-passthrough' if libvirt_host_passthrough else 'host-model' }}' migratable='off'/>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>

For devices which are shared, if we have 0 delta we could use the jinja2
include:

{% include './templates/foo.j2' %

this is already done on playbooks/roles/gen_nodes/templates/kdevops_nodes_split_start.j2.yaml
as an example.

Furthermore, if there is *slight* variation a jinja2 macro can be used,
as in playbooks/roles/gen_nodes/templates/hosts.j2:

{% from "gen_nodes_list.j2" import gen_nodes_list %}                             
{{ gen_nodes_list( nodes, kdevops_baseline_and_dev, 100,                         
		   pcie_passthrough_enable,                                      
		   pcie_passthrough_target_type,                                 
		   pcie_passthrough_target,                                      
		   pcie_passthrough_devices ) }}

It will hopefully save you some time in knowing that in macros you can
only use veriables passed down inside the macro, so variables in
extra_vars.yaml for example won't work.

My hope is that for the extra drives (virtio or nvme) we could use macros so
to share code between x86 and arm.

> +{% if bootlinux_9p %}
> +    <qemu:arg value='-device'/>
> +    <qemu:arg value='{{ bootlinux_9p_driver }},fsdev={{ bootlinux_9p_fsdev }},mount_tag={{ bootlinux_9p_mount_tag }},bus=pcie.0,addr=0x10'/>
> +    <qemu:arg value='-fsdev'/>
> +    <qemu:arg value='local,id={{ bootlinux_9p_fsdev }},path={{ bootlinux_9p_host_path }},security_model={{ bootlinux_9p_security_model }}'/>
> +{% endif %}

Hopefully this can be shared too.

  Luis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] guestfs: Enable destruction of guests with NVRAM
  2024-03-05 20:23   ` Luis Chamberlain
@ 2024-03-05 20:40     ` Chuck Lever
  2024-03-05 20:43       ` Luis Chamberlain
  0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2024-03-05 20:40 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Chuck Lever, kdevops

On Tue, Mar 05, 2024 at 12:23:26PM -0800, Luis Chamberlain wrote:
> On Tue, Mar 05, 2024 at 11:36:26AM -0500, Chuck Lever wrote:
> > From: Chuck Lever <chuck.lever@oracle.com>
> > 
> > The default guest configuration on ARM systems includes a virtual
> > NVRAM device. However, "virsh undefine" won't touch such guests
> > without an explicit command-line option that enables destruction of
> > that device and its contents.
> > 
> > Note that virsh(1) says:
> > 
> > > --nvram and --keep-nvram specify accordingly to delete or keep
> > > nvram (/domain/os/nvram/) file.
> > 
> > Although adding this option to "virsh undefine" prevents an error
> > and allows the "undefine" to continue to completion, it does not
> > seem to actually delete the virtual NVRAM device's backing file on
> > my system.
> > 
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> >  scripts/destroy_guestfs.sh |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/destroy_guestfs.sh b/scripts/destroy_guestfs.sh
> > index 125890dc34dc..9c627f231cc7 100755
> > --- a/scripts/destroy_guestfs.sh
> > +++ b/scripts/destroy_guestfs.sh
> > @@ -19,7 +19,7 @@ if [ -f "$GUESTFSDIR/kdevops_nodes.yaml" ]; then
> >  			if [ "$domstate" = 'running' ]; then
> >  				virsh destroy $name
> >  			fi
> > -			virsh undefine $name
> > +			virsh undefine --nvram $name
> 
> Do we want to add:
> 
> UNDEFINE_EXTRA=""
> UNDEFINE_EXTRA_CMD="true"
> if [[ "$CONFIG_LIBVIRT_MACHINE_TYPE_VIRT" == "y" ]]; then
> 	UNDEFINE_EXTRA="--nvram"
> 	UNDEFINE_EXTRA_CMD="rm -f nvram/foo"
> fi
> 
> virsh undefine $UNDEFINE_EXTRA $name
> $UNDEFINE_EXTRA_CMD
> 
> So we should nuke the nvram file, that can be done by augmenting the
> KDEVOPS_MRPROPER += with the nvram file on the Makefile for libguest.

It's not clear to me whether "virsh undefine" is supposed to do this
for us. The man page suggests that it is supposed to delete that
file. I'd rather not add extra logic to the script to do it if we're
just papering over a bug.

If we know for a fact the man page is incorrect, and "virsh undefine"
is not supposed to delete that backing file, then it makes sense to
add what you suggested. That could be done in a later patch, or I
could wrap it in now once it is confirmed how "virsh undefine
--nvram" is supposed to behave.


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] guestfs: Enable destruction of guests with NVRAM
  2024-03-05 20:40     ` Chuck Lever
@ 2024-03-05 20:43       ` Luis Chamberlain
  0 siblings, 0 replies; 14+ messages in thread
From: Luis Chamberlain @ 2024-03-05 20:43 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Chuck Lever, kdevops

On Tue, Mar 05, 2024 at 03:40:41PM -0500, Chuck Lever wrote:
> On Tue, Mar 05, 2024 at 12:23:26PM -0800, Luis Chamberlain wrote:
> > Do we want to add:
> > 
> > UNDEFINE_EXTRA=""
> > UNDEFINE_EXTRA_CMD="true"
> > if [[ "$CONFIG_LIBVIRT_MACHINE_TYPE_VIRT" == "y" ]]; then
> > 	UNDEFINE_EXTRA="--nvram"
> > 	UNDEFINE_EXTRA_CMD="rm -f nvram/foo"
> > fi
> > 
> > virsh undefine $UNDEFINE_EXTRA $name
> > $UNDEFINE_EXTRA_CMD
> > 
> > So we should nuke the nvram file, that can be done by augmenting the
> > KDEVOPS_MRPROPER += with the nvram file on the Makefile for libguest.
> 
> It's not clear to me whether "virsh undefine" is supposed to do this
> for us. The man page suggests that it is supposed to delete that
> file. I'd rather not add extra logic to the script to do it if we're
> just papering over a bug.

Ah. I see.

> If we know for a fact the man page is incorrect, and "virsh undefine"
> is not supposed to delete that backing file, then it makes sense to
> add what you suggested. That could be done in a later patch, or I
> could wrap it in now once it is confirmed how "virsh undefine
> --nvram" is supposed to behave.

Sounds good!

 Luis

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] guestfs: Specify host architecture to virt-builder
  2024-03-05 20:29       ` Luis Chamberlain
@ 2024-03-05 20:47         ` Chuck Lever
  2024-03-05 20:54           ` Luis Chamberlain
  0 siblings, 1 reply; 14+ messages in thread
From: Chuck Lever @ 2024-03-05 20:47 UTC (permalink / raw)
  To: Luis Chamberlain; +Cc: Chuck Lever, kdevops

On Tue, Mar 05, 2024 at 12:29:29PM -0800, Luis Chamberlain wrote:
> On Tue, Mar 05, 2024 at 03:22:35PM -0500, Chuck Lever wrote:
> > On Tue, Mar 05, 2024 at 12:11:03PM -0800, Luis Chamberlain wrote:
> > > On Tue, Mar 05, 2024 at 11:36:20AM -0500, Chuck Lever wrote:
> > > > From: Chuck Lever <chuck.lever@oracle.com>
> > > > 
> > > > According to documentation, the default architecture for
> > > > virt-builder isn't the same as the host's platform architecture.
> > > > To support platforms other than x86, specify the host's
> > > > architecture explicitly on the command line.
> > > > 
> > > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > > ---
> > > >  scripts/bringup_guestfs.sh |    2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
> > > > index 34ad48cbe81f..51b5a07218ac 100755
> > > > --- a/scripts/bringup_guestfs.sh
> > > > +++ b/scripts/bringup_guestfs.sh
> > > > @@ -67,7 +67,7 @@ _EOT
> > > >  	fi
> > > >  
> > > >  	echo "Generating new base image for ${OS_VERSION}"
> > > > -	virt-builder ${OS_VERSION} -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
> > > > +	virt-builder ${OS_VERSION} --arch `uname -m` -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
> > > 
> > > Our current kconfig allows target architecture to be different, the
> > > above seems to assume it matches the host arch so there is that
> > > disrepancy in this patch. It would break the flexibility we are
> > > providing.
> > > 
> > > Although your commit 44f7e0515dac3c0e ("Kconfig: Remove unused Kconfig
> > > option") removed TARGET_ARCH, here is an example where that could have
> > > been used.
> > > 
> > > config TARGET_ARCH
> > > 	string
> > > 	default "x86_64" if TARGET_ARCH_X86_64
> > > 	default "ppc64le" if TARGET_ARCH_PPC64LE
> > > 
> > > Then the above would use ${CONFIG_TARGET_ARCH}
> > > 
> > > A futher consideration is to add a new kconfig option which is disabled
> > > by default (the default for all kconfig options are to be disabled by
> > > default) which would *not* make it possible to override the target arch,
> > > only if that bool would be enabled  would we allow the user to override
> > > it. We want to discourage this as the performance would suck and the
> > > results may vary as well.
> > 
> > Then I'm a bit confused. You said before that the TARGET_ARCHITECTURE
> > setting is supposed to select the set of tests to run, not the ISA.
> 
> Sure, but when using virtualization it makes sense to match these since
> if you break away from it your host CPU will only be able to emulate and
> that's slow.

Agreed: the "--arch" setting should match the host's ISA in most
normal cases. That is the only behavior allowed by this patch, but
I can make it settable, as you suggest.


> For cloud solutions the target arch can be anything and that should just
> work fine.

As far as I can tell, this patch changes only the behavior of
guestfs-created guests. That excludes cloud environments, right?

It's always possible I'm missing something.


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] guestfs: Specify host architecture to virt-builder
  2024-03-05 20:47         ` Chuck Lever
@ 2024-03-05 20:54           ` Luis Chamberlain
  0 siblings, 0 replies; 14+ messages in thread
From: Luis Chamberlain @ 2024-03-05 20:54 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Chuck Lever, kdevops

On Tue, Mar 5, 2024 at 12:47 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Tue, Mar 05, 2024 at 12:29:29PM -0800, Luis Chamberlain wrote:
> > On Tue, Mar 05, 2024 at 03:22:35PM -0500, Chuck Lever wrote:
> > > On Tue, Mar 05, 2024 at 12:11:03PM -0800, Luis Chamberlain wrote:
> > > > On Tue, Mar 05, 2024 at 11:36:20AM -0500, Chuck Lever wrote:
> > > > > From: Chuck Lever <chuck.lever@oracle.com>
> > > > >
> > > > > According to documentation, the default architecture for
> > > > > virt-builder isn't the same as the host's platform architecture.
> > > > > To support platforms other than x86, specify the host's
> > > > > architecture explicitly on the command line.
> > > > >
> > > > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > > > > ---
> > > > >  scripts/bringup_guestfs.sh |    2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/scripts/bringup_guestfs.sh b/scripts/bringup_guestfs.sh
> > > > > index 34ad48cbe81f..51b5a07218ac 100755
> > > > > --- a/scripts/bringup_guestfs.sh
> > > > > +++ b/scripts/bringup_guestfs.sh
> > > > > @@ -67,7 +67,7 @@ _EOT
> > > > >         fi
> > > > >
> > > > >         echo "Generating new base image for ${OS_VERSION}"
> > > > > -       virt-builder ${OS_VERSION} -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
> > > > > +       virt-builder ${OS_VERSION} --arch `uname -m` -o $BASE_IMAGE --size 20G --format raw --commands-from-file $cmdfile
> > > >
> > > > Our current kconfig allows target architecture to be different, the
> > > > above seems to assume it matches the host arch so there is that
> > > > disrepancy in this patch. It would break the flexibility we are
> > > > providing.
> > > >
> > > > Although your commit 44f7e0515dac3c0e ("Kconfig: Remove unused Kconfig
> > > > option") removed TARGET_ARCH, here is an example where that could have
> > > > been used.
> > > >
> > > > config TARGET_ARCH
> > > >   string
> > > >   default "x86_64" if TARGET_ARCH_X86_64
> > > >   default "ppc64le" if TARGET_ARCH_PPC64LE
> > > >
> > > > Then the above would use ${CONFIG_TARGET_ARCH}
> > > >
> > > > A futher consideration is to add a new kconfig option which is disabled
> > > > by default (the default for all kconfig options are to be disabled by
> > > > default) which would *not* make it possible to override the target arch,
> > > > only if that bool would be enabled  would we allow the user to override
> > > > it. We want to discourage this as the performance would suck and the
> > > > results may vary as well.
> > >
> > > Then I'm a bit confused. You said before that the TARGET_ARCHITECTURE
> > > setting is supposed to select the set of tests to run, not the ISA.
> >
> > Sure, but when using virtualization it makes sense to match these since
> > if you break away from it your host CPU will only be able to emulate and
> > that's slow.
>
> Agreed: the "--arch" setting should match the host's ISA in most
> normal cases. That is the only behavior allowed by this patch, but
> I can make it settable, as you suggest.

I don't care for it to be settable myself as I realize it is insane to
do otherwise. I was just pointing out that today we do allow the
target arch to be modified by the user if using virtualization.

> > For cloud solutions the target arch can be anything and that should just
> > work fine.
>
> As far as I can tell, this patch changes only the behavior of
> guestfs-created guests. That excludes cloud environments, right?
>
> It's always possible I'm missing something.

Sure, I'm just saying if we modify the uname -m to be extracted via a
kconfig option it may be an option in kconfigs/arch/Kconfig which is
shared with cloud.

  Luis

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-03-05 20:54 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-05 16:36 [PATCH 0/4] Enable aarch64 with guestfs Chuck Lever
2024-03-05 16:36 ` [PATCH 1/4] guestfs: Specify host architecture to virt-builder Chuck Lever
2024-03-05 20:11   ` Luis Chamberlain
2024-03-05 20:22     ` Chuck Lever
2024-03-05 20:29       ` Luis Chamberlain
2024-03-05 20:47         ` Chuck Lever
2024-03-05 20:54           ` Luis Chamberlain
2024-03-05 16:36 ` [PATCH 2/4] guestfs: Enable destruction of guests with NVRAM Chuck Lever
2024-03-05 20:23   ` Luis Chamberlain
2024-03-05 20:40     ` Chuck Lever
2024-03-05 20:43       ` Luis Chamberlain
2024-03-05 16:36 ` [PATCH 3/4] gen_nodes: Instructions for adding a new guestfs architecture Chuck Lever
2024-03-05 16:36 ` [PATCH 4/4] libvirt: Support aarch64 guests Chuck Lever
2024-03-05 20:40   ` Luis Chamberlain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.