1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/5] Don't touch user data from tests
  4. View patch

[libvirt] [PATCH v2 2/5] Forget last daemon/ dir artefacts

Michal Privoznik posted 5 patches 7 years, 7 months ago
There is a newer version of this series
[libvirt] [PATCH v2 2/5] Forget last daemon/ dir artefacts
Posted by Michal Privoznik 7 years, 7 months ago 
The most important part is LIBVIRTD_PATH env var fix. It is used
in virFileFindResourceFull() from tests. The libvirtd no longer
lives under daemon/.

Then, libvirtd-fail test was still failing (as expected) but not
because of missing config file but because it was trying to
execute (nonexistent) top_builddir/daemon/libvirtd which
fulfilled expected outcome and thus test did not fail.

Thirdly, lcov was told to generate coverage for daemon/ dir too.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 Makefile.am         | 2 +-
 run.in              | 2 +-
 tests/libvirtd-fail | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1926e21b7a..709064c6a6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -80,7 +80,7 @@ check-access:
 cov: clean-cov
 	$(MKDIR_P) $(top_builddir)/coverage
 	$(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \
-	  -d $(top_builddir)/src  -d $(top_builddir)/daemon \
+	  -d $(top_builddir)/src \
 	  -d $(top_builddir)/tests
 	$(LCOV) -r $(top_builddir)/coverage/libvirt.info.tmp \
 	  -o $(top_builddir)/coverage/libvirt.info
diff --git a/run.in b/run.in
index cbef61a674..06ad54b62b 100644
--- a/run.in
+++ b/run.in
@@ -63,7 +63,7 @@ export PKG_CONFIG_PATH
 export LIBVIRT_DRIVER_DIR="$b/src/.libs"
 export LIBVIRT_LOCK_MANAGER_PLUGIN_DIR="$b/src/.libs"
 export VIRTLOCKD_PATH="$b/src"
-export LIBVIRTD_PATH="$b/daemon"
+export LIBVIRTD_PATH="$b/src"
 
 # This is a cheap way to find some use-after-free and uninitialized
 # read problems when using glibc.
diff --git a/tests/libvirtd-fail b/tests/libvirtd-fail
index 6c61b892cb..f9e927b61f 100755
--- a/tests/libvirtd-fail
+++ b/tests/libvirtd-fail
@@ -5,12 +5,12 @@
 
 if test "$VERBOSE" = yes; then
   set -x
-  $abs_top_builddir/daemon/libvirtd --version
+  $abs_top_builddir/src/libvirtd --version
 fi
 
 fail=0
 
-$abs_top_builddir/daemon/libvirtd --config=no-such-conf --timeout=5 2> log
+$abs_top_builddir/src/libvirtd --config=no-such-conf --timeout=5 2> log
 RET=$?
 
 test "$RET" != "0" && exit 0 || exit 1
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/5] Forget last daemon/ dir artefacts
Posted by John Ferlan 7 years, 6 months ago 

On 07/12/2018 03:37 AM, Michal Privoznik wrote:
> The most important part is LIBVIRTD_PATH env var fix. It is used
> in virFileFindResourceFull() from tests. The libvirtd no longer
> lives under daemon/.
> 
> Then, libvirtd-fail test was still failing (as expected) but not
> because of missing config file but because it was trying to
> execute (nonexistent) top_builddir/daemon/libvirtd which
> fulfilled expected outcome and thus test did not fail.
> 
> Thirdly, lcov was told to generate coverage for daemon/ dir too.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  Makefile.am         | 2 +-
>  run.in              | 2 +-
>  tests/libvirtd-fail | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 1926e21b7a..709064c6a6 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -80,7 +80,7 @@ check-access:
>  cov: clean-cov
>  	$(MKDIR_P) $(top_builddir)/coverage
>  	$(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \
> -	  -d $(top_builddir)/src  -d $(top_builddir)/daemon \
> +	  -d $(top_builddir)/src \

Since daemon is the former name and this appears to be a clean label
target for coverage, perhaps we should keep daemon just to clean up
"old" trees...

The rest looks reasonable to me although Makefiles, run scripts, etc.
aren't my favorite things to look at 0-)

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/5] Forget last daemon/ dir artefacts
Posted by Michal Prívozník 7 years, 6 months ago 
On 07/21/2018 02:11 PM, John Ferlan wrote:
> 
> 
> On 07/12/2018 03:37 AM, Michal Privoznik wrote:
>> The most important part is LIBVIRTD_PATH env var fix. It is used
>> in virFileFindResourceFull() from tests. The libvirtd no longer
>> lives under daemon/.
>>
>> Then, libvirtd-fail test was still failing (as expected) but not
>> because of missing config file but because it was trying to
>> execute (nonexistent) top_builddir/daemon/libvirtd which
>> fulfilled expected outcome and thus test did not fail.
>>
>> Thirdly, lcov was told to generate coverage for daemon/ dir too.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  Makefile.am         | 2 +-
>>  run.in              | 2 +-
>>  tests/libvirtd-fail | 4 ++--
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 1926e21b7a..709064c6a6 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -80,7 +80,7 @@ check-access:
>>  cov: clean-cov
>>  	$(MKDIR_P) $(top_builddir)/coverage
>>  	$(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \
>> -	  -d $(top_builddir)/src  -d $(top_builddir)/daemon \
>> +	  -d $(top_builddir)/src \
> 
> Since daemon is the former name and this appears to be a clean label
> target for coverage, perhaps we should keep daemon just to clean up
> "old" trees...
> 

No. This is no a clean label. The rule says: in order to make target
"cov" you need to make target "clean-cov" first as it is dependency. So
I'm changing the create target not the cleanup. And -d $dir to lcov
means "include directory $dir to search for .da files" (whatever they
are - doesn't matter now).

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/5] Forget last daemon/ dir artefacts
Posted by John Ferlan 7 years, 6 months ago 

On 07/23/2018 04:44 AM, Michal Prívozník wrote:
> On 07/21/2018 02:11 PM, John Ferlan wrote:
>>
>>
>> On 07/12/2018 03:37 AM, Michal Privoznik wrote:
>>> The most important part is LIBVIRTD_PATH env var fix. It is used
>>> in virFileFindResourceFull() from tests. The libvirtd no longer
>>> lives under daemon/.
>>>
>>> Then, libvirtd-fail test was still failing (as expected) but not
>>> because of missing config file but because it was trying to
>>> execute (nonexistent) top_builddir/daemon/libvirtd which
>>> fulfilled expected outcome and thus test did not fail.
>>>
>>> Thirdly, lcov was told to generate coverage for daemon/ dir too.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>> ---
>>>  Makefile.am         | 2 +-
>>>  run.in              | 2 +-
>>>  tests/libvirtd-fail | 4 ++--
>>>  3 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Makefile.am b/Makefile.am
>>> index 1926e21b7a..709064c6a6 100644
>>> --- a/Makefile.am
>>> +++ b/Makefile.am
>>> @@ -80,7 +80,7 @@ check-access:
>>>  cov: clean-cov
>>>  	$(MKDIR_P) $(top_builddir)/coverage
>>>  	$(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \
>>> -	  -d $(top_builddir)/src  -d $(top_builddir)/daemon \
>>> +	  -d $(top_builddir)/src \
>>
>> Since daemon is the former name and this appears to be a clean label
>> target for coverage, perhaps we should keep daemon just to clean up
>> "old" trees...
>>
> 
> No. This is no a clean label. The rule says: in order to make target
> "cov" you need to make target "clean-cov" first as it is dependency. So
> I'm changing the create target not the cleanup. And -d $dir to lcov
> means "include directory $dir to search for .da files" (whatever they
> are - doesn't matter now).
> 
> Michal
> 

Hence the reason I don't like to review Makefile changes ;-)

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.