diff mbox series

[V2,1/2] block/nfs: tear down aio before nfs_close

Message ID 20190910154110.6905-2-pl@kamp.de (mailing list archive)
State New, archived
Headers show
Series add support for nfs_umount | expand

Commit Message

Peter Lieven Sept. 10, 2019, 3:41 p.m. UTC
nfs_close is a sync call from libnfs and has its own event
handler polling on the nfs FD. Avoid that both QEMU and libnfs
are intefering here.

CC: qemu-stable@nongnu.org
Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/nfs.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Max Reitz Sept. 13, 2019, 9:51 a.m. UTC | #1
On 10.09.19 17:41, Peter Lieven wrote:
> nfs_close is a sync call from libnfs and has its own event
> handler polling on the nfs FD. Avoid that both QEMU and libnfs
> are intefering here.
> 
> CC: qemu-stable@nongnu.org
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  block/nfs.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

I’ve just seen that Kevin has already included the second patch (in its
v1) in his pull request.

So all that I can do is take this patch, which sounds good to me,
especially since Ronnie has agreed that we should remove our FD handler
there.

(So I’ve rebased the patch on top of Kevin’s pull request, and I’ve
taken it to my block branch.)

Max
Peter Lieven Sept. 13, 2019, 10:15 a.m. UTC | #2
Am 13.09.19 um 11:51 schrieb Max Reitz:
> On 10.09.19 17:41, Peter Lieven wrote:
>> nfs_close is a sync call from libnfs and has its own event
>> handler polling on the nfs FD. Avoid that both QEMU and libnfs
>> are intefering here.
>>
>> CC: qemu-stable@nongnu.org
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>  block/nfs.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
> I’ve just seen that Kevin has already included the second patch (in its
> v1) in his pull request.
>
> So all that I can do is take this patch, which sounds good to me,
> especially since Ronnie has agreed that we should remove our FD handler
> there.
>
> (So I’ve rebased the patch on top of Kevin’s pull request, and I’ve
> taken it to my block branch.)


Thank you. After I discovered that we had this bug also before I added the nfs_umount call I thought

it would be good to have this patch first and the add the umount call because the fix should also go into

stable because in theory we could also run into trouble with just the *sync* nfs_clsoe call.


Peter
Kevin Wolf Sept. 13, 2019, 10:21 a.m. UTC | #3
Am 13.09.2019 um 11:51 hat Max Reitz geschrieben:
> On 10.09.19 17:41, Peter Lieven wrote:
> > nfs_close is a sync call from libnfs and has its own event
> > handler polling on the nfs FD. Avoid that both QEMU and libnfs
> > are intefering here.
> > 
> > CC: qemu-stable@nongnu.org
> > Signed-off-by: Peter Lieven <pl@kamp.de>
> > ---
> >  block/nfs.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> I’ve just seen that Kevin has already included the second patch (in its
> v1) in his pull request.

Oops, sorry about this. Peter hasn't pulled yet, so I'll update the tag
for the pull request. Let's see which version gets merged in the end.

Kevin
diff mbox series

Patch

diff --git a/block/nfs.c b/block/nfs.c
index 0ec50953e4..2c98508275 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -390,12 +390,14 @@  static void nfs_attach_aio_context(BlockDriverState *bs,
 static void nfs_client_close(NFSClient *client)
 {
     if (client->context) {
+        qemu_mutex_lock(&client->mutex);
+        aio_set_fd_handler(client->aio_context, nfs_get_fd(client->context),
+                           false, NULL, NULL, NULL, NULL);
+        qemu_mutex_unlock(&client->mutex);
         if (client->fh) {
             nfs_close(client->context, client->fh);
             client->fh = NULL;
         }
-        aio_set_fd_handler(client->aio_context, nfs_get_fd(client->context),
-                           false, NULL, NULL, NULL, NULL);
         nfs_destroy_context(client->context);
         client->context = NULL;
     }