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

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Fri, 2025-08-15 at 10:47 +0200, Daniel Gomez wrote:
> 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".
> 

I have no idea if this modification is necessary or not. :) If you say
that it's complete crap, then I will be completely agreed. :) I simply
tried to make the first run of kdevops during last 1.5 month without
any success. Then, I gave up and I decided to ask Claude to help me.
This stuff has made this modification and I can run kdevops now.

I have no idea what I can add to the commit message. If you would like
to reject the patch, I am completely OK. If you would like to see
another commit message, then I need some help. Because, I have no idea
what this modification is about. :) Welcome to the AI world. :)

Thanks,
Slava.

> > 
> > 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>