[PATCH 2/2] erofs-utils: mount: fix ioctl-based NBD disconnect behavior

[Yifan Zhao]

Hi Xiang,


I'm not entirely clear on what does the disconnect variable solves.

Disconnection could occur at any point during the execution of loop_fn,

meaning any read/write operations on the socket performed by loop_fn

might return EPIPE, and we must handle this case appropriately,

and I think merely checking whether disconnection has occurred

at fixed points in the loop is insufficient?


Thanks,

Yifan

On 12/27/2025 8:40 PM, Gao Xiang wrote:
> Hi Yifan,
>
> On 2025/12/27 19:39, Yifan Zhao wrote:
>> Currently erofsmount_startnbd() doesn't ignore SIGPIPE, causing
>> erofsmount_nbd_loopfn() to be killed abruptly without clean up during
>> disconnect. Moreover, -EPIPE from NBD socket I/O is expected while
>> disconnecting, and erofsmount_startnbd() treats it as error, leading to
>> redundant print:
>> ```
>> <E> erofs: NBD worker failed with [Error 32] Broken pipe
>> ```
>>
>> Signed-off-by: Yifan Zhao <zhaoyifan28 at huawei.com>
>> ---
>>   mount/main.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/mount/main.c b/mount/main.c
>> index 5ba2e0a..965b0b8 100644
>> --- a/mount/main.c
>> +++ b/mount/main.c
>> @@ -621,11 +621,8 @@ static void *erofsmount_nbd_loopfn(void *arg)
>>           off_t pos;
>>             err = erofs_nbd_get_request(ctx->sk.fd, &rq);
>> -        if (err < 0) {
>> -            if (err == -EPIPE)
>> -                err = 0;
>> +        if (err < 0)
>>               break;
>> -        }
>>             if (rq.type != EROFS_NBD_CMD_READ) {
>>               err = erofs_nbd_send_reply_header(ctx->sk.fd,
>> @@ -653,6 +650,8 @@ static void *erofsmount_nbd_loopfn(void *arg)
>>   out:
>>       erofs_io_close(&ctx->vd);
>>       erofs_io_close(&ctx->sk);
>> +    if (err == -EPIPE)
>> +        err = 0;
>>       return (void *)(uintptr_t)err;
>>   }
>>   @@ -663,6 +662,12 @@ static int erofsmount_startnbd(int nbdfd, 
>> struct erofs_nbd_source *source)
>>       pthread_t th;
>>       int err, err2;
>>   +    /* Otherwise, NBD disconnect sends SIGPIPE, skipping cleanup */
>> +    if (signal(SIGPIPE, SIG_IGN) == SIG_ERR) {
>> +        err = -errno;
>> +        goto out_closefd;
>> +    }
>
> Can we register a signal handler for SIGPIPE instead, and setup
> a disconnected variable for erofsmount_nbd_loopfn() to notice
> for example too (in case of unnecessary erofs_nbd_get_request()).
>
> Thanks,
> Gao Xiang
>
>> +
>>       if (source->type == EROFSNBD_SOURCE_OCI) {
>>           if (source->ocicfg.tarindex_path || 
>> source->ocicfg.zinfo_path) {
>>               err = erofsmount_tarindex_open(&ctx.vd, &source->ocicfg,
>
>