[Qemu-devel] [PATCH v2] virtio-serial: Print out reason for aborting before calling abort()

[Qemu-devel] [PATCH v2] virtio-serial: Print out reason for aborting before calling abort()

[Amit Shah]

When a port returns an error for not consuming data, we can only handle
the -EAGAIN error type.  Any other error isn't handled.  Print out a
message indicating this and the error returned.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index a82fbe9..8b715b2 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -155,6 +155,8 @@ static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
                                         buf_size);
             if (ret < 0 && ret != -EAGAIN) {
                 /* We don't handle any other type of errors here */
+                error_report("%s: unexpected return %zd, aborting.\n",
+                             __func__, ret);
                 abort();
             }
             if (ret == -EAGAIN || (ret >= 0 && ret < buf_size)) {
-- 
1.7.4

[Qemu-devel] Re: [PATCH v2] virtio-serial: Print out reason for aborting before calling abort()

[Juan Quintela]

Amit Shah <amit.shah@redhat.com> wrote:
> When a port returns an error for not consuming data, we can only handle
> the -EAGAIN error type.  Any other error isn't handled.  Print out a
> message indicating this and the error returned.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  hw/virtio-serial-bus.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index a82fbe9..8b715b2 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -155,6 +155,8 @@ static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
>                                          buf_size);
>              if (ret < 0 && ret != -EAGAIN) {
>                  /* We don't handle any other type of errors here */
> +                error_report("%s: unexpected return %zd, aborting.\n",
> +                             __func__, ret);
>                  abort();
>              }
>              if (ret == -EAGAIN || (ret >= 0 && ret < buf_size)) {

Reviewed-by: Juan Quintela <quintela@redhat.com>

I agree that change is a step in the right direction.  But I don't think
the "abort" way of handling errors.  Not that this can be improved
without changing all virtual queues functions to allow for return codes
:-(

[Qemu-devel] Re: [PATCH v2] virtio-serial: Print out reason for aborting before calling abort()

[Amit Shah]

On (Fri) 25 Mar 2011 [10:23:16], Juan Quintela wrote:
> Amit Shah <amit.shah@redhat.com> wrote:
> > When a port returns an error for not consuming data, we can only handle
> > the -EAGAIN error type.  Any other error isn't handled.  Print out a
> > message indicating this and the error returned.
> >
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> >  hw/virtio-serial-bus.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> > index a82fbe9..8b715b2 100644
> > --- a/hw/virtio-serial-bus.c
> > +++ b/hw/virtio-serial-bus.c
> > @@ -155,6 +155,8 @@ static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
> >                                          buf_size);
> >              if (ret < 0 && ret != -EAGAIN) {
> >                  /* We don't handle any other type of errors here */
> > +                error_report("%s: unexpected return %zd, aborting.\n",
> > +                             __func__, ret);
> >                  abort();
> >              }
> >              if (ret == -EAGAIN || (ret >= 0 && ret < buf_size)) {
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> I agree that change is a step in the right direction.  But I don't think
> the "abort" way of handling errors.  Not that this can be improved
> without changing all virtual queues functions to allow for return codes
> :-(

Yes :-(

BTW I'm working on more patches here: first step is to have
implementations of have_data() take the right call in other error
conditions (e.g. chardev closed would give -EPIPE, so those type of
things should be handled there sanely.)

		Amit