diff mbox series

[4/6] NFSD: Implement NVMe storage for NFSD exports

Message ID 20241210192920.682914-5-cel@kernel.org (mailing list archive)
State New
Headers show
Series Recent fixes | expand

Commit Message

Chuck Lever Dec. 10, 2024, 7:29 p.m. UTC 
From: Chuck Lever <chuck.lever@oracle.com>

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 kconfigs/Kconfig.nfsd                  | 12 +++++++---
 playbooks/roles/nfsd/defaults/main.yml |  1 +
 playbooks/roles/nfsd/tasks/main.yml    | 31 +++++++++++++++++++++-----
 scripts/nfsd.Makefile                  |  4 ++++
 4 files changed, 39 insertions(+), 9 deletions(-)

Comments

Luis Chamberlain Dec. 11, 2024, 3:42 a.m. UTC | #1 
On Tue, Dec 10, 2024 at 02:29:17PM -0500, cel@kernel.org wrote:
> diff --git a/playbooks/roles/nfsd/tasks/main.yml b/playbooks/roles/nfsd/tasks/main.yml
> index 63388f857627..c7c41e79714b 100644
> --- a/playbooks/roles/nfsd/tasks/main.yml
> +++ b/playbooks/roles/nfsd/tasks/main.yml
> @@ -38,15 +38,34 @@
>    when:
>      - nfsd_export_storage_local|bool
>  
> -- name: Create a new LVM VG
> -  become: yes
> -  become_flags: 'su - -c'
> -  become_method: sudo
> +- name: Enumerate NVMe devices to provision as PVs
> +  become: true
> +  become_flags: "su - -c"
> +  become_method: ansible.builtin.sudo
> +  ansible.builtin.command:
> +    cmd: "lsblk -np -o PATH,SERIAL"
> +  register: lsblk_output
> +  changed_when: false
> +  when:
> +    - nfsd_export_storage_nvme|bool
> +
> +- name: Build a list of NVMe devices to provision as PVs
> +  ansible.builtin.set_fact:
> +    nfsd_lvm_pvs: "{{ nfsd_lvm_pvs + [item | split() | first] }}"
> +  loop: "{{ lsblk_output.stdout_lines }}"
> +  when:
> +    - nfsd_export_storage_nvme|bool
> +    - '"AWS" in item'

I think you may want to add a new output yaml for when the provider is
AWS so you get devops_terraform_provider_is_aws so you can just check
for that here as an extra bool. That will ensure with more determinism
that the bringup is on AWS.

  Luis
Chuck Lever Dec. 11, 2024, 1:50 p.m. UTC | #2 
On 12/10/24 10:42 PM, Luis Chamberlain wrote:
> On Tue, Dec 10, 2024 at 02:29:17PM -0500, cel@kernel.org wrote:
>> diff --git a/playbooks/roles/nfsd/tasks/main.yml b/playbooks/roles/nfsd/tasks/main.yml
>> index 63388f857627..c7c41e79714b 100644
>> --- a/playbooks/roles/nfsd/tasks/main.yml
>> +++ b/playbooks/roles/nfsd/tasks/main.yml
>> @@ -38,15 +38,34 @@
>>     when:
>>       - nfsd_export_storage_local|bool
>>   
>> -- name: Create a new LVM VG
>> -  become: yes
>> -  become_flags: 'su - -c'
>> -  become_method: sudo
>> +- name: Enumerate NVMe devices to provision as PVs
>> +  become: true
>> +  become_flags: "su - -c"
>> +  become_method: ansible.builtin.sudo
>> +  ansible.builtin.command:
>> +    cmd: "lsblk -np -o PATH,SERIAL"
>> +  register: lsblk_output
>> +  changed_when: false
>> +  when:
>> +    - nfsd_export_storage_nvme|bool
>> +
>> +- name: Build a list of NVMe devices to provision as PVs
>> +  ansible.builtin.set_fact:
>> +    nfsd_lvm_pvs: "{{ nfsd_lvm_pvs + [item | split() | first] }}"
>> +  loop: "{{ lsblk_output.stdout_lines }}"
>> +  when:
>> +    - nfsd_export_storage_nvme|bool
>> +    - '"AWS" in item'
> 
> I think you may want to add a new output yaml for when the provider is
> AWS so you get devops_terraform_provider_is_aws so you can just check
> for that here as an extra bool. That will ensure with more determinism
> that the bringup is on AWS.

Apologies for asking, but do you have a particular commit ID or file I
could use as an example?
Luis Chamberlain Dec. 12, 2024, 2:28 a.m. UTC | #3 
On Wed, Dec 11, 2024 at 08:50:03AM -0500, Chuck Lever wrote:
> On 12/10/24 10:42 PM, Luis Chamberlain wrote:
> > On Tue, Dec 10, 2024 at 02:29:17PM -0500, cel@kernel.org wrote:
> > > diff --git a/playbooks/roles/nfsd/tasks/main.yml b/playbooks/roles/nfsd/tasks/main.yml
> > > index 63388f857627..c7c41e79714b 100644
> > > --- a/playbooks/roles/nfsd/tasks/main.yml
> > > +++ b/playbooks/roles/nfsd/tasks/main.yml
> > > @@ -38,15 +38,34 @@
> > >     when:
> > >       - nfsd_export_storage_local|bool
> > > -- name: Create a new LVM VG
> > > -  become: yes
> > > -  become_flags: 'su - -c'
> > > -  become_method: sudo
> > > +- name: Enumerate NVMe devices to provision as PVs
> > > +  become: true
> > > +  become_flags: "su - -c"
> > > +  become_method: ansible.builtin.sudo
> > > +  ansible.builtin.command:
> > > +    cmd: "lsblk -np -o PATH,SERIAL"
> > > +  register: lsblk_output
> > > +  changed_when: false
> > > +  when:
> > > +    - nfsd_export_storage_nvme|bool
> > > +
> > > +- name: Build a list of NVMe devices to provision as PVs
> > > +  ansible.builtin.set_fact:
> > > +    nfsd_lvm_pvs: "{{ nfsd_lvm_pvs + [item | split() | first] }}"
> > > +  loop: "{{ lsblk_output.stdout_lines }}"
> > > +  when:
> > > +    - nfsd_export_storage_nvme|bool
> > > +    - '"AWS" in item'
> > 
> > I think you may want to add a new output yaml for when the provider is
> > AWS so you get devops_terraform_provider_is_aws so you can just check
> > for that here as an extra bool. That will ensure with more determinism
> > that the bringup is on AWS.
> 
> Apologies for asking, but do you have a particular commit ID or file I
> could use as an example?

I mean like this:

diff --git a/terraform/Kconfig.providers b/terraform/Kconfig.providers
index abe91511b949..3fd886b1284b 100644
--- a/terraform/Kconfig.providers
+++ b/terraform/Kconfig.providers
@@ -5,6 +5,7 @@ choice
 config TERRAFORM_GCE
 	bool "GCE - Google Cloud Engine"
 	depends on TARGET_ARCH_X86_64
+	output yaml
 	help
 	  Enabling this means you are going to use GCE for your cloud solution.
 
@@ -18,6 +19,7 @@ config TERRAFORM_AZURE
 	bool "Azure"
 	depends on TARGET_ARCH_X86_64
 	select TERRAFORM_PRIVATE_NET
+	output yaml
 	help
 	  Enabling this means you are going to use Azure for your cloud
 	  solution. Note that Azure requires a private network for the
@@ -26,12 +28,14 @@ config TERRAFORM_AZURE
 config TERRAFORM_OCI
 	bool "OCI - Oracle Cloud Infrastructure"
 	depends on TARGET_ARCH_X86_64
+	output yaml
 	help
 	  Enabling this means you are going to use OCI for your cloud
 	  solution.
 
 config TERRAFORM_OPENSTACK
 	bool "OpenStack"
+	output yaml
 	help
 	  Enabling this means you are going to use OpenStack for your cloud
 	  solution.

Then with this your task becomes more deterministic like this:

- name: Build a list of NVMe devices to provision as PVs
  ansible.builtin.set_fact:
    nfsd_lvm_pvs: "{{ nfsd_lvm_pvs + [item | split() | first] }}"
  loop: "{{ lsblk_output.stdout_lines }}"
  when:
   - nfsd_export_storage_nvme|bool
   - terraform_aws|bool 
   - '"AWS" in item'
diff mbox series

Patch

diff --git a/kconfigs/Kconfig.nfsd b/kconfigs/Kconfig.nfsd
index d071f5fba278..0cc671b396df 100644
--- a/kconfigs/Kconfig.nfsd
+++ b/kconfigs/Kconfig.nfsd
@@ -67,14 +67,14 @@  config NFSD_LEASE_TIME
 	  complete faster.
 
 choice
-	prompt "Local or external physical storage"
+	prompt "Persistent storage for exported file systems"
 	default NFSD_EXPORT_STORAGE_LOCAL
 
 config NFSD_EXPORT_STORAGE_LOCAL
 	bool "Local"
 	help
 	  Exported file systems will reside on physical storage
-	  local to the NFS server itself.
+	  devices local to the NFS server itself.
 
 config NFSD_EXPORT_STORAGE_ISCSI
 	bool "iSCSI"
@@ -83,12 +83,18 @@  config NFSD_EXPORT_STORAGE_ISCSI
 	  located on a separate target node and accessed via
 	  iSCSI.
 
+config NFSD_EXPORT_STORAGE_NVME
+	bool "NVMe"
+	help
+	  Exported file systems will reside in NVMe namespaces
+	  local to the NFS server itself.
+
 endchoice
 
 if NFSD_EXPORT_STORAGE_LOCAL
 
 config NFSD_EXPORT_DEVICE_PREFIX
-	string "The device prefix to use for LVM PVs"
+	string "The block device name 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
diff --git a/playbooks/roles/nfsd/defaults/main.yml b/playbooks/roles/nfsd/defaults/main.yml
index 271d2d1d8912..59e8d74f372b 100644
--- a/playbooks/roles/nfsd/defaults/main.yml
+++ b/playbooks/roles/nfsd/defaults/main.yml
@@ -10,5 +10,6 @@  nfsd_lease_time: "90"
 
 nfsd_export_storage_local: false
 nfsd_export_storage_iscsi: false
+nfsd_export_storage_nvme: false
 
 kdevops_krb5_enable: false
diff --git a/playbooks/roles/nfsd/tasks/main.yml b/playbooks/roles/nfsd/tasks/main.yml
index 63388f857627..c7c41e79714b 100644
--- a/playbooks/roles/nfsd/tasks/main.yml
+++ b/playbooks/roles/nfsd/tasks/main.yml
@@ -29,7 +29,7 @@ 
   when:
     - nfsd_export_storage_iscsi|bool
 
-- name: Build string of devices to use as PVs
+- name: Build a list of block devices to provision 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) }}"
@@ -38,15 +38,34 @@ 
   when:
     - nfsd_export_storage_local|bool
 
-- name: Create a new LVM VG
-  become: yes
-  become_flags: 'su - -c'
-  become_method: sudo
+- name: Enumerate NVMe devices to provision as PVs
+  become: true
+  become_flags: "su - -c"
+  become_method: ansible.builtin.sudo
+  ansible.builtin.command:
+    cmd: "lsblk -np -o PATH,SERIAL"
+  register: lsblk_output
+  changed_when: false
+  when:
+    - nfsd_export_storage_nvme|bool
+
+- name: Build a list of NVMe devices to provision as PVs
+  ansible.builtin.set_fact:
+    nfsd_lvm_pvs: "{{ nfsd_lvm_pvs + [item | split() | first] }}"
+  loop: "{{ lsblk_output.stdout_lines }}"
+  when:
+    - nfsd_export_storage_nvme|bool
+    - '"AWS" in item'
+
+- name: Create a new LVM Volume Group
+  become: true
+  become_flags: "su - -c"
+  become_method: ansible.builtin.sudo
   community.general.lvg:
     vg: "exports"
     pvs: "{{ nfsd_lvm_pvs | join(',') }}"
   when:
-    - nfsd_export_storage_local|bool
+    - nfsd_export_storage_local|bool or nfsd_export_storage_nvme|bool
 
 - name: Create {{ nfsd_export_path }}
   become: yes
diff --git a/scripts/nfsd.Makefile b/scripts/nfsd.Makefile
index 959cc4b7652d..b36fa25d915a 100644
--- a/scripts/nfsd.Makefile
+++ b/scripts/nfsd.Makefile
@@ -10,6 +10,10 @@  ifeq (y,$(CONFIG_NFSD_EXPORT_STORAGE_ISCSI))
 NFSD_EXTRA_ARGS += nfsd_export_storage_iscsi=true
 endif
 
+ifeq (y,$(CONFIG_NFSD_EXPORT_STORAGE_NVME))
+NFSD_EXTRA_ARGS += nfsd_export_storage_nvme=true
+endif
+
 ifeq (y,$(CONFIG_FSTESTS_NFS_SECTION_NFSD))
 NFSD_EXTRA_ARGS += kdevops_loopback_nfs_enable=true
 endif