[Qemu-devel] [for-2.9 PATCH 2/3] 9pfs: cancel active PDUs in virtfs_reset()

Greg Kurz posted 3 patches 8 years, 7 months ago
[Qemu-devel] [for-2.9 PATCH 2/3] 9pfs: cancel active PDUs in virtfs_reset()
Posted by Greg Kurz 8 years, 7 months ago
According to the 9P spec [1], the version operation should abort any
outstanding I/O and clunk all fids, so that a new session may be started
in a clean state.

The current code tries to clunk and free fids, but it doesn't wait for
active PDUs to complete. This can cause an I/O to actually complete after
the new session has begun, and confuse the client.

This patch modifies virtfs_reset() so that it explicitely cancels and waits
for inflight requests to terminate. All fids should thus be unreferenced
and ready to be freed. Let's make it clear with a an assertion.

[1] http://man.cat-v.org/plan_9/5/version

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index cc109367b030..86ed9065c4e2 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -536,9 +536,29 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
 {
     V9fsState *s = pdu->s;
     V9fsFidState *fidp;
+    bool done = false;
+
+    /* Drain any outstanding I/O */
+    while (!done) {
+        V9fsPDU *cancel_pdu;
+
+        done = true;
+        QLIST_FOREACH(cancel_pdu, &s->active_list, next) {
+            if (cancel_pdu != pdu) {
+                done = false;
+                cancel_pdu->cancelled = 1;
+                qemu_co_queue_wait(&cancel_pdu->complete, NULL);
+                cancel_pdu->cancelled = 0;
+                pdu_free(cancel_pdu);
+                break;
+            }
+        }
+    }
 
     /* Free all fids */
     while (s->fid_list) {
+        assert(!fidp->ref);
+
         /* Get fid */
         fidp = s->fid_list;
         fidp->ref++;
@@ -670,7 +690,7 @@ static void coroutine_fn pdu_complete(V9fsPDU *pdu, ssize_t len)
 
     pdu_push_and_notify(pdu);
 
-    /* Now wakeup anybody waiting in flush for this request */
+    /* Now wakeup anybody waiting in flush or reset for this request */
     if (!qemu_co_queue_next(&pdu->complete)) {
         pdu_free(pdu);
     }


Re: [Qemu-devel] [for-2.9 PATCH 2/3] 9pfs: cancel active PDUs in virtfs_reset()
Posted by 李强 8 years, 7 months ago

> -----Original Message-----
> From: Greg Kurz [mailto:groug@kaod.org]
> Sent: Friday, March 31, 2017 7:27 PM
> To: qemu-devel@nongnu.org
> Cc: Eric Blake; 李强; Greg Kurz
> Subject: [for-2.9 PATCH 2/3] 9pfs: cancel active PDUs in virtfs_reset()
> 
> According to the 9P spec [1], the version operation should abort any
> outstanding I/O and clunk all fids, so that a new session may be started in a
> clean state.
> 
> The current code tries to clunk and free fids, but it doesn't wait for active PDUs
> to complete. This can cause an I/O to actually complete after the new session
> has begun, and confuse the client.
> 
> This patch modifies virtfs_reset() so that it explicitely cancels and waits for
> inflight requests to terminate. All fids should thus be unreferenced and ready to
> be freed. Let's make it clear with a an assertion.
> 
> [1] http://man.cat-v.org/plan_9/5/version
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---

Reviewed-by: Li Qiang <liqiang6-s@360.cn>

>  hw/9pfs/9p.c |   22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index cc109367b030..86ed9065c4e2
> 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -536,9 +536,29 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
> {
>      V9fsState *s = pdu->s;
>      V9fsFidState *fidp;
> +    bool done = false;
> +
> +    /* Drain any outstanding I/O */
> +    while (!done) {
> +        V9fsPDU *cancel_pdu;
> +
> +        done = true;
> +        QLIST_FOREACH(cancel_pdu, &s->active_list, next) {
> +            if (cancel_pdu != pdu) {
> +                done = false;
> +                cancel_pdu->cancelled = 1;
> +                qemu_co_queue_wait(&cancel_pdu->complete, NULL);
> +                cancel_pdu->cancelled = 0;
> +                pdu_free(cancel_pdu);
> +                break;
> +            }
> +        }
> +    }
> 
>      /* Free all fids */
>      while (s->fid_list) {
> +        assert(!fidp->ref);
> +
>          /* Get fid */
>          fidp = s->fid_list;
>          fidp->ref++;
> @@ -670,7 +690,7 @@ static void coroutine_fn pdu_complete(V9fsPDU *pdu,
> ssize_t len)
> 
>      pdu_push_and_notify(pdu);
> 
> -    /* Now wakeup anybody waiting in flush for this request */
> +    /* Now wakeup anybody waiting in flush or reset for this request */
>      if (!qemu_co_queue_next(&pdu->complete)) {
>          pdu_free(pdu);
>      }