[PATCH] meson: Add back prefix path for runstatedir

Zhenzhong Duan posted 1 patch 8 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20250402075828.3004336-1-zhenzhong.duan@intel.com
There is a newer version of this series
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] meson: Add back prefix path for runstatedir
Posted by Zhenzhong Duan 8 months, 2 weeks ago
Currently libvirt favors /run instead of /var/run, but for local build
run test, a prefix path is still needed to avoid interoperating with OS
vendor provided binaries.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index bf4a245dd3..84c9796c2f 100644
--- a/meson.build
+++ b/meson.build
@@ -82,7 +82,7 @@ endif
 
 runstatedir = get_option('runstatedir')
 if runstatedir == ''
-  runstatedir = '/run'
+  runstatedir = prefix / 'run'
 endif
 
 initconfdir = get_option('initconfdir')
-- 
2.34.1
Re: [PATCH] meson: Add back prefix path for runstatedir
Posted by Peter Krempa via Devel 8 months, 2 weeks ago
On Wed, Apr 02, 2025 at 15:58:28 +0800, Zhenzhong Duan wrote:
> Currently libvirt favors /run instead of /var/run, but for local build
> run test, a prefix path is still needed to avoid interoperating with OS
> vendor provided binaries.

Could you please elaborate?

> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index bf4a245dd3..84c9796c2f 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -82,7 +82,7 @@ endif
>  
>  runstatedir = get_option('runstatedir')
>  if runstatedir == ''
> -  runstatedir = '/run'
> +  runstatedir = prefix / 'run'

If you need anything non-default you can always set it via
-Drunstatedir= when configuring the project.

>  endif
>  
>  initconfdir = get_option('initconfdir')
> -- 
> 2.34.1
>
RE: [PATCH] meson: Add back prefix path for runstatedir
Posted by Duan, Zhenzhong 8 months, 2 weeks ago

>-----Original Message-----
>From: Peter Krempa <pkrempa@redhat.com>
>Subject: Re: [PATCH] meson: Add back prefix path for runstatedir
>
>On Wed, Apr 02, 2025 at 15:58:28 +0800, Zhenzhong Duan wrote:
>> Currently libvirt favors /run instead of /var/run, but for local build
>> run test, a prefix path is still needed to avoid interoperating with OS
>> vendor provided binaries.
>
>Could you please elaborate?

The OS vendor provided binaries generates sockets in /run/libvirt/,
The local run './build/src/libvirtd -d' also generates sockets in /run/libvirt/.

If prefix path is used, local run generates sockets in /root/libvirt_build/run/libvirt/,
then there is no conflict.

>
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  meson.build | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/meson.build b/meson.build
>> index bf4a245dd3..84c9796c2f 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -82,7 +82,7 @@ endif
>>
>>  runstatedir = get_option('runstatedir')
>>  if runstatedir == ''
>> -  runstatedir = '/run'
>> +  runstatedir = prefix / 'run'
>
>If you need anything non-default you can always set it via
>-Drunstatedir= when configuring the project.

Yes, I have to specify it explicitly:

meson build --prefix /root/libvirt_build -Drunstatedir=/root/libvirt_build/var/run

In early days, I remember no need to use -Drunstatedir,
'meson build --prefix /root/libvirt_build' just work for me.

Thanks
Zhenzhong
Re: [PATCH] meson: Add back prefix path for runstatedir
Posted by Pavel Hrdina via Devel 8 months, 2 weeks ago
On Wed, Apr 02, 2025 at 08:33:23AM +0000, Duan, Zhenzhong wrote:
> 
> 
> >-----Original Message-----
> >From: Peter Krempa <pkrempa@redhat.com>
> >Subject: Re: [PATCH] meson: Add back prefix path for runstatedir
> >
> >On Wed, Apr 02, 2025 at 15:58:28 +0800, Zhenzhong Duan wrote:
> >> Currently libvirt favors /run instead of /var/run, but for local build
> >> run test, a prefix path is still needed to avoid interoperating with OS
> >> vendor provided binaries.
> >
> >Could you please elaborate?
> 
> The OS vendor provided binaries generates sockets in /run/libvirt/,
> The local run './build/src/libvirtd -d' also generates sockets in /run/libvirt/.
> 
> If prefix path is used, local run generates sockets in /root/libvirt_build/run/libvirt/,
> then there is no conflict.
> 
> >
> >>
> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> >> ---
> >>  meson.build | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/meson.build b/meson.build
> >> index bf4a245dd3..84c9796c2f 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -82,7 +82,7 @@ endif
> >>
> >>  runstatedir = get_option('runstatedir')
> >>  if runstatedir == ''
> >> -  runstatedir = '/run'
> >> +  runstatedir = prefix / 'run'
> >
> >If you need anything non-default you can always set it via
> >-Drunstatedir= when configuring the project.
> 
> Yes, I have to specify it explicitly:
> 
> meson build --prefix /root/libvirt_build -Drunstatedir=/root/libvirt_build/var/run
> 
> In early days, I remember no need to use -Drunstatedir,
> 'meson build --prefix /root/libvirt_build' just work for me.

Agreed, this is a regression introduced by commit
e5299ddf86121d3c792ca271ffcb54900eb19dc3 that unconditionally sets
runstatedir to `/run` which is wrong.

The patch needs to be improved as we have special option `system` that
forces some paths to be the same as when libvirt is installed using OS
packages.

In the meson file you can see `if get_option('system')` few lines above,
we need to set runstatedir to `/run` there and we can move the part you
are changing in this patch to the else branch.

Pavel
RE: [PATCH] meson: Add back prefix path for runstatedir
Posted by Duan, Zhenzhong 8 months, 2 weeks ago

>-----Original Message-----
>From: Pavel Hrdina <phrdina@redhat.com>
>Subject: Re: [PATCH] meson: Add back prefix path for runstatedir
>
>On Wed, Apr 02, 2025 at 08:33:23AM +0000, Duan, Zhenzhong wrote:
>>
>>
>> >-----Original Message-----
>> >From: Peter Krempa <pkrempa@redhat.com>
>> >Subject: Re: [PATCH] meson: Add back prefix path for runstatedir
>> >
>> >On Wed, Apr 02, 2025 at 15:58:28 +0800, Zhenzhong Duan wrote:
>> >> Currently libvirt favors /run instead of /var/run, but for local build
>> >> run test, a prefix path is still needed to avoid interoperating with OS
>> >> vendor provided binaries.
>> >
>> >Could you please elaborate?
>>
>> The OS vendor provided binaries generates sockets in /run/libvirt/,
>> The local run './build/src/libvirtd -d' also generates sockets in /run/libvirt/.
>>
>> If prefix path is used, local run generates sockets in
>/root/libvirt_build/run/libvirt/,
>> then there is no conflict.
>>
>> >
>> >>
>> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> >> ---
>> >>  meson.build | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/meson.build b/meson.build
>> >> index bf4a245dd3..84c9796c2f 100644
>> >> --- a/meson.build
>> >> +++ b/meson.build
>> >> @@ -82,7 +82,7 @@ endif
>> >>
>> >>  runstatedir = get_option('runstatedir')
>> >>  if runstatedir == ''
>> >> -  runstatedir = '/run'
>> >> +  runstatedir = prefix / 'run'
>> >
>> >If you need anything non-default you can always set it via
>> >-Drunstatedir= when configuring the project.
>>
>> Yes, I have to specify it explicitly:
>>
>> meson build --prefix /root/libvirt_build -Drunstatedir=/root/libvirt_build/var/run
>>
>> In early days, I remember no need to use -Drunstatedir,
>> 'meson build --prefix /root/libvirt_build' just work for me.
>
>Agreed, this is a regression introduced by commit
>e5299ddf86121d3c792ca271ffcb54900eb19dc3 that unconditionally sets
>runstatedir to `/run` which is wrong.
>
>The patch needs to be improved as we have special option `system` that
>forces some paths to be the same as when libvirt is installed using OS
>packages.
>
>In the meson file you can see `if get_option('system')` few lines above,
>we need to set runstatedir to `/run` there and we can move the part you
>are changing in this patch to the else branch.

Good idea, will do.

Thanks
Zhenzhong