[PATCH v1 07/13] volume_group: Detect the /data partition directly

[cel@kernel.org]

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

The volume_group role currently runs during "bringup", but the
/data partition is set up later by subsequent make targets (eg, when
the test kernel is built or when workflows are set up).

This means the volume_group role has to avoid using one of the block
devices so that later steps can put the /data partition on the
skipped device.

Instead of counting the number of available block devices and just
skipping one of them, explicitly avoid using the device listed in
{{ data_device }}. This is a more specific and reliable check.

Works only for guestfs. Support for terraform providers is next.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 kconfigs/Kconfig.iscsi                        | 22 -------
 kconfigs/Kconfig.nfsd                         | 21 -------
 kconfigs/Kconfig.smbd                         | 17 ------
 playbooks/roles/iscsi/defaults/main.yml       |  2 -
 playbooks/roles/iscsi/tasks/main.yml          |  2 -
 playbooks/roles/nfsd/defaults/main.yml        |  2 -
 playbooks/roles/nfsd/tasks/main.yml           |  2 -
 playbooks/roles/smbd/defaults/main.yml        |  2 -
 playbooks/roles/smbd/tasks/main.yml           |  2 -
 playbooks/roles/volume_group/README.md        |  6 --
 .../roles/volume_group/defaults/main.yml      |  1 +
 .../roles/volume_group/tasks/guestfs.yml      | 59 +++++++++++++++++++
 playbooks/roles/volume_group/tasks/main.yml   | 17 ++++--
 scripts/iscsi.Makefile                        |  2 -
 scripts/nfsd.Makefile                         |  2 -
 scripts/smbd.Makefile                         |  2 -
 16 files changed, 71 insertions(+), 90 deletions(-)
 create mode 100644 playbooks/roles/volume_group/tasks/guestfs.yml

diff --git a/kconfigs/Kconfig.iscsi b/kconfigs/Kconfig.iscsi
index d95c82ac34c7..2c40372621b8 100644
--- a/kconfigs/Kconfig.iscsi
+++ b/kconfigs/Kconfig.iscsi
@@ -19,26 +19,4 @@ config ISCSI_TARGET_WWN
 	  If you do not know what this means, the default is safe
 	  to use.
 
-config ISCSI_TARGET_PV_PREFIX
-	string "Prefix to use for iSCSI target LVM physical volume names"
-	default "/dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_kdevops" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_NVME
-	default "/dev/disk/by-id/virtio-kdevops" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_VIRTIO
-	default "/dev/disk/by-id/ata-QEMU_HARDDISK_kdevops" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_IDE
-	default ""
-	help
-	  This string is the prefix for LVM physical volume names.
-
-	  If you do not know what this means, the default is safe
-	  to use.
-
-config ISCSI_TARGET_PV_COUNT
-	int "Number of devices to add as LVM physical volumes"
-	default 3 if LIBVIRT
-	help
-	  The number of physical devices on the iSCSI target node to
-	  dedicate as LVM physical volumes.
-
-	  If you do not know what this means, the default is safe
-	  to use.
-
 endif
diff --git a/kconfigs/Kconfig.nfsd b/kconfigs/Kconfig.nfsd
index 60e9da1aba2d..69fc9d2e38d8 100644
--- a/kconfigs/Kconfig.nfsd
+++ b/kconfigs/Kconfig.nfsd
@@ -85,27 +85,6 @@ config NFSD_EXPORT_STORAGE_ISCSI
 
 endchoice
 
-if NFSD_EXPORT_STORAGE_LOCAL
-
-config NFSD_EXPORT_DEVICE_PREFIX
-	string "The device prefix to use for LVM PVs"
-	default "/dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_kdevops" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_NVME
-	default "/dev/disk/by-id/virtio-kdevops" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_VIRTIO
-	default "/dev/disk/by-id/ata-QEMU_HARDDISK_kdevops" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_IDE
-	default ""
-	help
-	  To set up nfsd for testing, we give it filesystems to export. This string
-	  will be the prefix for the block devices used as PVs for LVM.
-
-config NFSD_EXPORT_DEVICE_COUNT
-	int "Number of devices to add as LVM PVs"
-	default 3 if LIBVIRT
-	help
-	  The number of disk devices to dedicate as LVM PVs. In general, we
-	  avoid using device index 0 as that is used for /data.
-
-endif
-
 endmenu
 
 endif
diff --git a/kconfigs/Kconfig.smbd b/kconfigs/Kconfig.smbd
index 251327d67376..b7c33d180abd 100644
--- a/kconfigs/Kconfig.smbd
+++ b/kconfigs/Kconfig.smbd
@@ -11,23 +11,6 @@ if KDEVOPS_SETUP_SMBD
 
 menu "Configure the Samba SMB server"
 
-config SMBD_SHARE_DEVICE_PREFIX
-	string "The device prefix to use for LVM PVs"
-	default "/dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_kdevops" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_NVME
-	default "/dev/disk/by-id/virtio-kdevops" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_VIRTIO
-	default "/dev/disk/by-id/ata-QEMU_HARDDISK_kdevops" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_IDE
-	default ""
-	help
-	  To set up smbd for testing, we give it filesystems to share. This string
-	  will be the prefix for the block devices used as PVs for LVM.
-
-config SMBD_SHARE_DEVICE_COUNT
-	int "Number of devices to add as LVM PVs"
-	default 3 if LIBVIRT
-	help
-	  The number of disk devices to dedicate as LVM PVs. In general, we
-	  avoid using device index 0 as that is used for /data.
-
 choice
 	prompt "Type of filesystem to share"
 	default SMBD_SHARE_FSTYPE_BTRFS
diff --git a/playbooks/roles/iscsi/defaults/main.yml b/playbooks/roles/iscsi/defaults/main.yml
index 5219a2f3ba30..3617ac0d333a 100644
--- a/playbooks/roles/iscsi/defaults/main.yml
+++ b/playbooks/roles/iscsi/defaults/main.yml
@@ -1,6 +1,4 @@
 ---
 # Our sensible defaults for the iscsi role.
 iscsi_target_hostname: "{{ kdevops_host_prefix }}-iscsi"
-iscsi_target_pv_prefix: ""
-iscsi_target_pv_count: 0
 iscsi_target_vg_name: "iscsi_luns"
diff --git a/playbooks/roles/iscsi/tasks/main.yml b/playbooks/roles/iscsi/tasks/main.yml
index 2638bacc882b..a95351cca5a7 100644
--- a/playbooks/roles/iscsi/tasks/main.yml
+++ b/playbooks/roles/iscsi/tasks/main.yml
@@ -23,8 +23,6 @@
     name: volume_group
   vars:
     volume_group_name: "{{ iscsi_target_vg_name }}"
-    volume_device_prefix: "{{ iscsi_target_pv_prefix }}"
-    volume_device_count: "{{ iscsi_target_pv_count }}"
 
 - name: Create a directory for storing iSCSI persistent reservations
   become: true
diff --git a/playbooks/roles/nfsd/defaults/main.yml b/playbooks/roles/nfsd/defaults/main.yml
index ccdee468bd02..788d26224266 100644
--- a/playbooks/roles/nfsd/defaults/main.yml
+++ b/playbooks/roles/nfsd/defaults/main.yml
@@ -1,8 +1,6 @@
 # SPDX-License-Identifier GPL-2.0+
 ---
 # Our sensible defaults for the nfsd role.
-nfsd_export_device_prefix: ""
-nfsd_export_device_count: 0
 nfsd_export_label: "export"
 nfsd_export_fs_opts: ""
 nfsd_lease_time: "90"
diff --git a/playbooks/roles/nfsd/tasks/main.yml b/playbooks/roles/nfsd/tasks/main.yml
index 144ecd86686e..21446f224a08 100644
--- a/playbooks/roles/nfsd/tasks/main.yml
+++ b/playbooks/roles/nfsd/tasks/main.yml
@@ -35,8 +35,6 @@
     name: volume_group
   vars:
     volume_group_name: "exports"
-    volume_device_prefix: "{{ nfsd_export_device_prefix }}"
-    volume_device_count: "{{ nfsd_export_device_count }}"
   when:
     - nfsd_export_storage_local|bool
     - nfsd_export_fstype != "tmpfs"
diff --git a/playbooks/roles/smbd/defaults/main.yml b/playbooks/roles/smbd/defaults/main.yml
index 4161224f27b9..cbb4974ed3bc 100644
--- a/playbooks/roles/smbd/defaults/main.yml
+++ b/playbooks/roles/smbd/defaults/main.yml
@@ -1,4 +1,2 @@
 ---
-smbd_share_device_prefix: ""
-smbd_share_device_count: 0
 smbd_share_label: "share"
diff --git a/playbooks/roles/smbd/tasks/main.yml b/playbooks/roles/smbd/tasks/main.yml
index 486ccdc613bf..926358b3f3f6 100644
--- a/playbooks/roles/smbd/tasks/main.yml
+++ b/playbooks/roles/smbd/tasks/main.yml
@@ -27,8 +27,6 @@
     name: volume_group
   var:
     volume_group_name: "shares"
-    volume_device_prefix: "{{ smbd_share_device_prefix }}"
-    volume_device_count: "{{ smbd_share_device_count }}"
 
 - name: Create {{ smbd_share_path }}
   become: yes
diff --git a/playbooks/roles/volume_group/README.md b/playbooks/roles/volume_group/README.md
index 56cb7c55f9f4..cd1ab48ce3ba 100644
--- a/playbooks/roles/volume_group/README.md
+++ b/playbooks/roles/volume_group/README.md
@@ -14,10 +14,6 @@ Role Variables
 --------------
 
   * volume_group_name: The name for new volume group (string)
-  * volume_device_prefix: The pathname prefix for block devices to
-    consider for the new volume group (string)
-  * volume_device_count: The number of block devices to include in
-    the new volume group (int)
 
 Dependencies
 ------------
@@ -35,8 +31,6 @@ Below is an example playbook task:
     name: volume_group
   vars:
     volume_group_name: "exports"
-    volume_device_prefix: "/dev/disk/by-id/virtio*"
-    volume_count: 3
 ```
 
 For further examples refer to one of this role's users, the
diff --git a/playbooks/roles/volume_group/defaults/main.yml b/playbooks/roles/volume_group/defaults/main.yml
index 68f3fb3f5775..b7707cab59d5 100644
--- a/playbooks/roles/volume_group/defaults/main.yml
+++ b/playbooks/roles/volume_group/defaults/main.yml
@@ -1,2 +1,3 @@
 ---
 physical_volumes: []
+kdevops_enable_guestfs: false
diff --git a/playbooks/roles/volume_group/tasks/guestfs.yml b/playbooks/roles/volume_group/tasks/guestfs.yml
new file mode 100644
index 000000000000..a5536159a1b6
--- /dev/null
+++ b/playbooks/roles/volume_group/tasks/guestfs.yml
@@ -0,0 +1,59 @@
+---
+# Select unused block devices under /dev/disk/by-id/ .
+#
+# Avoid these two block devices:
+# 1. The block device where the root partition resides. For
+#    guestfs, this device is /dev/vda and is not listed under
+#    /dev/disk/by-id/
+# 2. The block device where the /data partition resides. For
+#    guestfs, this device is named by a path to a symlink
+#    under /dev/disk/by-id/
+#
+
+- name: Set the NVMe device search pattern
+  ansible.builtin.set_fact:
+    by_id_pattern: "nvme-QEMU_NVMe_Ctrl_kdevops*"
+  when:
+    - libvirt_extra_storage_drive_nvme is defined
+    - libvirt_extra_storage_drive_nvme|bool
+
+- name: Set the virtio block device search pattern
+  ansible.builtin.set_fact:
+    by_id_pattern: "virtio-kdevops*"
+  when:
+    - libvirt_extra_storage_drive_virtio is defined
+    - libvirt_extra_storage_drive_virtio|bool
+
+- name: Set the IDE device search pattern
+  ansible.builtin.set_fact:
+    by_id_pattern: "ata-QEMU_HARDDISK_kdevops*"
+  when:
+    - libvirt_extra_storage_drive_ide is defined
+    - libvirt_extra_storage_drive_ide|bool
+
+- name: Verify there are block devices to search
+  ansible.builtin.fail:
+    msg: No supported block devices are available for NFSD.
+  when:
+    - by_id_pattern is not defined
+
+- name: Show the pathname of the data device
+  ansible.builtin.debug:
+    msg: "Reserved device for /data: {{ data_device }}"
+
+- name: Discover usable block devices
+  ansible.builtin.find:
+    paths: /dev/disk/by-id
+    file_type: link
+    patterns: "{{ by_id_pattern }}"
+    excludes: "{{ data_device | basename }},*_?,*-part?"
+  register: device_ids
+  failed_when:
+    - device_ids.failed or device_ids.matched == 0
+
+- name: Build a list of block devices to provision as PVs
+  ansible.builtin.set_fact:
+    physical_volumes: "{{ physical_volumes + [item.path] }}"
+  loop: "{{ device_ids.files }}"
+  loop_control:
+    label: "Block device: {{ item.path }}"
diff --git a/playbooks/roles/volume_group/tasks/main.yml b/playbooks/roles/volume_group/tasks/main.yml
index 086c86454893..bc0fbcd8c578 100644
--- a/playbooks/roles/volume_group/tasks/main.yml
+++ b/playbooks/roles/volume_group/tasks/main.yml
@@ -15,12 +15,17 @@
       - lvm2
     state: present
 
-- name: Enumerate block devices to provision as physical volumes
-  ansible.builtin.set_fact:
-    physical_volumes: "{{ physical_volumes + [volume_device_prefix + item | string] }}"
-  with_items: "{{ range(1, volume_device_count + 1) }}"
-  loop_control:
-    label: "Block device: {{ volume_device_prefix + item | string }}"
+- name: Enumerate block devices on the target nodes
+  ansible.builtin.include_tasks:
+    file: "guestfs.yml"
+  when:
+    - kdevops_enable_guestfs|bool
+
+- name: Verify there are remaining candidates to use for physical volumes
+  ansible.builtin.fail:
+    msg: No local block devices are available for an LVM volume group.
+  when:
+    - physical_volumes|length == 0
 
 - name: Create an LVM Volume Group
   become: true
diff --git a/scripts/iscsi.Makefile b/scripts/iscsi.Makefile
index 9ea524fd4417..893183721dfc 100644
--- a/scripts/iscsi.Makefile
+++ b/scripts/iscsi.Makefile
@@ -1,8 +1,6 @@
 ifeq (y,$(CONFIG_KDEVOPS_ENABLE_ISCSI))
 
 ISCSI_EXTRA_ARGS += iscsi_target_wwn='$(subst ",,$(CONFIG_ISCSI_TARGET_WWN))'
-ISCSI_EXTRA_ARGS += iscsi_target_pv_prefix='$(subst ",,$(CONFIG_ISCSI_TARGET_PV_PREFIX))'
-ISCSI_EXTRA_ARGS += iscsi_target_pv_count='$(subst ",,$(CONFIG_ISCSI_TARGET_PV_COUNT))'
 ISCSI_EXTRA_ARGS += kdevops_enable_iscsi=true
 
 ANSIBLE_EXTRA_ARGS += $(ISCSI_EXTRA_ARGS)
diff --git a/scripts/nfsd.Makefile b/scripts/nfsd.Makefile
index 959cc4b7652d..39e4a817421a 100644
--- a/scripts/nfsd.Makefile
+++ b/scripts/nfsd.Makefile
@@ -2,8 +2,6 @@ ifeq (y,$(CONFIG_KDEVOPS_SETUP_NFSD))
 
 ifeq (y,$(CONFIG_NFSD_EXPORT_STORAGE_LOCAL))
 NFSD_EXTRA_ARGS += nfsd_export_storage_local=true
-NFSD_EXTRA_ARGS += nfsd_export_device_prefix='$(subst ",,$(CONFIG_NFSD_EXPORT_DEVICE_PREFIX))'
-NFSD_EXTRA_ARGS += nfsd_export_device_count='$(subst ",,$(CONFIG_NFSD_EXPORT_DEVICE_COUNT))'
 endif
 
 ifeq (y,$(CONFIG_NFSD_EXPORT_STORAGE_ISCSI))
diff --git a/scripts/smbd.Makefile b/scripts/smbd.Makefile
index ae23497d29f0..77d7727b137a 100644
--- a/scripts/smbd.Makefile
+++ b/scripts/smbd.Makefile
@@ -1,7 +1,5 @@
 ifeq (y,$(CONFIG_KDEVOPS_SETUP_SMBD))
 
-SMBD_EXTRA_ARGS += smbd_share_device_prefix='$(subst ",,$(CONFIG_SMBD_SHARE_DEVICE_PREFIX))'
-SMBD_EXTRA_ARGS += smbd_share_device_count='$(subst ",,$(CONFIG_SMBD_SHARE_DEVICE_COUNT))'
 SMBD_EXTRA_ARGS += smbd_share_fstype='$(subst ",,$(CONFIG_SMBD_SHARE_FSTYPE))'
 SMBD_EXTRA_ARGS += smbd_share_path='$(subst ",,$(CONFIG_SMBD_SHARE_PATH))'
 SMBD_EXTRA_ARGS += smb_root_pw='$(subst ",,$(CONFIG_SMB_ROOT_PW))'
-- 
2.48.1