On Mon, Apr 06, 2026 at 18:16:55 -0400, Cole Robinson via Devel wrote:
> Once upon a time dirname(cfg->channelTargetDir) was a unique dir,
> but nowadays it is the same as cfg->stateDir, so this is redundant.

The commit message is a bit light on the justification for this change.

historically this was in

 /var/lib/libvirt/qemu/channel/target/

thus it was supposed to also chown the 'channel' directory to the
appropriate mode.

Now commit d3759d3674ab9453e5fb5a27ab6c28b8ff8d5569 moved it to:

 /var/lib/libvirt/qemu/channel/

and subsequently commit 8abc979bb09ca4b93123e8f75f3d28cc421a0bb6 moved
it to:

 /var/run/libvirt/qemu/channel/


thus currently it chowns /var/run/libvirt/qemu.


I'd say this change would make sense to happen before the commit which
refactors it to ...

> 
> Signed-off-by: Cole Robinson <[email protected]>
> ---
>  src/qemu/qemu_driver.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0cf88b8be9..5dff049d85 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -523,7 +523,6 @@ qemuStateInitializeDirs(bool privileged,
>                          virQEMUDriverConfig *cfg)
>  {
>      size_t i;
> -    g_autofree char *channeldir = g_path_get_dirname(cfg->channelTargetDir);
>  
>      struct dirperms {
>          const char *dir;
> @@ -540,7 +539,6 @@ qemuStateInitializeDirs(bool privileged,
>          { cfg->slirpStateDir, 0777, cfg->user, cfg->group },
>          { cfg->passtStateDir, 0777, cfg->user, cfg->group },
>          { cfg->rdpStateDir, 0777, cfg->user, cfg->group },
> -        { channeldir, 0777, cfg->user, cfg->group },

.. the array syntax.

>          { cfg->channelTargetDir, 0777, cfg->user, cfg->group },

I'd even say that this'd warant a:

Fixes: d3759d3674ab9453e5fb5a27ab6c28b8ff8d5569

although it doesn't have a functional difference so it's not needed.

If you move this before the refactor:

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

Reply via email to