On Thu, 30 Mar 2017 08:25:25 -0500 Eric Blake <ebl...@redhat.com> wrote:
> On 03/30/2017 07:27 AM, Li Qiang wrote:
> > The guest can leave the pdu->s->migration_blocker exists by attach
>
> s/exists/in place/
> s/attach/attaching/
>
> > but not remove a fid. Then if we hot unplug the 9pfs device, the
>
In theory you're right, but the current 9p client in linux won't let you hot
unplug the device unless you unmount the 9p share first, hence freeing the
blocker.
> s/remove/removing/
>
> > v9fs_reset() just free the fids, but not free the migration_blocker.
> > This will leak a memory leak. This patch avoid this.
I had a similar issue sitting my TODO list for quite a time: the blocker
survives a system_reset. It doesn't cause a memory leak but it prevents
migration until the guest mounts/unmounts the 9p share again.
This boils down to virtfs_reset() calling free_fid() instead of put_fid() IIRC.
>
> s/leak a/cause a/
> s/avoid/avoids/
>
> >
> > Signed-off-by: Li Qiang <liqiang...@360.cn>
> > ---
> > hw/9pfs/9p.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
>
> Probably worth including in 2.9 as a bug fix.
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
>
> >
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 48babce..b55c02d 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -548,6 +548,12 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
> > free_fid(pdu, fidp);
> > }
> > }
> > +
> > + if (pdu->s->migration_blocker) {
> > + migrate_del_blocker(pdu->s->migration_blocker);
> > + error_free(pdu->s->migration_blocker);
> > + pdu->s->migration_blocker = NULL;
> > + }
I'd prefer to drain all PDUs in virtfs_reset() and have the loop above
to call put_fid() instead of free_fid(). If this isn't doable for 2.9,
I'll apply this patch with a comment.
> > }
> >
> > #define P9_QID_TYPE_DIR 0x80
> >
>
pgpHd0nnIUceV.pgp
Description: OpenPGP digital signature
