[PATCH v2] nfsd: rework how storage is allocated to the LVM VG

[PATCH v2] nfsd: rework how storage is allocated to the LVM VG

[Jeff Layton]

Currently we require the user to put in two different path strings, and
they have to reset that if the configuration changes. This is limiting,
since we need to add the ability to suck in a 3rd disk.

Rework it to take a device prefix string and a count. "count" number of
devices will be added to the VG, starting at index 1.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 kconfigs/Kconfig.nfsd                  | 26 ++++++++++++--------------
 playbooks/roles/nfsd/defaults/main.yml |  3 +++
 playbooks/roles/nfsd/tasks/main.yml    | 11 ++++++++++-
 scripts/nfsd.Makefile                  |  4 ++--
 4 files changed, 27 insertions(+), 17 deletions(-)

v2: I ended up sending an earlier draft by accident. This one actually
works.

diff --git a/kconfigs/Kconfig.nfsd b/kconfigs/Kconfig.nfsd
index 946c2c101b58..db80d8c0981e 100644
--- a/kconfigs/Kconfig.nfsd
+++ b/kconfigs/Kconfig.nfsd
@@ -1,22 +1,20 @@
 # FIXME: need defaults for terraform hosts
-config NFSD_EXPORT_DEVICE_0
-	string "First device to be exported"
-	default "/dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_kdevops2" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_NVME
-	default "/dev/disk/by-id/virtio-kdevops2" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_VIRTIO
-	default "/dev/disk/by-id/ata-QEMU_HARDDISK_kdevops2" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_IDE
+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
-	  block device is used as a PV for LVM, and volumes are provisioned from it.
+	  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_1
-	string "Second device to be exported"
-	default "/dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_kdevops3" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_NVME
-	default "/dev/disk/by-id/virtio-kdevops3" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_VIRTIO
-	default "/dev/disk/by-id/ata-QEMU_HARDDISK_kdevops3" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_IDE
+config NFSD_EXPORT_DEVICE_COUNT
+	int "Number of devices to add as LVM PVs"
+	default 3 if LIBVIRT
 	help
-	  To set up nfsd for testing, we give it filesystems to export. This
-	  block device is used as a PV for LVM, and volumes are provisioned from it.
+	  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 export"
diff --git a/playbooks/roles/nfsd/defaults/main.yml b/playbooks/roles/nfsd/defaults/main.yml
index 4a72f4f0b048..38718aec2bf7 100644
--- a/playbooks/roles/nfsd/defaults/main.yml
+++ b/playbooks/roles/nfsd/defaults/main.yml
@@ -1,6 +1,9 @@
 # SPDX-License-Identifier GPL-2.0+
 ---
 # Our sensible defaults for the nfsd role.
+nfsd_lvm_pvs: []
+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 e051d21bd819..fe5f6919864d 100644
--- a/playbooks/roles/nfsd/tasks/main.yml
+++ b/playbooks/roles/nfsd/tasks/main.yml
@@ -22,13 +22,22 @@
     group: root
     mode: 0644
 
+- name: Build string of devices to use as PVs
+  set_fact:
+    nfsd_lvm_pvs: "{{ nfsd_lvm_pvs + [ nfsd_export_device_prefix + item|string ] }}"
+  with_items: "{{ range(1, nfsd_export_device_count + 1) }}"
+
+- name: Print the PV list
+  ansible.builtin.debug:
+    var: nfsd_lvm_pvs
+
 - name: Create a new LVM VG
   become: yes
   become_flags: 'su - -c'
   become_method: sudo
   community.general.lvg:
     vg: "exports"
-    pvs: "{{ nfsd_export_device_0 }},{{ nfsd_export_device_1 }}"
+    pvs: "{{ nfsd_lvm_pvs | join(',') }}"
 
 - name: Create {{ nfsd_export_path }}
   become: yes
diff --git a/scripts/nfsd.Makefile b/scripts/nfsd.Makefile
index 40338ab93fce..d3549a29af84 100644
--- a/scripts/nfsd.Makefile
+++ b/scripts/nfsd.Makefile
@@ -1,5 +1,5 @@
-NFSD_EXTRA_ARGS += nfsd_export_device_0='$(subst ",,$(CONFIG_NFSD_EXPORT_DEVICE_0))'
-NFSD_EXTRA_ARGS += nfsd_export_device_1='$(subst ",,$(CONFIG_NFSD_EXPORT_DEVICE_1))'
+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))'
 NFSD_EXTRA_ARGS += nfsd_export_fstype='$(subst ",,$(CONFIG_NFSD_EXPORT_FSTYPE))'
 NFSD_EXTRA_ARGS += nfsd_export_path='$(subst ",,$(CONFIG_NFSD_EXPORT_PATH))'
 NFSD_EXTRA_ARGS += nfsd_export_options='$(subst ",,$(CONFIG_NFSD_EXPORT_OPTIONS))'
-- 
2.43.2

Re: [PATCH v2] nfsd: rework how storage is allocated to the LVM VG

[Chuck Lever]

On Fri, Mar 01, 2024 at 02:31:56PM -0500, Jeff Layton wrote:
> Currently we require the user to put in two different path strings, and
> they have to reset that if the configuration changes. This is limiting,
> since we need to add the ability to suck in a 3rd disk.
> 
> Rework it to take a device prefix string and a count. "count" number of
> devices will be added to the VG, starting at index 1.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

No objection to any of these. My current kdevops system is too small
to run more than three guests at once, so I haven't hit this
particular limitation so far. My new system will be able to, though.


> ---
>  kconfigs/Kconfig.nfsd                  | 26 ++++++++++++--------------
>  playbooks/roles/nfsd/defaults/main.yml |  3 +++
>  playbooks/roles/nfsd/tasks/main.yml    | 11 ++++++++++-
>  scripts/nfsd.Makefile                  |  4 ++--
>  4 files changed, 27 insertions(+), 17 deletions(-)
> 
> v2: I ended up sending an earlier draft by accident. This one actually
> works.
> 
> diff --git a/kconfigs/Kconfig.nfsd b/kconfigs/Kconfig.nfsd
> index 946c2c101b58..db80d8c0981e 100644
> --- a/kconfigs/Kconfig.nfsd
> +++ b/kconfigs/Kconfig.nfsd
> @@ -1,22 +1,20 @@
>  # FIXME: need defaults for terraform hosts
> -config NFSD_EXPORT_DEVICE_0
> -	string "First device to be exported"
> -	default "/dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_kdevops2" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_NVME
> -	default "/dev/disk/by-id/virtio-kdevops2" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_VIRTIO
> -	default "/dev/disk/by-id/ata-QEMU_HARDDISK_kdevops2" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_IDE
> +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
> -	  block device is used as a PV for LVM, and volumes are provisioned from it.
> +	  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_1
> -	string "Second device to be exported"
> -	default "/dev/disk/by-id/nvme-QEMU_NVMe_Ctrl_kdevops3" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_NVME
> -	default "/dev/disk/by-id/virtio-kdevops3" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_VIRTIO
> -	default "/dev/disk/by-id/ata-QEMU_HARDDISK_kdevops3" if LIBVIRT && LIBVIRT_EXTRA_STORAGE_DRIVE_IDE
> +config NFSD_EXPORT_DEVICE_COUNT
> +	int "Number of devices to add as LVM PVs"
> +	default 3 if LIBVIRT
>  	help
> -	  To set up nfsd for testing, we give it filesystems to export. This
> -	  block device is used as a PV for LVM, and volumes are provisioned from it.
> +	  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 export"
> diff --git a/playbooks/roles/nfsd/defaults/main.yml b/playbooks/roles/nfsd/defaults/main.yml
> index 4a72f4f0b048..38718aec2bf7 100644
> --- a/playbooks/roles/nfsd/defaults/main.yml
> +++ b/playbooks/roles/nfsd/defaults/main.yml
> @@ -1,6 +1,9 @@
>  # SPDX-License-Identifier GPL-2.0+
>  ---
>  # Our sensible defaults for the nfsd role.
> +nfsd_lvm_pvs: []
> +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 e051d21bd819..fe5f6919864d 100644
> --- a/playbooks/roles/nfsd/tasks/main.yml
> +++ b/playbooks/roles/nfsd/tasks/main.yml
> @@ -22,13 +22,22 @@
>      group: root
>      mode: 0644
>  
> +- name: Build string of devices to use as PVs
> +  set_fact:
> +    nfsd_lvm_pvs: "{{ nfsd_lvm_pvs + [ nfsd_export_device_prefix + item|string ] }}"
> +  with_items: "{{ range(1, nfsd_export_device_count + 1) }}"
> +
> +- name: Print the PV list
> +  ansible.builtin.debug:
> +    var: nfsd_lvm_pvs
> +
>  - name: Create a new LVM VG
>    become: yes
>    become_flags: 'su - -c'
>    become_method: sudo
>    community.general.lvg:
>      vg: "exports"
> -    pvs: "{{ nfsd_export_device_0 }},{{ nfsd_export_device_1 }}"
> +    pvs: "{{ nfsd_lvm_pvs | join(',') }}"
>  
>  - name: Create {{ nfsd_export_path }}
>    become: yes
> diff --git a/scripts/nfsd.Makefile b/scripts/nfsd.Makefile
> index 40338ab93fce..d3549a29af84 100644
> --- a/scripts/nfsd.Makefile
> +++ b/scripts/nfsd.Makefile
> @@ -1,5 +1,5 @@
> -NFSD_EXTRA_ARGS += nfsd_export_device_0='$(subst ",,$(CONFIG_NFSD_EXPORT_DEVICE_0))'
> -NFSD_EXTRA_ARGS += nfsd_export_device_1='$(subst ",,$(CONFIG_NFSD_EXPORT_DEVICE_1))'
> +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))'
>  NFSD_EXTRA_ARGS += nfsd_export_fstype='$(subst ",,$(CONFIG_NFSD_EXPORT_FSTYPE))'
>  NFSD_EXTRA_ARGS += nfsd_export_path='$(subst ",,$(CONFIG_NFSD_EXPORT_PATH))'
>  NFSD_EXTRA_ARGS += nfsd_export_options='$(subst ",,$(CONFIG_NFSD_EXPORT_OPTIONS))'
> -- 
> 2.43.2
> 
> 

-- 
Chuck Lever