On Tue, 16 Jul 2019 at 22:42, Stefan Hajnoczi <stefa...@gmail.com> wrote:
>
> On Mon, Jul 15, 2019 at 06:23:25PM +0800, elohi...@gmail.com wrote:
> > From: Xie Yongji <xieyon...@baidu.com>
> >
> > This avoids memory leak when device hotplug is failed.
> >
> > Signed-off-by: Xie Yongji <xieyon...@baidu.com>
> > ---
> > hw/scsi/vhost-scsi.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> > index 4090f99ee4..db4a090576 100644
> > --- a/hw/scsi/vhost-scsi.c
> > +++ b/hw/scsi/vhost-scsi.c
> > @@ -210,7 +210,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error
> > **errp)
> > if (err) {
> > error_propagate(errp, err);
> > error_free(vsc->migration_blocker);
> > - goto close_fd;
> > + goto free_virtio;
> > }
> > }
> >
> > @@ -240,6 +240,8 @@ static void vhost_scsi_realize(DeviceState *dev, Error
> > **errp)
> > migrate_del_blocker(vsc->migration_blocker);
> > }
> > g_free(vsc->dev.vqs);
> > + free_virtio:
> > + virtio_scsi_common_unrealize(dev, errp);
>
> error_set*() requires that *errp == NULL:
>
> static void error_setv(Error **errp, ...
> ...
> assert(*errp == NULL);
>
> Today virtio_scsi_common_unrealize() doesn't use the errp argument but
> if it ever uses it then QEMU will hit an assertion failure.
>
> Please do this instead:
>
> virtio_scsi_common_unrealize(dev, &error_abort);
>
> If virtio_scsi_common_unrealize() ever produces an error there will be
> an message explaining that errors are unexpected.
>
> This also applies to Patch 2.
>
> Alternatively you could do this to handle all cases and propagate the
> error:
>
> Error *local_err = NULL;
> virtio_scsi_common_unrealize(dev, &local_err);
> error_propagate(errp, local_err);
Will fix it in v2. Thank you.
Thanks,
Yongji
