| Message ID | 149095963571.26233.9370507789591510204.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
> -----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); > }
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); }
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(-)