Re: [Qemu-devel] [PATCH v2] 9pfs: Correctly handle cancelled requests

[Greg Kurz]

On Tue, 12 Dec 2017 15:14:06 -0800 (PST)
Stefano Stabellini <address@hidden> wrote:

> On Fri, 8 Dec 2017, Greg Kurz wrote:
> > Cc'ing Stefano using a more appropriate address :)  
> 
> Thanks Greg for the ping, one comment inline below.
> 
[...]
> 
> I think we also need to:
> 
>     /* this is required to consume the request */
>     ring->intf->out_cons = ring->out_cons;
>     xen_wmb();
> 
>     /* this is required to complete the cancellation of the pdu operation */
>     ring->inprogress = false;
> 
>     /* this is required to let the other end know that there is more
>      * space on the ring available */
>     xenevtchn_notify(ring->evtchndev, ring->local_port);
> 

That makes me think that we should also probably notify the guest with
virtio since this isn't a reset... :)

>     /* trigger the read of the next request */
>     qemu_bh_schedule(ring->bh);
> 
> 
> Which is basically almost everything else from xen_9pfs_push_and_notify,
> except the modification of in_prod, because we are not going to write
> back anything to the ring.
> 

... in which case, the same statement stands for virtio_9pfs_push_and_notify()

We should probably unify push_and_notify and discard into a single operation:

-    void        (*push_and_notify)(V9fsPDU *pdu);
+    void        (*return_to_guest)(V9fsPDU *pdu, bool discard);

Let's do this in a separate patch, and respin this fix on top.

> 
> 
> > > > +}
> > > > +
> > > >  static const struct V9fsTransport xen_9p_transport = {
> > > >      .pdu_vmarshal = xen_9pfs_pdu_vmarshal,
> > > >      .pdu_vunmarshal = xen_9pfs_pdu_vunmarshal,
> > > >      .init_in_iov_from_pdu = xen_9pfs_init_in_iov_from_pdu,
> > > >      .init_out_iov_from_pdu = xen_9pfs_init_out_iov_from_pdu,
> > > >      .push_and_notify = xen_9pfs_push_and_notify,
> > > > +    .discard = xen_9pfs_discard,
> > > >  };
> > > >  
> > > >  static int xen_9pfs_init(struct XenDevice *xendev)    
>