Skip to content

Restore qemu error messages - #3101

Merged
lgirdwood merged 2 commits into
thesofproject:masterfrom
marc-hb:restore-qemu-errors
Jun 30, 2020
Merged

Restore qemu error messages#3101
lgirdwood merged 2 commits into
thesofproject:masterfrom
marc-hb:restore-qemu-errors

Conversation

@marc-hb

@marc-hb marc-hb commented Jun 27, 2020

Copy link
Copy Markdown
Collaborator

See commit messages

Fixes 428804e ("scripts/qemu-check.sh: add set -e") which failed to consider the case of an empty $IPC_REG or $IPC_MSG.

Address two shellcheck warnings

marc-hb added 2 commits June 26, 2020 22:30
@marc-hb
qemu-check.sh: make $ROM a quoted bash array "${ROM[@]}"
3fcd238 
Removes shellcheck warning and could allow directory names with
whitespace in a very distant future.

Silence printf "$@" shellcheck warning in die() function.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
qemu-check.sh: restore error messages when test fails
aaea5a7 
Fixes 428804e ("scripts/qemu-check.sh: add set -e") which failed to
consider the case of an empty $IPC_REG or $IPC_MSG.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb mentioned this pull request Jun 27, 2020
Merged
@marc-hb

marc-hb commented Jun 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

The only checkpatch warning is this and I really couldn't find a non-convoluted way to avoid it:

WARNING: line over 80 characters
#35: FILE: scripts/qemu-check.sh:150:
+		ROM=(-r "${SOF_BUILDS}/build_${platform}_gcc/src/arch/xtensa/rom-$platform.bin")

sof-ci/jenkins/pr-fw-build — FW Build Failed:byt_xcc at https://sof-ci.01.org/sofpr/PR3101/build6429/build is the usual #3063

@marc-hb
marc-hb marked this pull request as ready for review June 27, 2020 05:43
@marc-hb
marc-hb requested a review from jajanusz as a code owner June 27, 2020 05:43
@marc-hb marc-hb changed the title Restore qemu errors Restore qemu error messages Jun 27, 2020
@marc-hb marc-hb mentioned this pull request Jun 27, 2020
Merged
xiulipan
xiulipan approved these changes Jun 29, 2020
View reviewed changes

@xiulipan xiulipan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for fixing the shellcheck.

@lgirdwood

lgirdwood commented Jun 29, 2020

Copy link
Copy Markdown
Member

SOFCI TEST

@marc-hb marc-hb mentioned this pull request Jun 29, 2020
Merged
@marc-hb
marc-hb requested a review from bkokoszx June 29, 2020 21:34
@marc-hb

marc-hb commented Jun 29, 2020

Copy link
Copy Markdown
Collaborator Author

We could probably increase the timeout; it would not be the first time qemu runs much slower and less deterministically on a (cloud) VM than on real hardware. 2 seconds is pretty short.

BTW icelake failed in https://travis-ci.org/github/thesofproject/sof/jobs/701624042 / #3083 whereas cannonlake failed in https://travis-ci.org/github/thesofproject/sof/jobs/702557005 / #3064

@lgirdwood

lgirdwood commented Jun 30, 2020

Copy link
Copy Markdown
Member

@marc-hb do we need to adjust this further, it's still pending for Jenkins but Travis is passing the same test ?

@aiChaoSONG

aiChaoSONG commented Jun 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@lgirdwood There is CI update, I retriggered CI test, please return to see the result a moment later, should be positive.

aiChaoSONG
aiChaoSONG approved these changes Jun 30, 2020
View reviewed changes

@aiChaoSONG aiChaoSONG left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI verified

@lgirdwood
lgirdwood merged commit 1c4d14a into thesofproject:master Jun 30, 2020
@lgirdwood

lgirdwood commented Jun 30, 2020

Copy link
Copy Markdown
Member

Thank you @aiChaoSONG

@marc-hb
marc-hb deleted the restore-qemu-errors branch June 30, 2020 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lgirdwood lgirdwood lgirdwood approved these changes
@jajanusz jajanusz Awaiting requested review from jajanusz
@dbaluta dbaluta Awaiting requested review from dbaluta
@ranj063 ranj063 Awaiting requested review from ranj063
@bkokoszx bkokoszx Awaiting requested review from bkokoszx
+2 more reviewers
@xiulipan xiulipan xiulipan approved these changes
@aiChaoSONG aiChaoSONG aiChaoSONG approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@marc-hb @lgirdwood @aiChaoSONG @xiulipan