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

[Paul Holzinger <pholzing@redhat.com>]

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 <pholzing@redhat.com>
>> ---
>>   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);
>>