All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
Cc: Amit Shah <amit.shah@redhat.com>,
	Virtualization List <virtualization@lists.linux-foundation.org>
Subject: Re: [PATCH 1/1] virtio: console: replace EMFILE with EBUSY for already-open port
Date: Mon, 15 Apr 2013 12:00:27 +0930	[thread overview]
Message-ID: <87sj2s8r0c.fsf@rustcorp.com.au> (raw)
In-Reply-To: <2d57d36ef32e2d9073b0ca3e4e4b2dc32897932f.1365660074.git.amit.shah@redhat.com>

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

      reply	other threads:[~2013-04-15  2:30 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-11  6:08 [PATCH 1/1] virtio: console: replace EMFILE with EBUSY for already-open port Amit Shah
2013-04-15  2:30 ` Rusty Russell [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87sj2s8r0c.fsf@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=amit.shah@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.