Re: [Qemu-devel] [PATCH 1/3] virtio: Introduce VirtIODevice.broken

[Fam Zheng]

On Tue, 04/22 12:05, Andreas Färber wrote:
> Am 22.04.2014 10:55, schrieb Fam Zheng:
> > If guest driver behaves abnormally, emulation code could mark the device
> > as "broken".
> > 
> > Once "broken" is set, device emulation will typically wait for a reset
> > command and ignore any other operations, but it could also return error
> > responds. In other words, whether and how does guest know about this
> > error status is device specific.
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  hw/virtio/virtio.c         | 12 ++++++++++++
> >  include/hw/virtio/virtio.h |  3 +++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index aeabf3a..222bb73 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -538,6 +538,16 @@ void virtio_set_status(VirtIODevice *vdev, uint8_t val)
> >      vdev->status = val;
> >  }
> >  
> > +bool virtio_broken(VirtIODevice *vdev)
> > +{
> > +    return vdev->broken;
> > +}
> > +
> > +void virtio_set_broken(VirtIODevice *vdev)
> > +{
> > +    vdev->broken = true;
> > +}
> > +
> >  void virtio_reset(void *opaque)
> >  {
> >      VirtIODevice *vdev = opaque;
> > @@ -554,6 +564,7 @@ void virtio_reset(void *opaque)
> >      vdev->queue_sel = 0;
> >      vdev->status = 0;
> >      vdev->isr = 0;
> > +    vdev->broken = false;
> >      vdev->config_vector = VIRTIO_NO_VECTOR;
> >      virtio_notify_vector(vdev, vdev->config_vector);
> >  
> > @@ -995,6 +1006,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
> >      vdev->status = 0;
> >      vdev->isr = 0;
> >      vdev->queue_sel = 0;
> > +    vdev->broken = 0;
> 
> false

OK.

> 
> >      vdev->config_vector = VIRTIO_NO_VECTOR;
> >      vdev->vq = g_malloc0(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
> >      vdev->vm_running = runstate_is_running();
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index 3e54e90..5b16faa 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -121,6 +121,7 @@ struct VirtIODevice
> >      bool vm_running;
> >      VMChangeStateEntry *vmstate;
> >      char *bus_name;
> > +    bool broken;
> 
> I'd suggest to use a slightly more verbose name... needs_reset maybe? Or
> is_in_invalid_state?

OK, needs_reset works for me.

Thanks,
Fam