Re: [PATCH] tap: include errno in error when tap_ns_tun() fails

On 02/08/2023 03:39, David Gibson wrote:

On Tue, Aug 01, 2023 at 01:50:17PM +0200, Paul Holzinger wrote:

...

It is important to know why a syscall failed so pasta should include the errno in the error message. This is still not perfect as we do not know which of functions (open, ioctl, if_nametoindex) failed but it should at least include more important context. Uh.. we certainly want this, but I don't think this implementation will quite do it.

...

This change was inspiered by a podman issue[1].

[1] https://github.com/containers/podman/issues/19428

Signed-off-by: Paul Holzinger --- tap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tap.c b/tap.c index a6a73d3..c212616 100644 --- a/tap.c +++ b/tap.c @@ -1205,7 +1205,8 @@ static void tap_sock_tun_init(struct ctx *c)

NS_CALL(tap_ns_tun, c); NS_CALL means we're running the function in an ephemeral thread/process/thingy..

...

if (tun_ns_fd == -1) - die("Failed to open tun socket in namespace"); + die("Failed to open tun socket in namespace: %s", + strerror(errno)); ..which means we can't rely on it actually setting errno in the original process. The ephemeral thread does share memory, but modern errno is a weird per-thread thingy, so I'm not entirely sure what will happen here, but I'm certainly not confident about it working as we'd like.

I'll have a crack at a more robust approach today.

Thanks, I only did a quick test with chmod 600 /dev/net/tun and got the expected EACCES so I assumed it worked fine but anyway your series looks much better.

...

pasta_ns_conf(c);