[kvm-unit-tests PATCH v2] scripts/arch-run.bash: Drop the dependency on "jq"

[kvm-unit-tests PATCH v2] scripts/arch-run.bash: Drop the dependency on "jq"

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

For checking whether a panic event occurred, a simple "grep"
for the related text in the output is enough - it's very unlikely
that the output of QEMU will change. This way we can drop the
dependency on the program "jq" which might not be installed on
some systems.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2: Use [[:blank:]]* as suggested by Claudio

 scripts/arch-run.bash | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 58e4f93f..4642cf95 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -287,11 +287,6 @@ do_migration ()
 
 run_panic ()
 {
-	if ! command -v jq >/dev/null 2>&1; then
-		echo "${FUNCNAME[0]} needs jq" >&2
-		return 77
-	fi
-
 	trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
 	trap 'rm -f ${qmp}.in ${qmp}.out' RETURN EXIT
 
@@ -303,8 +298,7 @@ run_panic ()
 		-mon chardev=mon,mode=control -S &
 	echo '{ "execute": "qmp_capabilities" }{ "execute": "cont" }' > ${qmp}.in
 
-	panic_event_count=$(jq -c 'select(.event == "GUEST_PANICKED")' < ${qmp}.out | wc -l)
-	if [ "$panic_event_count" -lt 1 ]; then
+	if ! grep -E -q '"event"[[:blank:]]*:[[:blank:]]*"GUEST_PANICKED"' ${qmp}.out ; then
 		echo "FAIL: guest did not panic"
 		ret=3
 	else
-- 
2.50.1

[kvm-unit-tests PATCH v2] scripts/arch-run.bash: Drop the dependency on "jq"

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

For checking whether a panic event occurred, a simple "grep"
for the related text in the output is enough - it's very unlikely
that the output of QEMU will change. This way we can drop the
dependency on the program "jq" which might not be installed on
some systems.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 v2: Change the regular expression according to Claudio's suggestion

 scripts/arch-run.bash | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 36222355..16417a1e 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -296,11 +296,6 @@ do_migration ()
 
 run_panic ()
 {
-	if ! command -v jq >/dev/null 2>&1; then
-		echo "${FUNCNAME[0]} needs jq" >&2
-		return 77
-	fi
-
 	trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
 	trap 'rm -f ${qmp}.in ${qmp}.out' RETURN EXIT
 
@@ -312,8 +307,7 @@ run_panic ()
 		-mon chardev=mon,mode=control -S &
 	echo '{ "execute": "qmp_capabilities" }{ "execute": "cont" }' > ${qmp}.in
 
-	panic_event_count=$(jq -c 'select(.event == "GUEST_PANICKED")' < ${qmp}.out | wc -l)
-	if [ "$panic_event_count" -lt 1 ]; then
+	if ! grep -E -q '"event"[[:blank:]]*:[[:blank:]]*"GUEST_PANICKED"' ${qmp}.out ; then
 		echo "FAIL: guest did not panic"
 		ret=3
 	else
-- 
2.51.0

Re: [kvm-unit-tests PATCH v2] scripts/arch-run.bash: Drop the dependency on "jq"

[Claudio Imbrenda]

On Tue,  9 Sep 2025 06:58:55 +0200
Thomas Huth <thuth@redhat.com> wrote:

> From: Thomas Huth <thuth@redhat.com>
> 
> For checking whether a panic event occurred, a simple "grep"
> for the related text in the output is enough - it's very unlikely
> that the output of QEMU will change. This way we can drop the
> dependency on the program "jq" which might not be installed on
> some systems.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  v2: Change the regular expression according to Claudio's suggestion
> 
>  scripts/arch-run.bash | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 36222355..16417a1e 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -296,11 +296,6 @@ do_migration ()
>  
>  run_panic ()
>  {
> -	if ! command -v jq >/dev/null 2>&1; then
> -		echo "${FUNCNAME[0]} needs jq" >&2
> -		return 77
> -	fi
> -
>  	trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
>  	trap 'rm -f ${qmp}.in ${qmp}.out' RETURN EXIT
>  
> @@ -312,8 +307,7 @@ run_panic ()
>  		-mon chardev=mon,mode=control -S &
>  	echo '{ "execute": "qmp_capabilities" }{ "execute": "cont" }' > ${qmp}.in
>  
> -	panic_event_count=$(jq -c 'select(.event == "GUEST_PANICKED")' < ${qmp}.out | wc -l)
> -	if [ "$panic_event_count" -lt 1 ]; then
> +	if ! grep -E -q '"event"[[:blank:]]*:[[:blank:]]*"GUEST_PANICKED"' ${qmp}.out ; then
>  		echo "FAIL: guest did not panic"
>  		ret=3
>  	else

Re: [kvm-unit-tests PATCH v2] scripts/arch-run.bash: Drop the dependency on "jq"

[Andrew Jones]

On Tue, Sep 09, 2025 at 06:58:55AM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> For checking whether a panic event occurred, a simple "grep"
> for the related text in the output is enough - it's very unlikely
> that the output of QEMU will change. This way we can drop the
> dependency on the program "jq" which might not be installed on
> some systems.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Change the regular expression according to Claudio's suggestion
> 
>  scripts/arch-run.bash | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 36222355..16417a1e 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -296,11 +296,6 @@ do_migration ()
>  
>  run_panic ()
>  {
> -	if ! command -v jq >/dev/null 2>&1; then
> -		echo "${FUNCNAME[0]} needs jq" >&2
> -		return 77
> -	fi
> -
>  	trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
>  	trap 'rm -f ${qmp}.in ${qmp}.out' RETURN EXIT
>  
> @@ -312,8 +307,7 @@ run_panic ()
>  		-mon chardev=mon,mode=control -S &
>  	echo '{ "execute": "qmp_capabilities" }{ "execute": "cont" }' > ${qmp}.in
>  
> -	panic_event_count=$(jq -c 'select(.event == "GUEST_PANICKED")' < ${qmp}.out | wc -l)
> -	if [ "$panic_event_count" -lt 1 ]; then
> +	if ! grep -E -q '"event"[[:blank:]]*:[[:blank:]]*"GUEST_PANICKED"' ${qmp}.out ; then
>  		echo "FAIL: guest did not panic"
>  		ret=3
>  	else
> -- 
> 2.51.0
>

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

Re: [kvm-unit-tests PATCH v2] scripts/arch-run.bash: Drop the dependency on "jq"

[Nico Boehr]

On Tue Sep 9, 2025 at 6:58 AM CEST, Thomas Huth wrote:
> For checking whether a panic event occurred, a simple "grep"
> for the related text in the output is enough - it's very unlikely
> that the output of QEMU will change. This way we can drop the
> dependency on the program "jq" which might not be installed on
> some systems.

Trying to understand which problem you're trying to solve here.

Is there any major distribution which doesn't have jq in its repos? Or any
reason why you wouldn't install it?

> diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> index 36222355..16417a1e 100644
> --- a/scripts/arch-run.bash
> +++ b/scripts/arch-run.bash
> @@ -296,11 +296,6 @@ do_migration ()
>  
>  run_panic ()
>  {
[...]
> -	panic_event_count=$(jq -c 'select(.event == "GUEST_PANICKED")' < ${qmp}.out | wc -l)
> -	if [ "$panic_event_count" -lt 1 ]; then
> +	if ! grep -E -q '"event"[[:blank:]]*:[[:blank:]]*"GUEST_PANICKED"' ${qmp}.out ; then

This changes behaviour.

Now "event" can be arbitrarily deep nested in the JSON. It could even be
completely invalid JSON.

Not saying we shouldn't do this, it just comes with a cost and we need to see if
it's worth paying that.

Re: [kvm-unit-tests PATCH v2] scripts/arch-run.bash: Drop the dependency on "jq"

[Claudio Imbrenda]

On Wed, 10 Sep 2025 09:57:17 +0200
"Nico Boehr" <nrb@linux.ibm.com> wrote:

> On Tue Sep 9, 2025 at 6:58 AM CEST, Thomas Huth wrote:
> > For checking whether a panic event occurred, a simple "grep"
> > for the related text in the output is enough - it's very unlikely
> > that the output of QEMU will change. This way we can drop the
> > dependency on the program "jq" which might not be installed on
> > some systems.  
> 
> Trying to understand which problem you're trying to solve here.
> 
> Is there any major distribution which doesn't have jq in its repos? Or any
> reason why you wouldn't install it?

I think it's just a matter of trying to avoid too many dependencies,
especially for something this trivial

> 
> > diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
> > index 36222355..16417a1e 100644
> > --- a/scripts/arch-run.bash
> > +++ b/scripts/arch-run.bash
> > @@ -296,11 +296,6 @@ do_migration ()
> >  
> >  run_panic ()
> >  {  
> [...]
> > -	panic_event_count=$(jq -c 'select(.event == "GUEST_PANICKED")' < ${qmp}.out | wc -l)
> > -	if [ "$panic_event_count" -lt 1 ]; then
> > +	if ! grep -E -q '"event"[[:blank:]]*:[[:blank:]]*"GUEST_PANICKED"' ${qmp}.out ; then  
> 
> This changes behaviour.
> 
> Now "event" can be arbitrarily deep nested in the JSON. It could even be
> completely invalid JSON.

in that case we have other issues

> 
> Not saying we shouldn't do this, it just comes with a cost and we need to see if
> it's worth paying that.

I don't have a strong opinion on this, but in general I like the idea
of having fewer dependencies

Re: [kvm-unit-tests PATCH v2] scripts/arch-run.bash: Drop the dependency on "jq"

[Thomas Huth]

On 10/09/2025 11.00, Claudio Imbrenda wrote:
> On Wed, 10 Sep 2025 09:57:17 +0200
> "Nico Boehr" <nrb@linux.ibm.com> wrote:
> 
>> On Tue Sep 9, 2025 at 6:58 AM CEST, Thomas Huth wrote:
>>> For checking whether a panic event occurred, a simple "grep"
>>> for the related text in the output is enough - it's very unlikely
>>> that the output of QEMU will change. This way we can drop the
>>> dependency on the program "jq" which might not be installed on
>>> some systems.
>>
>> Trying to understand which problem you're trying to solve here.
>>
>> Is there any major distribution which doesn't have jq in its repos? Or any
>> reason why you wouldn't install it?
> 
> I think it's just a matter of trying to avoid too many dependencies,
> especially for something this trivial

Yes ... actually, a while ago, I noticed that I was never running this test 
on fresh installations since "jq" is not there by default. I guess it's the 
same for many other people, too, since we don't really tell them to install 
"jq" first.

So let's give this patch a try for a while, and if we run into problems, we 
can still revert it.

  Thomas

Re: [kvm-unit-tests PATCH v2] scripts/arch-run.bash: Drop the dependency on "jq"

[Nico Boehr]

On Wed Sep 10, 2025 at 11:06 AM CEST, Thomas Huth wrote:
> On 10/09/2025 11.00, Claudio Imbrenda wrote:
>> On Wed, 10 Sep 2025 09:57:17 +0200
>> "Nico Boehr" <nrb@linux.ibm.com> wrote:
>> 
>>> On Tue Sep 9, 2025 at 6:58 AM CEST, Thomas Huth wrote:
>>>> For checking whether a panic event occurred, a simple "grep"
>>>> for the related text in the output is enough - it's very unlikely
>>>> that the output of QEMU will change. This way we can drop the
>>>> dependency on the program "jq" which might not be installed on
>>>> some systems.
>>>
>>> Trying to understand which problem you're trying to solve here.
>>>
>>> Is there any major distribution which doesn't have jq in its repos? Or any
>>> reason why you wouldn't install it?
>> 
>> I think it's just a matter of trying to avoid too many dependencies,
>> especially for something this trivial
>
> Yes ... actually, a while ago, I noticed that I was never running this test 
> on fresh installations since "jq" is not there by default. I guess it's the 
> same for many other people, too, since we don't really tell them to install 
> "jq" first.
>
> So let's give this patch a try for a while, and if we run into problems, we 
> can still revert it.

Acked-by: Nico Boehr <nrb@linux.ibm.com>