Re: [Qemu-devel] [PATCH] vhost-user: Slave crashes as Master unmaps vrings during guest reboot

[Michael S. Tsirkin]

On Fri, Jan 15, 2016 at 12:12:44PM -0800, Shesha Sreenivasamurthy wrote:
> Send VHOST_USER_RESET_OWNER when the device is stopped.
> 
> Signed-off-by: Shesha Sreenivasamurthy <address@hidden>

That's a bad commit log.  A good one should describe why changes are
made, not what they are (that can be seen from the change diff).
The cover letter is no good for that, it
should just give some introduction in case of a large patchset.

I commented on the cover letter for now since that is where
you put the motivation.

> ---
>  hw/virtio/vhost.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index de29968..808184f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1256,6 +1256,11 @@ void vhost_dev_stop(struct vhost_dev *hdev, 
> VirtIODevice *vdev)
>                               hdev->vq_index + i);
>      }
>  

If rings keep going at this point then the index
retrieved above will be wrong, and we will not
be able to re-start the device.

> +    if (hdev->vhost_ops->vhost_reset_device(hdev) < 0) {
> +        fprintf(stderr, "vhost reset device %s failed\n", vdev->name);
> +        fflush(stderr);
> +    }
> +


Looks more or less like a revert of
        commit 12b8cbac3c8243b3dd485aaebb82547aefa06adb
        Author: Yuanhan Liu <address@hidden>
        Date:   Fri Nov 13 15:24:10 2015 +0800

            vhost: don't send RESET_OWNER at stop

If you still think it's a good idea, pls
copy people signed on that patch.


>      vhost_log_put(hdev, true);
>      hdev->started = false;
>      hdev->log = NULL;
> -- 
> 1.9.5 (Apple Git-50.3)