New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: ceph-disk: use communicate() instead of wait() for output #16347

Merged
merged 2 commits into from Jul 15, 2017

Conversation

Projects
None yet
1 participant
@tchaikov
Contributor

tchaikov commented Jul 14, 2017

to avoid possible deadlock. quote from doc of Popen.wait()

This will deadlock when using stdout=PIPE and/or stderr=PIPE and the
child process generates enough output to a pipe such that it blocks
waiting for the OS pipe buffer to accept more data. Use communicate() to
avoid that.

and print out the stdout and stderr using LOG.warn() if the command
fails.

Signed-off-by: Kefu Chai kchai@redhat.com

@tchaikov tchaikov requested a review from Jul 14, 2017

@ghost

ghost approved these changes Jul 14, 2017

tchaikov added some commits Jul 14, 2017

tests: ceph-disk: use communicate() instead of wait() for output
to avoid possible deadlock. quote from doc of Popen.wait()

> This will deadlock when using stdout=PIPE and/or stderr=PIPE and the
child process generates enough output to a pipe such that it blocks
waiting for the OS pipe buffer to accept more data. Use communicate() to
avoid that.

and print out the stdout and stderr using LOG.warn() if the command
fails.

Signed-off-by: Kefu Chai <kchai@redhat.com>
qa/suites/ceph-disk: whitelist health warnings
Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 15, 2017

changelog

@dachary could you please take a look again?

@tchaikov

This comment has been minimized.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 15, 2017

retest this please.

@ghost

This comment has been minimized.

ghost commented Jul 15, 2017

👍

@tchaikov tchaikov merged commit 742a117 into ceph:master Jul 15, 2017

3 of 4 checks passed

make check (arm64) make check failed
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details

@tchaikov tchaikov deleted the tchaikov:wip-test-ceph-disk branch Jul 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment