Skip to content

container: wrap execv in retry-on-eintr#702

Merged
giuseppe merged 1 commit intocontainers:mainfrom
kolyshkin:exec-eintr
Jul 8, 2021
Merged

container: wrap execv in retry-on-eintr#702
giuseppe merged 1 commit intocontainers:mainfrom
kolyshkin:exec-eintr

Conversation

@kolyshkin
Copy link
Copy Markdown
Collaborator

Apparently, execve(2) can return EINTR on Azure (CIFS share),
which results in sporadic failures in crun exec (and possibly crun run).

To fix, wrap execv in TEMP_FAILURE_RETRY macro.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1917662
See-also: opencontainers/runc#3045

@kolyshkin
Copy link
Copy Markdown
Collaborator Author

I suspect that exec from CIFS mount [from Azure] is the only case where execve returns EINTR, so this is a not well-known case.

In general, EINTR is not very well documented. For this particular case, is not listed in execve(2) man page as a possible code. signal(7) has a section ("Interruption of system calls and library functions by signal handlers") that talks about EINTR, and execve is not mentioned either.

Anyway, we see it in the wild, and wrapping makes sense.

@giuseppe
Copy link
Copy Markdown
Member

giuseppe commented Jul 2, 2021

Thanks for the patch! LGTM, could you drop the change to libocispec?

@flouthoc
Copy link
Copy Markdown
Collaborator

flouthoc commented Jul 2, 2021

@kolyshkin @giuseppe @mrunalp this reverts libocispec to an older version.

@flouthoc
Copy link
Copy Markdown
Collaborator

flouthoc commented Jul 2, 2021

Although it should not do any harm as far as change seems.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Jul 2, 2021

LGTM

Apparently, execve(2) can return EINTR on Azure, which results
in sporadic failures in crun exec (and possibly crun run).

To fix, wrap execv in TEMP_FAILURE_RETRY macro.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Copy Markdown
Collaborator Author

could you drop the change to libocispec?

Sorry, my bad. Fixed, rebased, pushed.

Copy link
Copy Markdown
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

@giuseppe giuseppe merged commit 71986c7 into containers:main Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants