| Message ID | 1457718924-19338-1-git-send-email-marcandre.lureau@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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++) {