Fix conmon attach socket buffer size #11503
Conversation
openshift-ci
bot
added
the
approved
|
@mheon @vrothberg @rhatdan PTAL |
|
Test LGTM. I confirmed that it fails in main using podman-remote. Nice quick fix. |
|
Restarted one test, otherwise LGTM |
|
Great find @Luap99 |
openshift-ci
bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Luap99
force-pushed
the
remote-attach
branch
from
67b0fa4 to
0cf8ffe
Compare
|
/lgtm |
The conmon buffer size is 8192, however the attach socket needs two extra bytes. The first byte of each message will be the STREAM type. The last byte is a null byte. So when we want to read 8192 message bytes we need to read 8193 bytes since the first one is special. check https://github.com/containers/conmon/blob/1ef246896b4f6566964ed861b98cd32d0e7bf7a2/src/ctr_stdio.c#L101-L107 This problem can be seen in podman-remote run/exec when it prints output with 8192 or more bytes. The output will miss the 8192 byte. Fixes containers#11496 Signed-off-by: Paul Holzinger <[email protected]>
Luap99
force-pushed
the
remote-attach
branch
from
0cf8ffe to
7cf2227
Compare
|
Rebased to fix the tests |
|
Flake is #10386. Maybe getting rid of ubuntu-2010 will sweep that under the rug. |
| bufferSize = conmonConfig.BufSize | ||
| // Important: The conmon attach socket uses an extra byte at the beginning of each | ||
| // message to specify the STREAM so we have to increase the buffer size by one | ||
| bufferSize = conmonConfig.BufSize + 1 |
There was a problem hiding this comment.
Would it make sense to also use this constant in oci_attach_linux.go line 225?
There was a problem hiding this comment.
Yes but it is not worth to repush for this change, thanks for catching this.
|
/lgtm hold remains active from before; leaving in place pending resolution of @mnk's question |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
github-actions
bot
added
the
locked - please file new issue/PR
The conmon buffer size is 8192, however the attach socket needs two extra
bytes. The first byte of each message will be the STREAM type. The last
byte is a null byte. So when we want to read 8192 message bytes we need
to read 8193 bytes since the first one is special.
check https://github.com/containers/conmon/blob/1ef246896b4f6566964ed861b98cd32d0e7bf7a2/src/ctr_stdio.c#L101-L107
This problem can be seen in podman-remote run/exec when it prints output
with 8192 or more bytes. The output will miss the 8192 byte.
Fixes #11496
Signed-off-by: Paul Holzinger [email protected]