Re: [PATCH] kvmtool: virtio-9p: Convert EMFILE error at the server to ENFILE for the guest

[Will Deacon <will.deacon@arm.com>]

On Fri, Jan 16, 2015 at 11:30:10AM +0000, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> If an open at the 9p server(host) fails with EMFILE (Too many open files for
> the process), we should return ENFILE(too many open files in the system) to
> the guest to indicate the actual status within the guest.
> 
> This was uncovered during LTP, where getdtablesize01 fails to open the maximum
> number-open-files.
> 
> getdtablesize01    0  TINFO  :  Maximum number of files a process can have opened is 1024
> getdtablesize01    0  TINFO  :  Checking with the value returned by getrlimit...RLIMIT_NOFILE
> getdtablesize01    1  TPASS  :  got correct dtablesize, value is 1024
> getdtablesize01    0  TINFO  :  Checking Max num of files that can be opened by a process.Should be: RLIMIT_NOFILE - 1
> getdtablesize01    2  TFAIL  :  getdtablesize01.c:102: 974 != 1023
> 
> For a more practial impact:
> 
>  # ./getdtablesize01 &
> [1] 1834
>  getdtablesize01    0  TINFO  :  Maximum number of files a process can have opened is 1024
>  getdtablesize01    0  TINFO  :  Checking with the value returned by getrlimit...RLIMIT_NOFILE
>  getdtablesize01    1  TPASS  :  got correct dtablesize, value is 1024
>  getdtablesize01    0  TINFO  :  Checking Max num of files that can be opened by a process.Should be: RLIMIT_NOFILE - 1
>  getdtablesize01    2  TFAIL  :  getdtablesize01.c:102: 974 != 1023
>  [--- Modified to sleep indefinitely, without closing the files --- ]
> 
>  # ls
>  bash: /bin/ls: Too many open files
> 
> That gives a wrong error message for the bash, when getdtablesize01 has exhausted the system
> wide limits, giving false indicators.
> 
> With the fix, we get :
> 
>  # ls
>  bash: /bin/ls: Too many open files in system

I doubt this is affecting anybody in practice, but:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
> ---
>  tools/kvm/virtio/9p.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
> index 9073a1e..b24c0f2 100644
> --- a/tools/kvm/virtio/9p.c
> +++ b/tools/kvm/virtio/9p.c
> @@ -152,6 +152,10 @@ static void virtio_p9_error_reply(struct p9_dev *p9dev,
>  {
>  	u16 tag;
>  
> +	/* EMFILE at server implies ENFILE for the VM */
> +	if (err == EMFILE)
> +		err = ENFILE;
> +
>  	pdu->write_offset = VIRTIO_9P_HDR_LEN;
>  	virtio_p9_pdu_writef(pdu, "d", err);
>  	*outlen = pdu->write_offset;
> -- 
> 1.7.9.5
>