On Mon, Apr 06, 2026 at 18:16:56 -0400, Cole Robinson via Devel wrote:
> The 0777 permission mask we use when creating most 'cfg' dirs
> does not match what we put on disk via the RPM spec file. Generally
> those dirs are more locked down.
> 
> Match driver startup permissions and owners with what we
> encode in the RPM spec. Presumably this is safe because this has
> been tested with real world usage.

The question may be whether packagers from other distros agree. Although
I agree that the permissions are a bit saner the way you've changed
them.

> Some dirs are created here but not in the RPM spec. Leave their
> permission mask as is, we will deal with them in future patches.
> 
> The 2 runtime changes for an RPM installed libvirt is that
> stateDir is now chown(qemu, qemu) and runDir is now chown(0, 0)
> where previously there was no chown() calls for these dirs.
> I don't think that should cause problems
> 
> Signed-off-by: Cole Robinson <[email protected]>
> ---
>  src/qemu/qemu_driver.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5dff049d85..f351aab009 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -524,6 +524,9 @@ qemuStateInitializeDirs(bool privileged,
>  {
>      size_t i;
>  
> +    uid_t daemon_uid = geteuid();
> +    gid_t daemon_gid = getegid();
> +
>      struct dirperms {
>          const char *dir;
>          int mode;
> @@ -532,24 +535,24 @@ qemuStateInitializeDirs(bool privileged,
>      };
>      struct dirperms dirs[] = {
>          /* example: /var/cache/libvirt/qemu */
> -        { cfg->cacheDir, 0777, -1, -1 },
> +        { cfg->cacheDir, 0750, daemon_uid, daemon_gid },

/var/cache/libvirt is declared as 711 in libvirt.spec.in

[...]

To me this looks good, but maybe leave some time for packagers of other
distros to have opportunity to chime in.

Reviewed-by: Peter Krempa <[email protected]>

Reply via email to