* [Qemu-devel] [PATCH] char: translate from QIOChannel error to errno
@ 2016-03-11 17:55 marcandre.lureau
2016-03-11 18:36 ` Daniel P. Berrange
2016-03-18 11:55 ` Paolo Bonzini
0 siblings, 2 replies; 4+ messages in thread
From: marcandre.lureau @ 2016-03-11 17:55 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, danpb
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Caller of CharDriverState.chr* callback assume errno error conventions.
Translate QIOChannel error to errno (this fixes potential EAGAIN
regression, for ex if a vhost-user backend block, qemu_chr_fe_read_all()
could get error -2 and not wait)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qemu-char.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/qemu-char.c b/qemu-char.c
index ad11b75..4317388 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2727,6 +2727,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
NULL);
}
+ if (ret == QIO_CHANNEL_ERR_BLOCK) {
+ errno = EAGAIN;
+ ret = -1;
+ } else if (ret == -1) {
+ errno = EIO;
+ }
+
if (msgfds_num) {
/* close and clean read_msgfds */
for (i = 0; i < s->read_msgfds_num; i++) {
--
2.5.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] char: translate from QIOChannel error to errno
2016-03-11 17:55 [Qemu-devel] [PATCH] char: translate from QIOChannel error to errno marcandre.lureau
@ 2016-03-11 18:36 ` Daniel P. Berrange
2016-03-18 11:55 ` Paolo Bonzini
1 sibling, 0 replies; 4+ messages in thread
From: Daniel P. Berrange @ 2016-03-11 18:36 UTC (permalink / raw)
To: marcandre.lureau; +Cc: qemu-devel
On Fri, Mar 11, 2016 at 06:55:24PM +0100, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Caller of CharDriverState.chr* callback assume errno error conventions.
> Translate QIOChannel error to errno (this fixes potential EAGAIN
> regression, for ex if a vhost-user backend block, qemu_chr_fe_read_all()
> could get error -2 and not wait)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> qemu-char.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index ad11b75..4317388 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2727,6 +2727,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> NULL);
> }
>
> + if (ret == QIO_CHANNEL_ERR_BLOCK) {
> + errno = EAGAIN;
> + ret = -1;
> + } else if (ret == -1) {
> + errno = EIO;
> + }
> +
> if (msgfds_num) {
> /* close and clean read_msgfds */
> for (i = 0; i < s->read_msgfds_num; i++) {
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] char: translate from QIOChannel error to errno
2016-03-11 17:55 [Qemu-devel] [PATCH] char: translate from QIOChannel error to errno marcandre.lureau
2016-03-11 18:36 ` Daniel P. Berrange
@ 2016-03-18 11:55 ` Paolo Bonzini
2016-03-18 12:03 ` Daniel P. Berrange
1 sibling, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2016-03-18 11:55 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: danpb
On 11/03/2016 18:55, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Caller of CharDriverState.chr* callback assume errno error conventions.
> Translate QIOChannel error to errno (this fixes potential EAGAIN
> regression, for ex if a vhost-user backend block, qemu_chr_fe_read_all()
> could get error -2 and not wait)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qemu-char.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index ad11b75..4317388 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2727,6 +2727,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> NULL);
> }
>
> + if (ret == QIO_CHANNEL_ERR_BLOCK) {
> + errno = EAGAIN;
> + ret = -1;
> + } else if (ret == -1) {
> + errno = EIO;
> + }
> +
> if (msgfds_num) {
> /* close and clean read_msgfds */
> for (i = 0; i < s->read_msgfds_num; i++) {
>
I can take this patch as it fixes a real regression, but could
QIOChannel just return negative errno?
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] char: translate from QIOChannel error to errno
2016-03-18 11:55 ` Paolo Bonzini
@ 2016-03-18 12:03 ` Daniel P. Berrange
0 siblings, 0 replies; 4+ messages in thread
From: Daniel P. Berrange @ 2016-03-18 12:03 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: marcandre.lureau, qemu-devel
On Fri, Mar 18, 2016 at 12:55:34PM +0100, Paolo Bonzini wrote:
>
>
> On 11/03/2016 18:55, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Caller of CharDriverState.chr* callback assume errno error conventions.
> > Translate QIOChannel error to errno (this fixes potential EAGAIN
> > regression, for ex if a vhost-user backend block, qemu_chr_fe_read_all()
> > could get error -2 and not wait)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > qemu-char.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/qemu-char.c b/qemu-char.c
> > index ad11b75..4317388 100644
> > --- a/qemu-char.c
> > +++ b/qemu-char.c
> > @@ -2727,6 +2727,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char *buf, size_t len)
> > NULL);
> > }
> >
> > + if (ret == QIO_CHANNEL_ERR_BLOCK) {
> > + errno = EAGAIN;
> > + ret = -1;
> > + } else if (ret == -1) {
> > + errno = EIO;
> > + }
> > +
> > if (msgfds_num) {
> > /* close and clean read_msgfds */
> > for (i = 0; i < s->read_msgfds_num; i++) {
> >
>
> I can take this patch as it fixes a real regression, but could
> QIOChannel just return negative errno?
No, QIOChannel explicitly does not ever use errno's because that only
works if all the functions QIOChannel uses internally set errnos. This
is not the case for the TLS impl of QIOChannel. For this reason, the
QIOChannel APIs all use 'Error **errp' for error reporting.
Ultimately as callers of the QIOChannel code in QEMU are converted to
propagate 'Error **errp' we'll be ok, but in the immediate some callers
like chardev still have a built-in assumption that they'll have errno's
available, so we have to do this compat hack.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-18 12:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-11 17:55 [Qemu-devel] [PATCH] char: translate from QIOChannel error to errno marcandre.lureau
2016-03-11 18:36 ` Daniel P. Berrange
2016-03-18 11:55 ` Paolo Bonzini
2016-03-18 12:03 ` Daniel P. Berrange
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).