[Qemu-devel] [PATCH for-2.5] virtio-9p-device: add minimal unrealize handler

[Qemu-devel] [PATCH for-2.5] virtio-9p-device: add minimal unrealize handler

[Greg Kurz]

Since commit 4652f1640e029e1f2433fa77ba6af285 "virtio-9p: add savevm handlers",
if the user hot-unplugs a quiescent 9p device and live migrates, the source
QEMU crashes before migration completetion... This happens because virtio-9p
devices have a realize handler which calls virtio_init() and register_savevm().
Both calls store pointers to the device internals, that get dereferenced during
migration even if the device got unplugged.

This patch simply adds an unrealize handler to perform minimal cleanup and
avoid the crash. Hot unplug of non-quiescent 9p devices is still not supported
in QEMU, and not supported by linux guests either.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/9pfs/virtio-9p-device.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 944b5f5e9fcc..b42d3b30a027 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -145,6 +145,17 @@ out:
     v9fs_path_free(&path);
 }
 
+static void virtio_9p_device_unrealize(DeviceState *dev, Error **errp)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+    V9fsState *s = VIRTIO_9P(dev);
+
+    virtio_cleanup(vdev);
+    unregister_savevm(dev, "virtio-9p", s);
+    g_free(s->ctx.fs_root);
+    g_free(s->tag);
+}
+
 /* virtio-9p device */
 
 static Property virtio_9p_properties[] = {
@@ -161,6 +172,7 @@ static void virtio_9p_class_init(ObjectClass *klass, void *data)
     dc->props = virtio_9p_properties;
     set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
     vdc->realize = virtio_9p_device_realize;
+    vdc->unrealize = virtio_9p_device_unrealize;
     vdc->get_features = virtio_9p_get_features;
     vdc->get_config = virtio_9p_get_config;
 }

Re: [Qemu-devel] [PATCH for-2.5] virtio-9p-device: add minimal unrealize handler

[Michael S. Tsirkin]

On Tue, Dec 08, 2015 at 04:54:57PM +0100, Greg Kurz wrote:
> Since commit 4652f1640e029e1f2433fa77ba6af285 "virtio-9p: add savevm handlers",
> if the user hot-unplugs a quiescent 9p device and live migrates, the source
> QEMU crashes before migration completetion... This happens because virtio-9p
> devices have a realize handler which calls virtio_init() and register_savevm().
> Both calls store pointers to the device internals, that get dereferenced during
> migration even if the device got unplugged.
> 
> This patch simply adds an unrealize handler to perform minimal cleanup and
> avoid the crash. Hot unplug of non-quiescent 9p devices is still not supported
> in QEMU, and not supported by linux guests either.
> 
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/9pfs/virtio-9p-device.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 944b5f5e9fcc..b42d3b30a027 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -145,6 +145,17 @@ out:
>      v9fs_path_free(&path);
>  }
>  
> +static void virtio_9p_device_unrealize(DeviceState *dev, Error **errp)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> +    V9fsState *s = VIRTIO_9P(dev);
> +
> +    virtio_cleanup(vdev);
> +    unregister_savevm(dev, "virtio-9p", s);
> +    g_free(s->ctx.fs_root);
> +    g_free(s->tag);
> +}
> +
>  /* virtio-9p device */
>  
>  static Property virtio_9p_properties[] = {
> @@ -161,6 +172,7 @@ static void virtio_9p_class_init(ObjectClass *klass, void *data)
>      dc->props = virtio_9p_properties;
>      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>      vdc->realize = virtio_9p_device_realize;
> +    vdc->unrealize = virtio_9p_device_unrealize;
>      vdc->get_features = virtio_9p_get_features;
>      vdc->get_config = virtio_9p_get_config;
>  }

Re: [Qemu-devel] [PATCH for-2.5] virtio-9p-device: add minimal unrealize handler

[Peter Maydell]

On 8 December 2015 at 16:19, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Dec 08, 2015 at 04:54:57PM +0100, Greg Kurz wrote:
>> Since commit 4652f1640e029e1f2433fa77ba6af285 "virtio-9p: add savevm handlers",
>> if the user hot-unplugs a quiescent 9p device and live migrates, the source
>> QEMU crashes before migration completetion... This happens because virtio-9p
>> devices have a realize handler which calls virtio_init() and register_savevm().
>> Both calls store pointers to the device internals, that get dereferenced during
>> migration even if the device got unplugged.
>>
>> This patch simply adds an unrealize handler to perform minimal cleanup and
>> avoid the crash. Hot unplug of non-quiescent 9p devices is still not supported
>> in QEMU, and not supported by linux guests either.
>>
>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Applied to master, thanks. (I wrapped the lines in the commit message
which were a bit too long; wrap at 75 chars or so is recommended so
that when you read the commit via 'git log' on an 80 column terminal
they still fit.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.5] virtio-9p-device: add minimal unrealize handler

[Greg Kurz]

On Thu, 10 Dec 2015 11:17:09 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 8 December 2015 at 16:19, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Dec 08, 2015 at 04:54:57PM +0100, Greg Kurz wrote:
> >> Since commit 4652f1640e029e1f2433fa77ba6af285 "virtio-9p: add savevm handlers",
> >> if the user hot-unplugs a quiescent 9p device and live migrates, the source
> >> QEMU crashes before migration completetion... This happens because virtio-9p
> >> devices have a realize handler which calls virtio_init() and register_savevm().
> >> Both calls store pointers to the device internals, that get dereferenced during
> >> migration even if the device got unplugged.
> >>
> >> This patch simply adds an unrealize handler to perform minimal cleanup and
> >> avoid the crash. Hot unplug of non-quiescent 9p devices is still not supported
> >> in QEMU, and not supported by linux guests either.
> >>
> >> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> >
> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Applied to master, thanks. (I wrapped the lines in the commit message
> which were a bit too long; wrap at 75 chars or so is recommended so
> that when you read the commit via 'git log' on an 80 column terminal
> they still fit.)
> 
> thanks
> -- PMM
> 

Oops my bad. I'll :set textwidth accordingly for my future posts.

Thanks.

--
Greg