2019-09-06 05:14:42

by Vivek Goyal

[permalink] [raw]
Subject: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

Hi,

Michael Tsirkin pointed out issues w.r.t various locking related TODO
items and races w.r.t device removal.

In this first round of cleanups, I have taken care of most pressing
issues.

These patches apply on top of following.

git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4

I have tested these patches with mount/umount and device removal using
qemu monitor. For example.

virsh qemu-monitor-command --hmp vm9-f28 device_del myvirtiofs

Now a mounted device can be removed and file system will return errors
-ENOTCONN and one can unmount it.

Miklos, if you are fine with the patches, I am fine if you fold these
all into existing patch. I kept them separate so that review is easier.

Any feedback or comments are welcome.

Thanks
Vivek


Vivek Goyal (18):
virtiofs: Remove request from processing list before calling end
virtiofs: Check whether hiprio queue is connected at submission time
virtiofs: Pass fsvq instead of vq as parameter to
virtio_fs_enqueue_req
virtiofs: Check connected state for VQ_REQUEST queue as well
Maintain count of in flight requests for VQ_REQUEST queue
virtiofs: ->remove should not clean virtiofs fuse devices
virtiofs: Stop virtiofs queues when device is being removed
virtiofs: Drain all pending requests during ->remove time
virtiofs: Add an helper to start all the queues
virtiofs: Do not use device managed mem for virtio_fs and virtio_fs_vq
virtiofs: stop and drain queues after sending DESTROY
virtiofs: Use virtio_fs_free_devs() in error path
virtiofs: Do not access virtqueue in request submission path
virtiofs: Add a fuse_iqueue operation to put() reference
virtiofs: Make virtio_fs object refcounted
virtiofs: Use virtio_fs_mutex for races w.r.t ->remove and mount path
virtiofs: Remove TODO to quiesce/end_requests
virtiofs: Remove TODO item from virtio_fs_free_devs()

fs/fuse/fuse_i.h | 5 +
fs/fuse/inode.c | 6 +
fs/fuse/virtio_fs.c | 259 ++++++++++++++++++++++++++++++++------------
3 files changed, 198 insertions(+), 72 deletions(-)

--
2.20.1


2019-09-06 06:03:10

by Vivek Goyal

[permalink] [raw]
Subject: [PATCH 04/18] virtiofs: Check connected state for VQ_REQUEST queue as well

Right now we are checking ->connected state only for VQ_HIPRIO. Now we want
to make use of this method for all queues. So check it for VQ_REQUEST as
well.

This will be helpful if device has been removed and virtqueue is gone. In
that case ->connected will be false and request can't be submitted anymore
and user space will see error -ENOTCONN.

Signed-off-by: Vivek Goyal <[email protected]>
---
fs/fuse/virtio_fs.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 9d30530e3ca9..c46dd4d284d6 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -755,6 +755,12 @@ static int virtio_fs_enqueue_req(struct virtio_fs_vq *fsvq,

spin_lock(&fsvq->lock);

+ if (!fsvq->connected) {
+ spin_unlock(&fsvq->lock);
+ ret = -ENOTCONN;
+ goto out;
+ }
+
vq = fsvq->vq;
ret = virtqueue_add_sgs(vq, sgs, out_sgs, in_sgs, req, GFP_ATOMIC);
if (ret < 0) {
--
2.20.1

2019-09-06 14:11:19

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
>
> Hi,
>
> Michael Tsirkin pointed out issues w.r.t various locking related TODO
> items and races w.r.t device removal.
>
> In this first round of cleanups, I have taken care of most pressing
> issues.
>
> These patches apply on top of following.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
>
> I have tested these patches with mount/umount and device removal using
> qemu monitor. For example.

Is device removal mandatory? Can't this be made a non-removable
device? Is there a good reason why removing the virtio-fs device
makes sense?

Thanks,
Miklos

2019-09-06 15:13:04

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi <[email protected]> wrote:
>
> On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
> > On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
> > >
> > > Hi,
> > >
> > > Michael Tsirkin pointed out issues w.r.t various locking related TODO
> > > items and races w.r.t device removal.
> > >
> > > In this first round of cleanups, I have taken care of most pressing
> > > issues.
> > >
> > > These patches apply on top of following.
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
> > >
> > > I have tested these patches with mount/umount and device removal using
> > > qemu monitor. For example.
> >
> > Is device removal mandatory? Can't this be made a non-removable
> > device? Is there a good reason why removing the virtio-fs device
> > makes sense?
>
> Hot plugging and unplugging virtio PCI adapters is common. I'd very
> much like removal to work from the beginning.

Can you give an example use case?

Thanks,
Miklos

2019-09-06 15:49:00

by Stefan Hajnoczi

[permalink] [raw]
Subject: Re: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
> On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
> >
> > Hi,
> >
> > Michael Tsirkin pointed out issues w.r.t various locking related TODO
> > items and races w.r.t device removal.
> >
> > In this first round of cleanups, I have taken care of most pressing
> > issues.
> >
> > These patches apply on top of following.
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
> >
> > I have tested these patches with mount/umount and device removal using
> > qemu monitor. For example.
>
> Is device removal mandatory? Can't this be made a non-removable
> device? Is there a good reason why removing the virtio-fs device
> makes sense?

Hot plugging and unplugging virtio PCI adapters is common. I'd very
much like removal to work from the beginning.

Stefan


Attachments:
(No filename) (902.00 B)
signature.asc (499.00 B)
Download all attachments

2019-09-06 16:00:05

by Stefan Hajnoczi

[permalink] [raw]
Subject: Re: [PATCH 04/18] virtiofs: Check connected state for VQ_REQUEST queue as well

On Thu, Sep 05, 2019 at 03:48:45PM -0400, Vivek Goyal wrote:
> Right now we are checking ->connected state only for VQ_HIPRIO. Now we want
> to make use of this method for all queues. So check it for VQ_REQUEST as
> well.
>
> This will be helpful if device has been removed and virtqueue is gone. In
> that case ->connected will be false and request can't be submitted anymore
> and user space will see error -ENOTCONN.
>
> Signed-off-by: Vivek Goyal <[email protected]>
> ---
> fs/fuse/virtio_fs.c | 6 ++++++
> 1 file changed, 6 insertions(+)

Reviewed-by: Stefan Hajnoczi <[email protected]>


Attachments:
(No filename) (615.00 B)
signature.asc (499.00 B)
Download all attachments

2019-09-06 17:51:10

by Vivek Goyal

[permalink] [raw]
Subject: Re: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Fri, Sep 06, 2019 at 01:52:41PM +0200, Miklos Szeredi wrote:
> On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi <[email protected]> wrote:
> >
> > On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
> > > On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
> > > >
> > > > Hi,
> > > >
> > > > Michael Tsirkin pointed out issues w.r.t various locking related TODO
> > > > items and races w.r.t device removal.
> > > >
> > > > In this first round of cleanups, I have taken care of most pressing
> > > > issues.
> > > >
> > > > These patches apply on top of following.
> > > >
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
> > > >
> > > > I have tested these patches with mount/umount and device removal using
> > > > qemu monitor. For example.
> > >
> > > Is device removal mandatory? Can't this be made a non-removable
> > > device? Is there a good reason why removing the virtio-fs device
> > > makes sense?
> >
> > Hot plugging and unplugging virtio PCI adapters is common. I'd very
> > much like removal to work from the beginning.
>
> Can you give an example use case?

David Gilbert mentioned this could be useful if daemon stops responding
or dies. One could remove device. That will fail all future requests
and allow unmounting filesystem.

Havind said that, current implementation will help in above situation
only if there are no pending requests. If there are pending requests
and daemon stops responding, then removal will hang too, as we wait
for draining the queues.

So at some point of time, we also need some sort of timeout functionality
where we end requests with error after a timeout.

I feel we should support removing device at some point of time. But its
not necessarily a must have feature for first round.

Thanks
Vivek

2019-09-06 18:49:35

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Thu, Sep 05, 2019 at 03:48:41PM -0400, Vivek Goyal wrote:
> Hi,
>
> Michael Tsirkin pointed out issues w.r.t various locking related TODO
> items and races w.r.t device removal.
>
> In this first round of cleanups, I have taken care of most pressing
> issues.
>
> These patches apply on top of following.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
>
> I have tested these patches with mount/umount and device removal using
> qemu monitor. For example.
>
> virsh qemu-monitor-command --hmp vm9-f28 device_del myvirtiofs
>
> Now a mounted device can be removed and file system will return errors
> -ENOTCONN and one can unmount it.
>
> Miklos, if you are fine with the patches, I am fine if you fold these
> all into existing patch. I kept them separate so that review is easier.
>
> Any feedback or comments are welcome.
>
> Thanks
> Vivek
>


Another version of a patch with fixes all rolled in would also
be nice.
> Vivek Goyal (18):
> virtiofs: Remove request from processing list before calling end
> virtiofs: Check whether hiprio queue is connected at submission time
> virtiofs: Pass fsvq instead of vq as parameter to
> virtio_fs_enqueue_req
> virtiofs: Check connected state for VQ_REQUEST queue as well
> Maintain count of in flight requests for VQ_REQUEST queue
> virtiofs: ->remove should not clean virtiofs fuse devices
> virtiofs: Stop virtiofs queues when device is being removed
> virtiofs: Drain all pending requests during ->remove time
> virtiofs: Add an helper to start all the queues
> virtiofs: Do not use device managed mem for virtio_fs and virtio_fs_vq
> virtiofs: stop and drain queues after sending DESTROY
> virtiofs: Use virtio_fs_free_devs() in error path
> virtiofs: Do not access virtqueue in request submission path
> virtiofs: Add a fuse_iqueue operation to put() reference
> virtiofs: Make virtio_fs object refcounted
> virtiofs: Use virtio_fs_mutex for races w.r.t ->remove and mount path
> virtiofs: Remove TODO to quiesce/end_requests
> virtiofs: Remove TODO item from virtio_fs_free_devs()
>
> fs/fuse/fuse_i.h | 5 +
> fs/fuse/inode.c | 6 +
> fs/fuse/virtio_fs.c | 259 ++++++++++++++++++++++++++++++++------------
> 3 files changed, 198 insertions(+), 72 deletions(-)
>
> --
> 2.20.1

2019-09-06 19:15:35

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Fri, Sep 6, 2019 at 3:57 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Fri, Sep 06, 2019 at 08:08:17AM -0400, Vivek Goyal wrote:
> > On Fri, Sep 06, 2019 at 01:52:41PM +0200, Miklos Szeredi wrote:
> > > On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi <[email protected]> wrote:
> > > >
> > > > On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
> > > > > On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > Michael Tsirkin pointed out issues w.r.t various locking related TODO
> > > > > > items and races w.r.t device removal.
> > > > > >
> > > > > > In this first round of cleanups, I have taken care of most pressing
> > > > > > issues.
> > > > > >
> > > > > > These patches apply on top of following.
> > > > > >
> > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
> > > > > >
> > > > > > I have tested these patches with mount/umount and device removal using
> > > > > > qemu monitor. For example.
> > > > >
> > > > > Is device removal mandatory? Can't this be made a non-removable
> > > > > device? Is there a good reason why removing the virtio-fs device
> > > > > makes sense?
> > > >
> > > > Hot plugging and unplugging virtio PCI adapters is common. I'd very
> > > > much like removal to work from the beginning.
> > >
> > > Can you give an example use case?
> >
> > David Gilbert mentioned this could be useful if daemon stops responding
> > or dies. One could remove device. That will fail all future requests
> > and allow unmounting filesystem.
> >
> > Havind said that, current implementation will help in above situation
> > only if there are no pending requests. If there are pending requests
> > and daemon stops responding, then removal will hang too, as we wait
> > for draining the queues.
> >
> > So at some point of time, we also need some sort of timeout functionality
> > where we end requests with error after a timeout.
> >
> > I feel we should support removing device at some point of time. But its
> > not necessarily a must have feature for first round.
> >
> > Thanks
> > Vivek
>
> Without removal how do we stop guest poking at some files if we want to?
>
> I guess we could invent a special event to block accesses,
> but unplug will just do it.
>
> blk and scsi support removal out of box, if this is supposed
> to be a drop in replacement then I think yes, you want this
> support.

This is not a drop in replacement for blk and scsi transports. More
for virtio-9p. Does that have anything similar?

If we get a request for this feature, then yes, what you are saying
makes sense. But that hasn't happened yet, so I think this can wait.

Thanks,
Miklos

2019-09-06 22:19:28

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Fri, Sep 6, 2019 at 2:08 PM Vivek Goyal <[email protected]> wrote:
>
> On Fri, Sep 06, 2019 at 01:52:41PM +0200, Miklos Szeredi wrote:
> > On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi <[email protected]> wrote:
> > >
> > > On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
> > > > On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > Michael Tsirkin pointed out issues w.r.t various locking related TODO
> > > > > items and races w.r.t device removal.
> > > > >
> > > > > In this first round of cleanups, I have taken care of most pressing
> > > > > issues.
> > > > >
> > > > > These patches apply on top of following.
> > > > >
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
> > > > >
> > > > > I have tested these patches with mount/umount and device removal using
> > > > > qemu monitor. For example.
> > > >
> > > > Is device removal mandatory? Can't this be made a non-removable
> > > > device? Is there a good reason why removing the virtio-fs device
> > > > makes sense?
> > >
> > > Hot plugging and unplugging virtio PCI adapters is common. I'd very
> > > much like removal to work from the beginning.
> >
> > Can you give an example use case?
>
> David Gilbert mentioned this could be useful if daemon stops responding
> or dies. One could remove device. That will fail all future requests
> and allow unmounting filesystem.
>
> Havind said that, current implementation will help in above situation
> only if there are no pending requests. If there are pending requests
> and daemon stops responding, then removal will hang too, as we wait
> for draining the queues.
>
> So at some point of time, we also need some sort of timeout functionality
> where we end requests with error after a timeout.
>
> I feel we should support removing device at some point of time. But its
> not necessarily a must have feature for first round.

If there's no compelling reason to do it in the first round, than I'd
prefer to not do it. More complexity -> more bugs.

Thanks,
Miklos

2019-09-06 22:23:42

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Fri, Sep 06, 2019 at 08:08:17AM -0400, Vivek Goyal wrote:
> On Fri, Sep 06, 2019 at 01:52:41PM +0200, Miklos Szeredi wrote:
> > On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi <[email protected]> wrote:
> > >
> > > On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
> > > > On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > Michael Tsirkin pointed out issues w.r.t various locking related TODO
> > > > > items and races w.r.t device removal.
> > > > >
> > > > > In this first round of cleanups, I have taken care of most pressing
> > > > > issues.
> > > > >
> > > > > These patches apply on top of following.
> > > > >
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
> > > > >
> > > > > I have tested these patches with mount/umount and device removal using
> > > > > qemu monitor. For example.
> > > >
> > > > Is device removal mandatory? Can't this be made a non-removable
> > > > device? Is there a good reason why removing the virtio-fs device
> > > > makes sense?
> > >
> > > Hot plugging and unplugging virtio PCI adapters is common. I'd very
> > > much like removal to work from the beginning.
> >
> > Can you give an example use case?
>
> David Gilbert mentioned this could be useful if daemon stops responding
> or dies. One could remove device. That will fail all future requests
> and allow unmounting filesystem.
>
> Havind said that, current implementation will help in above situation
> only if there are no pending requests. If there are pending requests
> and daemon stops responding, then removal will hang too, as we wait
> for draining the queues.
>
> So at some point of time, we also need some sort of timeout functionality
> where we end requests with error after a timeout.
>
> I feel we should support removing device at some point of time. But its
> not necessarily a must have feature for first round.
>
> Thanks
> Vivek

Without removal how do we stop guest poking at some files if we want to?

I guess we could invent a special event to block accesses,
but unplug will just do it.

blk and scsi support removal out of box, if this is supposed
to be a drop in replacement then I think yes, you want this
support.

--
MST

2019-09-06 23:34:58

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Fri, Sep 06, 2019 at 04:11:45PM +0200, Miklos Szeredi wrote:
> This is not a drop in replacement for blk and scsi transports. More
> for virtio-9p. Does that have anything similar?

9p seems to supports unplug, yes. It's not great in that it
blocks until we close the channel, but it's there and
it does not crash or leak memory.

2019-09-09 09:43:01

by piaojun

[permalink] [raw]
Subject: Re: [Virtio-fs] [PATCH 00/18] virtiofs: Fix various races and cleanups round 1



On 2019/9/6 19:52, Miklos Szeredi wrote:
> On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi <[email protected]> wrote:
>>
>> On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
>>> On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Michael Tsirkin pointed out issues w.r.t various locking related TODO
>>>> items and races w.r.t device removal.
>>>>
>>>> In this first round of cleanups, I have taken care of most pressing
>>>> issues.
>>>>
>>>> These patches apply on top of following.
>>>>
>>>> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
>>>>
>>>> I have tested these patches with mount/umount and device removal using
>>>> qemu monitor. For example.
>>>
>>> Is device removal mandatory? Can't this be made a non-removable
>>> device? Is there a good reason why removing the virtio-fs device
>>> makes sense?
>>
>> Hot plugging and unplugging virtio PCI adapters is common. I'd very
>> much like removal to work from the beginning.
>
> Can you give an example use case?

I think VirtFS migration need hot plugging, or it may cause QEMU crash
or some problems.

Thanks,
Jun

2019-09-10 18:33:29

by Stefan Hajnoczi

[permalink] [raw]
Subject: Re: [Virtio-fs] [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Sun, Sep 08, 2019 at 07:53:55PM +0800, piaojun wrote:
>
>
> On 2019/9/6 19:52, Miklos Szeredi wrote:
> > On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi <[email protected]> wrote:
> >>
> >> On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
> >>> On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> Michael Tsirkin pointed out issues w.r.t various locking related TODO
> >>>> items and races w.r.t device removal.
> >>>>
> >>>> In this first round of cleanups, I have taken care of most pressing
> >>>> issues.
> >>>>
> >>>> These patches apply on top of following.
> >>>>
> >>>> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
> >>>>
> >>>> I have tested these patches with mount/umount and device removal using
> >>>> qemu monitor. For example.
> >>>
> >>> Is device removal mandatory? Can't this be made a non-removable
> >>> device? Is there a good reason why removing the virtio-fs device
> >>> makes sense?
> >>
> >> Hot plugging and unplugging virtio PCI adapters is common. I'd very
> >> much like removal to work from the beginning.
> >
> > Can you give an example use case?
>
> I think VirtFS migration need hot plugging, or it may cause QEMU crash
> or some problems.

Live migration is currently unsupported. Hot unplugging the virtio-fs
device would allow the guest to live migrate successfully, so it's a
useful feature to work around the missing live migration support.

Is this what you mean?

Stefan


Attachments:
(No filename) (1.53 kB)
signature.asc (499.00 B)
Download all attachments

2019-09-10 18:34:13

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [Virtio-fs] [PATCH 00/18] virtiofs: Fix various races and cleanups round 1

On Mon, Sep 09, 2019 at 06:14:55PM +0200, Stefan Hajnoczi wrote:
> On Sun, Sep 08, 2019 at 07:53:55PM +0800, piaojun wrote:
> >
> >
> > On 2019/9/6 19:52, Miklos Szeredi wrote:
> > > On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi <[email protected]> wrote:
> > >>
> > >> On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
> > >>> On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
> > >>>>
> > >>>> Hi,
> > >>>>
> > >>>> Michael Tsirkin pointed out issues w.r.t various locking related TODO
> > >>>> items and races w.r.t device removal.
> > >>>>
> > >>>> In this first round of cleanups, I have taken care of most pressing
> > >>>> issues.
> > >>>>
> > >>>> These patches apply on top of following.
> > >>>>
> > >>>> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
> > >>>>
> > >>>> I have tested these patches with mount/umount and device removal using
> > >>>> qemu monitor. For example.
> > >>>
> > >>> Is device removal mandatory? Can't this be made a non-removable
> > >>> device? Is there a good reason why removing the virtio-fs device
> > >>> makes sense?
> > >>
> > >> Hot plugging and unplugging virtio PCI adapters is common. I'd very
> > >> much like removal to work from the beginning.
> > >
> > > Can you give an example use case?
> >
> > I think VirtFS migration need hot plugging, or it may cause QEMU crash
> > or some problems.
>
> Live migration is currently unsupported. Hot unplugging the virtio-fs
> device would allow the guest to live migrate successfully, so it's a
> useful feature to work around the missing live migration support.
>
> Is this what you mean?
>
> Stefan

Exactly. That's what I also said. To add to that, hotplug can not be
negotiated, so it's not a feature we can easily add down the road if old
guests crash on unplug. Thus a driver that does not support unplug
should not claim to support removal.

--
MST

2019-09-10 18:46:43

by piaojun

[permalink] [raw]
Subject: Re: [Virtio-fs] [PATCH 00/18] virtiofs: Fix various races and cleanups round 1



On 2019/9/10 0:14, Stefan Hajnoczi wrote:
> On Sun, Sep 08, 2019 at 07:53:55PM +0800, piaojun wrote:
>>
>>
>> On 2019/9/6 19:52, Miklos Szeredi wrote:
>>> On Fri, Sep 6, 2019 at 12:36 PM Stefan Hajnoczi <[email protected]> wrote:
>>>>
>>>> On Fri, Sep 06, 2019 at 10:15:14AM +0200, Miklos Szeredi wrote:
>>>>> On Thu, Sep 5, 2019 at 9:49 PM Vivek Goyal <[email protected]> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Michael Tsirkin pointed out issues w.r.t various locking related TODO
>>>>>> items and races w.r.t device removal.
>>>>>>
>>>>>> In this first round of cleanups, I have taken care of most pressing
>>>>>> issues.
>>>>>>
>>>>>> These patches apply on top of following.
>>>>>>
>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#virtiofs-v4
>>>>>>
>>>>>> I have tested these patches with mount/umount and device removal using
>>>>>> qemu monitor. For example.
>>>>>
>>>>> Is device removal mandatory? Can't this be made a non-removable
>>>>> device? Is there a good reason why removing the virtio-fs device
>>>>> makes sense?
>>>>
>>>> Hot plugging and unplugging virtio PCI adapters is common. I'd very
>>>> much like removal to work from the beginning.
>>>
>>> Can you give an example use case?
>>
>> I think VirtFS migration need hot plugging, or it may cause QEMU crash
>> or some problems.
>
> Live migration is currently unsupported. Hot unplugging the virtio-fs
> device would allow the guest to live migrate successfully, so it's a
> useful feature to work around the missing live migration support.
>
> Is this what you mean?

Agreed, migration support is necessary for user, and hot
plugging/unplugging is also common for virtio device.

Jun