Re: [Qemu-devel] [PATCH] block/nfs: add support for nfs_umount

[Peter Lieven]

> Am 03.09.2019 um 16:56 schrieb Kevin Wolf <address@hidden>:
> 
> Am 03.09.2019 um 15:44 hat Peter Lieven geschrieben:
>> libnfs recently added support for unmounting. Add support
>> in Qemu too.
>> 
>> Signed-off-by: Peter Lieven <address@hidden>
> 
> Looks trivial enough to review even for me. :-)
> 
> Thanks, applied to the block branch.
> 
> Kevin


I am not sure what the reason is, but with this patch I sometimes run into 
nfs_process_read being called for a cdrom mounted from nfs after I ejected it 
(and the whole nfs client context is already destroyed).

It seems that the following fixes the issue. It might be that the nfs_umount 
call just reveals this bug. nfs_close is also doing sync I/O with the libnfs 
library. If we mount a nfs share we are doing everything with sync calls and 
then set up the aio stuff with nfs_set_events. I think that we need to stop the 
aio before we are executing the sync calls to nfs_close and nfs_umount.
Also not sure if holding the mutex is necessary here.

diff --git a/block/nfs.c b/block/nfs.c
index ffa6484c1a..cb2e0d375a 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -389,6 +389,10 @@ 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;
@@ -396,8 +400,6 @@ static void nfs_client_close(NFSClient *client)
 #ifdef LIBNFS_FEATURE_UMOUNT
         nfs_umount(client->context);
 #endif
-        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;
     }


Peter