[PATCH 1/1] virtio: console: replace EMFILE with EBUSY for already-open port

[PATCH 1/1] virtio: console: replace EMFILE with EBUSY for already-open port

[Amit Shah]

Returning EMFILE (process has too many open files) is incorrect to
indicate a port is already open by another process.  Use EBUSY for that.

This does change what we report to userspace, but I believe userspace
can look at it this way: it gets EBUSY, a new error code, instead of
EMFILE.  It's still an error, and that's not changing.

Reported-by: Mateusz Guzik <mguzik@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
Rusty, is this OK?  It's a change for the userspace, so is it considered
breakage?  OTOH the current return value is obviously wrong, 

'Broken' userspace code could be relying on the exact error type
(current EMFILE) to detect if other processes are using the same file,
that's the only thing I can think  of where this change can break
existing userspace.  But is that a strong enough reason to not fix this?


 drivers/char/virtio_console.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index ce5f3fc..b94be04 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1040,7 +1040,7 @@ static int port_fops_open(struct inode *inode, struct file *filp)
 	spin_lock_irq(&port->inbuf_lock);
 	if (port->guest_connected) {
 		spin_unlock_irq(&port->inbuf_lock);
-		ret = -EMFILE;
+		ret = -EBUSY;
 		goto out;
 	}
 
-- 
1.8.1.4

Re: [PATCH 1/1] virtio: console: replace EMFILE with EBUSY for already-open port

[Rusty Russell]

Amit Shah <amit.shah@redhat.com> writes:
> Returning EMFILE (process has too many open files) is incorrect to
> indicate a port is already open by another process.  Use EBUSY for that.
>
> This does change what we report to userspace, but I believe userspace
> can look at it this way: it gets EBUSY, a new error code, instead of
> EMFILE.  It's still an error, and that's not changing.
>
> Reported-by: Mateusz Guzik <mguzik@redhat.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> Rusty, is this OK?  It's a change for the userspace, so is it considered
> breakage?  OTOH the current return value is obviously wrong, 

Yes, I agree.  I don't know of anyone testing for EMFILE particularly,
so this is OK.

Applied,
Rusty.

>
> 'Broken' userspace code could be relying on the exact error type
> (current EMFILE) to detect if other processes are using the same file,
> that's the only thing I can think  of where this change can break
> existing userspace.  But is that a strong enough reason to not fix this?
>
>
>  drivers/char/virtio_console.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index ce5f3fc..b94be04 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1040,7 +1040,7 @@ static int port_fops_open(struct inode *inode, struct file *filp)
>  	spin_lock_irq(&port->inbuf_lock);
>  	if (port->guest_connected) {
>  		spin_unlock_irq(&port->inbuf_lock);
> -		ret = -EMFILE;
> +		ret = -EBUSY;
>  		goto out;
>  	}
>  
> -- 
> 1.8.1.4