Skip to content

Commit 23193f0

Browse files
committed
guestfs: replace ansible group permisison requirement on libvirt system uri
The bringup process for libvirt system URI support (not session), so all debian based distros, requieres us to be paranoid about the permissions of our storage directory where we place our libvirt storage pool, and guestfs images. We used to be stupid and were hammering with a sledge hammer a crazy sudo chown -R on a target storage path. That was removed by commit c31459d ("scripts/bringup_guestfs.sh: fix silly directory permission fix"). I rushed that change in because it was affecting live systems and we needed to get testing moving. This replaces the old requirement with some more less aggressive directory creation and directory permission requirements and adds some sanity checks which don't do the crazy wild permission changes that are possible with a recursive call. Signed-off-by: Luis Chamberlain <[email protected]>
1 parent 95a1acf commit 23193f0

File tree

4 files changed

+87
-4
lines changed

4 files changed

+87
-4
lines changed

kconfigs/Kconfig.guestfs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ config STORAGE_POOL_PATH
55
output yaml
66
default LIBVIRT_STORAGE_POOL_PATH
77

8+
config GUESTFS_STORAGE_DIR
9+
string
10+
output yaml
11+
default "{{ kdevops_storage_pool_path }}/kdevops/guestfs"
12+
13+
config GUESTFS_BASE_IMAGE_DIR
14+
string
15+
output yaml
16+
default "{{ guestfs_storage_dir }}/base_images"
17+
818
config GUESTFS_HAS_CUSTOM_RAW_IMAGE
919
bool
1020

playbooks/roles/bringup_guestfs/tasks/main.yml

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,82 @@
4242
when: guestfs_subdirectories.matched == 0
4343
tags: [ 'config-check' ]
4444

45+
- name: Create storage pool path directory if (libvirt session uri)
46+
file:
47+
path: "{{ libvirt_storage_pool_path }}"
48+
state: directory
49+
when: 'not libvirt_uri_system|bool'
50+
tags: ['storage-pool-path']
51+
52+
- name: Create storage pool path directory and set group if using (libvirt system uri)
53+
file:
54+
path: "{{ libvirt_storage_pool_path }}"
55+
state: directory
56+
owner: root
57+
group: "{{ libvirt_qemu_group }}"
58+
mode: "0775"
59+
when: 'libvirt_uri_system|bool'
60+
tags: ['storage-pool-path']
61+
62+
- name: Create kdevops guestfs storage directory if missing (libvirt session uri)
63+
file:
64+
path: "{{ guestfs_base_image_dir }}"
65+
state: directory
66+
mode: '0755'
67+
tags: ['storage-pool-path']
68+
when:
69+
- 'not libvirt_uri_system|bool'
70+
71+
- name: Create kdevops guestfs storage directory if missing (libvirt system uri)
72+
become: yes
73+
become_flags: 'su - -c'
74+
become_method: sudo
75+
file:
76+
path: "{{ guestfs_base_image_dir }}"
77+
state: directory
78+
mode: '0775'
79+
group: "{{ libvirt_qemu_group }}"
80+
tags: ['storage-pool-path']
81+
when:
82+
- 'libvirt_uri_system|bool'
83+
84+
- name: Check if directory is owned by the correct group (libvirt system uri)
85+
become: yes
86+
become_flags: 'su - -c'
87+
become_method: sudo
88+
command: stat -c '%G' "{{ libvirt_storage_pool_path }}"
89+
register: dir_group
90+
changed_when: false
91+
tags: ['storage-pool-path']
92+
when:
93+
- 'libvirt_uri_system|bool'
94+
95+
- name: Check if directory has group write permissions (libvirt system uri)
96+
become: yes
97+
become_flags: 'su - -c'
98+
become_method: sudo
99+
command: stat -c '%A' "{{ libvirt_storage_pool_path }}"
100+
register: dir_perms
101+
changed_when: false
102+
tags: ['storage-pool-path']
103+
when:
104+
- 'libvirt_uri_system|bool'
105+
106+
- name: Verify storage pool path directory is group-writable (libvirt system uri)
107+
become: yes
108+
become_flags: 'su - -c'
109+
become_method: sudo
110+
fail:
111+
msg: |
112+
The permissions for {{ libvirt_storage_pool_path }} should be group
113+
writeable by the group used by libvirt: {{ libvirt_qemu_group }}
114+
Current group: {{ dir_group.stdout }}
115+
Current permissions: {{ dir_perms.stdout }}
116+
tags: ['storage-pool-path']
117+
when:
118+
- 'libvirt_uri_system|bool'
119+
- (dir_group.stdout != libvirt_qemu_group) or (dir_perms.stdout[5] != 'w')
120+
45121
- name: Check for dnsmasq configuration files
46122
stat:
47123
path: "{{ item }}"

scripts/bringup_guestfs.sh

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,6 @@ if [[ "$CONFIG_LIBVIRT_URI_SYSTEM" == "y" ]]; then
271271
USE_SUDO="sudo "
272272
fi
273273

274-
$USE_SUDO mkdir -p $STORAGEDIR
275-
$USE_SUDO mkdir -p $BASE_IMAGE_DIR
276-
277274
cmdfile=$(mktemp)
278275

279276
if [ ! -f $BASE_IMAGE ]; then

scripts/guestfs.Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ bringup_guestfs: $(GUESTFS_BRINGUP_DEPS)
8383
playbooks/bringup_guestfs.yml \
8484
-e 'ansible_python_interpreter=/usr/bin/python3' \
8585
--extra-vars=@./extra_vars.yaml \
86-
--tags config-check,network
86+
--tags config-check,network,storage-pool-path
8787
$(Q)$(TOPDIR)/scripts/bringup_guestfs.sh
8888
$(Q)ansible-playbook $(ANSIBLE_VERBOSE) --connection=local \
8989
--inventory localhost, \

0 commit comments

Comments
 (0)