diff mbox

qemu-ga: do not run qga test when guest agent disabled

Message ID 1461051553-14720-1-git-send-email-hongyang.yang@easystack.cn
State New
Headers show

Commit Message

Yang Hongyang April 19, 2016, 7:39 a.m. UTC
When configure with --disable-guest-agent, make check will fail with:
ERROR:tests/test-qga.c:74:fixture_setup: assertion failed (error == NULL):
 Failed to execute child process "/home/xx/qemu/qemu-ga" (No such file or
directory) (g-exec-error-quark, 8)
make: *** [check-tests/test-qga] Error 1

This check was commented out by bab47d9a75a. I think that was by
mistake, because the commit message of that commit didn't mention
this change.

Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Yang Hongyang April 19, 2016, 7:52 a.m. UTC | #1
I think this patch should also be backported to stable v2.5.1

On Tue, Apr 19, 2016 at 3:39 PM, Yang Hongyang <hongyang.yang@easystack.cn>
wrote:

> When configure with --disable-guest-agent, make check will fail with:
> ERROR:tests/test-qga.c:74:fixture_setup: assertion failed (error == NULL):
>  Failed to execute child process "/home/xx/qemu/qemu-ga" (No such file or
> directory) (g-exec-error-quark, 8)
> make: *** [check-tests/test-qga] Error 1
>
> This check was commented out by bab47d9a75a. I think that was by
> mistake, because the commit message of that commit didn't mention
> this change.
>
> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 9de9598..9194f18 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -83,7 +83,9 @@ check-unit-y += tests/test-crypto-cipher$(EXESUF)
>  check-unit-y += tests/test-crypto-secret$(EXESUF)
>  check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)
>  check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF)
> -#check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
> +ifneq (,$(findstring qemu-ga,$(TOOLS)))
> +check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
> +endif
>  check-unit-y += tests/test-timed-average$(EXESUF)
>  check-unit-y += tests/test-io-task$(EXESUF)
>  check-unit-y += tests/test-io-channel-socket$(EXESUF)
> --
> 1.8.3.1
>
>
>
Michael Roth April 19, 2016, 10:01 p.m. UTC | #2
Quoting Yang Hongyang (2016-04-19 02:39:13)
> When configure with --disable-guest-agent, make check will fail with:
> ERROR:tests/test-qga.c:74:fixture_setup: assertion failed (error == NULL):
>  Failed to execute child process "/home/xx/qemu/qemu-ga" (No such file or
> directory) (g-exec-error-quark, 8)
> make: *** [check-tests/test-qga] Error 1
> 
> This check was commented out by bab47d9a75a. I think that was by
> mistake, because the commit message of that commit didn't mention
> this change.
> 
> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>

Thanks, applied to qga tree:
  https://github.com/mdroth/qemu/commits/qga

As much as I'd like to get this fixed for 2.6, the net effect, thanks to
the inadvertant commenting out of qga test case in bab47d9a75a, is that
the qemu-ga unit test currently gets skipped during make check. Given
RC3 is going to be tagged soon, and afaik is the last RC, I'm not
sure I would consider this enough of a blocker to send a last-minute
pull.

Peter: if you think there's still a window to get this in let me know
and I'll send the pull immediately. But for now I'll queue this for
2.7 and for stable.

> ---
>  tests/Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 9de9598..9194f18 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -83,7 +83,9 @@ check-unit-y += tests/test-crypto-cipher$(EXESUF)
>  check-unit-y += tests/test-crypto-secret$(EXESUF)
>  check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)
>  check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF)
> -#check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
> +ifneq (,$(findstring qemu-ga,$(TOOLS)))
> +check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
> +endif
>  check-unit-y += tests/test-timed-average$(EXESUF)
>  check-unit-y += tests/test-io-task$(EXESUF)
>  check-unit-y += tests/test-io-channel-socket$(EXESUF)
> -- 
> 1.8.3.1
>
Peter Maydell April 19, 2016, 10:08 p.m. UTC | #3
On 19 April 2016 at 23:01, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> Quoting Yang Hongyang (2016-04-19 02:39:13)
>> When configure with --disable-guest-agent, make check will fail with:
>> ERROR:tests/test-qga.c:74:fixture_setup: assertion failed (error == NULL):
>>  Failed to execute child process "/home/xx/qemu/qemu-ga" (No such file or
>> directory) (g-exec-error-quark, 8)
>> make: *** [check-tests/test-qga] Error 1
>>
>> This check was commented out by bab47d9a75a. I think that was by
>> mistake, because the commit message of that commit didn't mention
>> this change.
>>
>> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>
> Thanks, applied to qga tree:
>   https://github.com/mdroth/qemu/commits/qga
>
> As much as I'd like to get this fixed for 2.6, the net effect, thanks to
> the inadvertant commenting out of qga test case in bab47d9a75a, is that
> the qemu-ga unit test currently gets skipped during make check. Given
> RC3 is going to be tagged soon, and afaik is the last RC, I'm not
> sure I would consider this enough of a blocker to send a last-minute
> pull.
>
> Peter: if you think there's still a window to get this in let me know
> and I'll send the pull immediately. But for now I'll queue this for
> 2.7 and for stable.

Well, I'm not planning to tag RC3 til Thursday, so you have time
in that sense. Whether it's worth putting into RC3 I leave to
your judgement (it sounds like the only effect of not having it
is "there's a test case we could be running that we don't run" ?)

thanks
-- PMM
Michael Roth April 19, 2016, 10:25 p.m. UTC | #4
Quoting Peter Maydell (2016-04-19 17:08:03)
> On 19 April 2016 at 23:01, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > Quoting Yang Hongyang (2016-04-19 02:39:13)
> >> When configure with --disable-guest-agent, make check will fail with:
> >> ERROR:tests/test-qga.c:74:fixture_setup: assertion failed (error == NULL):
> >>  Failed to execute child process "/home/xx/qemu/qemu-ga" (No such file or
> >> directory) (g-exec-error-quark, 8)
> >> make: *** [check-tests/test-qga] Error 1
> >>
> >> This check was commented out by bab47d9a75a. I think that was by
> >> mistake, because the commit message of that commit didn't mention
> >> this change.
> >>
> >> Signed-off-by: Yang Hongyang <hongyang.yang@easystack.cn>
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>
> >> Cc: Michael S. Tsirkin <mst@redhat.com>
> >> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> >> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >
> > Thanks, applied to qga tree:
> >   https://github.com/mdroth/qemu/commits/qga
> >
> > As much as I'd like to get this fixed for 2.6, the net effect, thanks to
> > the inadvertant commenting out of qga test case in bab47d9a75a, is that
> > the qemu-ga unit test currently gets skipped during make check. Given
> > RC3 is going to be tagged soon, and afaik is the last RC, I'm not
> > sure I would consider this enough of a blocker to send a last-minute
> > pull.
> >
> > Peter: if you think there's still a window to get this in let me know
> > and I'll send the pull immediately. But for now I'll queue this for
> > 2.7 and for stable.
> 
> Well, I'm not planning to tag RC3 til Thursday, so you have time
> in that sense. Whether it's worth putting into RC3 I leave to
> your judgement (it sounds like the only effect of not having it
> is "there's a test case we could be running that we don't run" ?)

Yes. Although the patch fixes a more serious build/make check issue
with --disable-guest-agent, the inadvertant commenting in bab47d9a75a
(which was probably to work around that bug) masks the build failures
by unconditionally disabling the test.

Since disabling the unit test is a late regression, I think it's
probably worthwhile to try to fix as long as it isn't holding up
the release. Will send a pull shortly.

> 
> thanks
> -- PMM
>
diff mbox

Patch

diff --git a/tests/Makefile b/tests/Makefile
index 9de9598..9194f18 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -83,7 +83,9 @@  check-unit-y += tests/test-crypto-cipher$(EXESUF)
 check-unit-y += tests/test-crypto-secret$(EXESUF)
 check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)
 check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF)
-#check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
+ifneq (,$(findstring qemu-ga,$(TOOLS)))
+check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
+endif
 check-unit-y += tests/test-timed-average$(EXESUF)
 check-unit-y += tests/test-io-task$(EXESUF)
 check-unit-y += tests/test-io-channel-socket$(EXESUF)