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(-)

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;
     }
-- 
2.17.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

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

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