Re: [libvirt] [PATCH 1/2] virt-aa-helper: add rules for shmem devices

On Tue, Nov 19, 2019 at 10:25 PM Jamie Strandboge <jamie(a)canonical.com&gt; wrote: > On Tue, 22 Oct 2019, Christian Ehrhardt wrote: > > + for (i = 0; i < ctl->def->nshmems; i++) { > > + if (ctl->def->shmems[i]) { > > + virDomainShmemDef *shmem = ctl->def->shmems[i]; > > + /* server path can be on any type and overwrites defaults */ > > + if (shmem->server.enabled && > > + shmem->server.chr.data.nix.path) { > > + if (vah_add_file(&buf, shmem->server.chr.data.nix.path, > > + "rw") != 0) > > + goto cleanup; > > I'll let others comment on the code changes, but this apparmor rule > looks ok. > > > + } else { > > That said, I wonder about the logic here since up above we have: > > if (shmem->server.enabled && shmem->server.chr.data.nix.path) > > but here we just have 'else'. What happens if server.enabled is false > and server.chr.data.nix.path is set or vice versa? Does this 'else' > clause correctly handle those scenarios? Yes if either of the above isn't fulfilled it will fall back to use the default paths. So a single else without other checks should be correct. The following switch then differs the default paths used base on the model.