Re: [RFC PATCH] storage-subsystem: correct playbooks for Fedora distro

[Daniel Gomez <da.gomez@kernel.org>]

On 14/08/2025 12.09, Viacheslav Dubeyko wrote:
> From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> 
> Claude AI helped to make the first kdevops run successful
> under Fedora Linux 42 (Workstation Edition). This patch
> contains modifications suggested by Claude AI. It modifies
> playbooks/roles/guestfs/tasks/bringup/extra-disks.yml and
> playbooks/roles/guestfs/tasks/bringup/storage-pool-path.yml.

Can you elaborate the commit message explaining why is this needed?

I can see the code below, but adding that to the commit message makes it easier
to follow. Also for git history I'd say it's "mandatory".

> 
> Generated-by: Claude AI
> Signed-off-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> ---
>  playbooks/roles/guestfs/tasks/bringup/extra-disks.yml | 11 ++++++++++-
>  .../roles/guestfs/tasks/bringup/storage-pool-path.yml |  1 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/playbooks/roles/guestfs/tasks/bringup/extra-disks.yml b/playbooks/roles/guestfs/tasks/bringup/extra-disks.yml
> index 438b65e..a2a4f84 100644
> --- a/playbooks/roles/guestfs/tasks/bringup/extra-disks.yml
> +++ b/playbooks/roles/guestfs/tasks/bringup/extra-disks.yml
> @@ -9,8 +9,17 @@
>        - "{{ path }}"
>        - "100G"
>  
> -- name: Update the permission settings of the drive image file
> +- name: Update the permission settings of the drive image file (libvirt system uri)
>    ansible.builtin.file:
>      path: "{{ path }}"
>      group: "{{ libvirt_qemu_group }}"
>      mode: "g+rw,o-rw"
> +  when:
> +    - libvirt_uri_system|bool
> +
> +- name: Update the permission settings of the drive image file (libvirt session uri)
> +  ansible.builtin.file:
> +    path: "{{ path }}"
> +    mode: "u+rw,g+r,o-rw"
> +  when:
> +    - not libvirt_uri_system|bool

Do you need o-rw in session mode? And is the group field missing intentional
[1]?

Link: https://docs.ansible.com/ansible/latest/collections/ansible/builtin/file_module.html#parameter-group [1]


> diff --git a/playbooks/roles/guestfs/tasks/bringup/storage-pool-path.yml b/playbooks/roles/guestfs/tasks/bringup/storage-pool-path.yml
> index 385b6cd..7a4bbbc 100644
> --- a/playbooks/roles/guestfs/tasks/bringup/storage-pool-path.yml
> +++ b/playbooks/roles/guestfs/tasks/bringup/storage-pool-path.yml
> @@ -11,6 +11,7 @@
>    ansible.builtin.fail:
>      msg: "Ansible user is not part of the {{ libvirt_qemu_group }} group."
>    when:
> +    - libvirt_uri_system|bool
>      - libvirt_qemu_group not in user_groups
>  
>  - name: Create storage pool path directory (libvirt session uri)

LGTM. With the changes/questions addressed above.

Reviewed-by: Daniel Gomez <da.gomez@samsung.com>