On 07/03/2019 18.15, Michal Privoznik wrote:
On 3/1/19 12:10 PM, Thomas Huth wrote:
> When running virt-host-validate on an s390x host, the tool currently
> warns
> that it is "Unknown if this platform has IOMMU support". We can use the
> common check for entries in /sys/kernel/iommu_groups here, too, but it
> only
> makes sense to check it if there are also PCI devices available. It's
> also
> common on s390x that there are no PCI devices assigned to the LPAR,
> and in
> that case there is no need for the PCI-related IOMMU, so without PCI
> devices
> we should simply skip this test.
>
> Signed-off-by: Thomas Huth <thuth(a)redhat.com>
> ---
> v2:
> - Use virDirOpen() + virDirRead() instead of stat() - this should
> hopefully
> work now as expected.
>
> tools/virt-host-validate-common.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/tools/virt-host-validate-common.c
> b/tools/virt-host-validate-common.c
> index 73e3bdb..e27b558 100644
> --- a/tools/virt-host-validate-common.c
> +++ b/tools/virt-host-validate-common.c
> @@ -337,7 +337,12 @@ int virHostValidateIOMMU(const char *hvname,
> virBitmapPtr flags;
> struct stat sb;
> const char *bootarg = NULL;
> - bool isAMD = false, isIntel = false, isPPC = false;
> + bool isAMD = false, isIntel = false;
> + virArch arch = virArchFromHost();
> + struct dirent *dent;
> + DIR *dir;
> + int rc;
> +
> flags = virHostValidateGetCPUFlags();
> if (flags && virBitmapIsBitSet(flags,
> VIR_HOST_VALIDATE_CPU_FLAG_VMX))
> @@ -347,8 +352,6 @@ int virHostValidateIOMMU(const char *hvname,
> virBitmapFree(flags);
> - isPPC = ARCH_IS_PPC64(virArchFromHost());
> -
> if (isIntel) {
> virHostMsgCheck(hvname, "%s", _("for device assignment
IOMMU
> support"));
> if (access("/sys/firmware/acpi/tables/DMAR", F_OK) == 0) {
> @@ -373,8 +376,17 @@ int virHostValidateIOMMU(const char *hvname,
> "hardware platform");
> return -1;
> }
> - } else if (isPPC) {
> + } else if (ARCH_IS_PPC64(arch)) {
> /* Empty Block */
> + } else if (ARCH_IS_S390(arch)) {
> + /* On s390x, we skip the IOMMU check if there are no PCI devices
> + * (which is quite usual on s390x) */
> + if (!virDirOpen(&dir, "/sys/bus/pci/devices"))
> + return 0;
> + rc = virDirRead(dir, &dent, NULL);
> + VIR_DIR_CLOSE(dir);
Is this diropen and dirread really necessary?
Yes.
I mean, would something
like plain test of the path presence be okay, e.g.
virFileExists("/sys/bus/pci/devices").
No, that's unfortunately not enough. The "/sys/bus/pci/devices"
directory is always present, even if there are no PCI devices available,
it's just empty in that case. So as far as I can see, the dirread is
necessary here.
Thomas