Re: [PATCH v2 29/37] guestfs: Configure how many extra drives to provision

On 9/2/25 4:19 PM, Daniel Gomez wrote:
> On 02/09/2025 15.54, Chuck Lever wrote:
>> From: Chuck Lever <[email protected]>
>>
>> Some workflows create several guests but use only one or two extra
>> storage drives per guest. Enable configurations that reduce the
>> amount of storage allocated per guest to conserve local persistent
>> storage space on the host.
>>
>> Signed-off-by: Chuck Lever <[email protected]>
> 
> This should allow workflows to specify the nr of drives needed, right?
> 
> Looks great optimization, thanks!
> 
> Reviewed-by: Daniel Gomez <[email protected]>
> 
>> ---
>>  kconfigs/Kconfig.libvirt                      | 45 +++++++++++++++++++
>>  .../roles/gen_nodes/templates/gen_drives.j2   |  8 ++--
>>  .../roles/guestfs/tasks/bringup/main.yml      |  2 +-
>>  3 files changed, 50 insertions(+), 5 deletions(-)
>>
>> diff --git a/kconfigs/Kconfig.libvirt b/kconfigs/Kconfig.libvirt
>> index 7fe231668229..961276ccc1b1 100644
>> --- a/kconfigs/Kconfig.libvirt
>> +++ b/kconfigs/Kconfig.libvirt
>> @@ -556,6 +556,51 @@ config LIBVIRT_HOST_PASSTHROUGH
>>  	      --pre 'make -s mrproper defconfig' \
>>  	    \-- make -s -j$(nproc) bzImage
>>  
>> +choice
>> +	prompt "Libvirt extra storage drive count"
>> +	default LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_4
> 
> I see we are defaulting to what we had. So it's up to the user or defconfig to
> leverage this.
> 
>> +	help
>> +	  This option selects the number of extra storage drives to
>> +	  provision for each libvirt guest. Reduce this number to
>> +	  conserve local host storage capacity, if your workflows do
>> +	  not use all the extra drives.
>> +
>> +config LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_0
>> +	bool "0"
>> +	help
>> +	  Provision no extra storage drives per guest.
> 
> Curious, any reason to have a menu/choice instead of int?

This is the way drive count is handled in the terraform provider menus.
So, the reason is "consistency with existing input methods", no other
reason.


>> +
>> +config LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_1
>> +	bool "1"
>> +	help
>> +	  Provision one extra storage drive per guest.
>> +
>> +config LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_2
>> +	bool "2"
>> +	help
>> +	  Provision two extra storage drives per guest.
>> +
>> +config LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_3
>> +	bool "3"
>> +	help
>> +	  Provision three extra storage drives per guest.
>> +
>> +config LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_4
>> +	bool "4"
>> +	help
>> +	  Provision four extra storages drive per guest.
>> +
>> +endchoice
>> +
>> +config LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT
>> +	int
>> +	output yaml
>> +	default 0 if LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_0
>> +	default 1 if LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_1
>> +	default 2 if LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_2
>> +	default 3 if LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_3
>> +	default 4 if LIBVIRT_EXTRA_STORAGE_DRIVE_COUNT_4
>> +
>>  choice
>>  	prompt "Libvirt extra storage driver to use"
>>  	default LIBVIRT_EXTRA_STORAGE_DRIVE_VIRTIO
>> diff --git a/playbooks/roles/gen_nodes/templates/gen_drives.j2 b/playbooks/roles/gen_nodes/templates/gen_drives.j2
>> index 2de13da4ab8e..1040c0e19d84 100644
>> --- a/playbooks/roles/gen_nodes/templates/gen_drives.j2
>> +++ b/playbooks/roles/gen_nodes/templates/gen_drives.j2
>> @@ -1,6 +1,6 @@
>>  {% import './templates/drives.j2' as drives %}
>>  {% if libvirt_extra_storage_drive_ide %}
>> -{{ drives.gen_drive_ide(4,
>> +{{ drives.gen_drive_ide(libvirt_extra_storage_drive_count,
>>  			kdevops_storage_pool_path,
>>  			hostname,
>>  			libvirt_extra_drive_format,
>> @@ -18,7 +18,7 @@
>>  				    libvirt_extra_storage_aio_cache_mode,
>>  				    kdevops_storage_pool_path) }}
>>  {% else %}
>> -{{ drives.gen_drive_virtio(4,
>> +{{ drives.gen_drive_virtio(libvirt_extra_storage_drive_count,
>>                             kdevops_storage_pool_path,
>>                             hostname,
>>                             libvirt_extra_drive_format,
>> @@ -28,7 +28,7 @@
>>                             libvirt_extra_storage_virtio_physical_block_size) }}
>>  {% endif %}
>>  {% elif libvirt_extra_storage_drive_scsi %}
>> -{{ drives.gen_drive_scsi(4,
>> +{{ drives.gen_drive_scsi(libvirt_extra_storage_drive_count,
>>                           kdevops_storage_pool_path,
>>                           hostname,
>>                           libvirt_extra_drive_format,
>> @@ -46,7 +46,7 @@
>>  				  libvirt_extra_storage_aio_cache_mode,
>>  				  kdevops_storage_pool_path) }}
>>  {% else %}
>> -{{ drives.gen_drive_nvme(4,
>> +{{ drives.gen_drive_nvme(libvirt_extra_storage_drive_count,
>>  			 kdevops_storage_pool_path,
>>  			 hostname,
>>  			 libvirt_extra_drive_format,
>> diff --git a/playbooks/roles/guestfs/tasks/bringup/main.yml b/playbooks/roles/guestfs/tasks/bringup/main.yml
>> index bd9f52603a3b..3fd677ff4659 100644
>> --- a/playbooks/roles/guestfs/tasks/bringup/main.yml
>> +++ b/playbooks/roles/guestfs/tasks/bringup/main.yml
>> @@ -102,7 +102,7 @@
>>          path: "{{ storagedir }}/{{ inventory_hostname }}/extra{{ item }}.{{ libvirt_extra_drive_format }}"
>>        ansible.builtin.include_tasks:
>>          file: "{{ role_path }}/tasks/bringup/extra-disks.yml"
>> -      loop: "{{ range(0, 4) | list }}"
>> +      loop: "{{ range(0, libvirt_extra_storage_drive_count) | list }}"
>>        when:
>>          - not libvirt_enable_largeio|bool
>>  


-- 
Chuck Lever