Re: [PATCH] qemu: Switch to virtio-scsi on ARM

On Thu, Jun 26, 2025 at 03:29:58PM -0600, Jim Fehlig via Devel wrote: > Similar to x86, the default SCSI controller model for ARM is lsilogic. > But unlike x86, the ARM virt machine type prefers virtio devices. Switch > the default controller model for ARM from lsilogic to virtio-scsi. > > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > > IMO, the lsilogic SCSI controller is a poor default for the ARM virt machine > type. One could argue modern operating systems are more likely to contain a > functional virtio-scsi driver than an LSI one. Agreed. Note that virt-manager has been defaulting to virtio-scsi on all architectures for a fairly long time now[1]. > However, I do understand this > change could break existing ARM VM configurations containing a SCSI > controller without a model specification. One could also argue the pain > inflicted is tolerable :-). I don't think this should necessarily be a concern. Unlike, say, USB controllers, where in some cases you could end up with no model recorded in the XML, for SCSI controllers we always either figure out a suitable model or fail defining the domain altogether. So changing the default here should have no impact on existing domains and simply improve things for newly-created ones.

I had a similar change for RISC-V as part of a series I posted a while ago[2] and that I should probably pick back up. Extending that to Arm sounds good to me. > The test churn is interesting. I haven't yet investigated if there's an > underlying bug, or if it's a consequence of libvirt's processing of > controllers. Much appreciated if anyone has an explanation handy :-). The diff looks good. Unlike virtio-scsi-pci, lsi is a traditional PCI device, so attaching it to the PCIe-native virt machine type requires using a pcie-pci-bridge controller. Once libvirt no longer needs to add that, PCI addresses will naturally shift around a bit.

> +++ b/src/qemu/qemu_domain.c > @@ -4252,7 +4252,8 @@ qemuDomainGetSCSIControllerModel(const virDomainDef *def, > > if (qemuDomainIsPSeries(def)) > return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI; > - if (ARCH_IS_S390(def->os.arch) || qemuDomainIsLoongArchVirt(def)) > + if (ARCH_IS_ARM(def->os.arch) || ARCH_IS_S390(def->os.arch) || > + qemuDomainIsLoongArchVirt(def)) > return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI; I would use qemuDomainIsARMVirt() instead of ARCH_IS_ARM() here.

I don't have a strong motivation to offer, but generally I've been using the former because it's much easier to justify that some changes are valid when applied to the virt machine type specifically, rather than to any possible Arm machine type. With that changed Reviewed-by: Andrea Bolognani <abologna(a)redhat.com&gt; but please wait a few days before pushing so that other developers have a chance to point out any potential issue with this change that I might not have considered.