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

[Michael S. Tsirkin]

On Tue, Apr 22, 2014 at 04:55:15PM +0800, Fam Zheng wrote:
> 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>

I'm assuming the idea is to make debugging guest drivers easier
for people not familiar with qemu?
I'd like this motivation reflected in the change log.

With that in mind, this patch might help some developers by not making
guest crash, but might also interfere with others by hiding the
real cause of the bug.


> ---
>  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;
>      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;
>  };
>  
>  typedef struct VirtioDeviceClass {
> @@ -211,6 +212,8 @@ void virtio_queue_notify(VirtIODevice *vdev, int n);
>  uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
>  void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector);
>  void virtio_set_status(VirtIODevice *vdev, uint8_t val);
> +void virtio_set_broken(VirtIODevice *vdev);
> +bool virtio_broken(VirtIODevice *vdev);
>  void virtio_reset(void *opaque);
>  void virtio_update_irq(VirtIODevice *vdev);
>  int virtio_set_features(VirtIODevice *vdev, uint32_t val);
> -- 
> 1.9.2