> -----Original Message----- > From: Jürgen Groß <[email protected]> > Sent: 09 December 2019 13:39 > To: Durrant, Paul <[email protected]>; Roger Pau Monné > <[email protected]> > Cc: [email protected]; [email protected]; Stefano > Stabellini <[email protected]>; Boris Ostrovsky > <[email protected]> > Subject: Re: [Xen-devel] [PATCH 2/4] xenbus: limit when state is forced to > closed > > On 09.12.19 13:19, Durrant, Paul wrote: > >> -----Original Message----- > >> From: Jürgen Groß <[email protected]> > >> Sent: 09 December 2019 12:09 > >> To: Durrant, Paul <[email protected]>; Roger Pau Monné > >> <[email protected]> > >> Cc: [email protected]; [email protected]; > Stefano > >> Stabellini <[email protected]>; Boris Ostrovsky > >> <[email protected]> > >> Subject: Re: [Xen-devel] [PATCH 2/4] xenbus: limit when state is forced > to > >> closed > >> > >> On 09.12.19 13:03, Durrant, Paul wrote: > >>>> -----Original Message----- > >>>> From: Jürgen Groß <[email protected]> > >>>> Sent: 09 December 2019 11:55 > >>>> To: Roger Pau Monné <[email protected]>; Durrant, Paul > >>>> <[email protected]> > >>>> Cc: [email protected]; [email protected]; > >> Stefano > >>>> Stabellini <[email protected]>; Boris Ostrovsky > >>>> <[email protected]> > >>>> Subject: Re: [Xen-devel] [PATCH 2/4] xenbus: limit when state is > forced > >> to > >>>> closed > >>>> > >>>> On 09.12.19 12:39, Roger Pau Monné wrote: > >>>>> On Thu, Dec 05, 2019 at 02:01:21PM +0000, Paul Durrant wrote: > >>>>>> Only force state to closed in the case when the toolstack may need > to > >>>>>> clean up. This can be detected by checking whether the state in > >>>> xenstore > >>>>>> has been set to closing prior to device removal. > >>>>> > >>>>> I'm not sure I see the point of this, I would expect that a failure > to > >>>>> probe or the removal of the device would leave the xenbus state as > >>>>> closed, which is consistent with the actual driver state. > >>>>> > >>>>> Can you explain what's the benefit of leaving a device without a > >>>>> driver in such unknown state? > >>>> > >>>> And more concerning: did you check that no frontend/backend is > >>>> relying on the closed state to be visible without closing having been > >>>> set before? > >>> > >>> Blkfront doesn't seem to mind and I believe the Windows PV drivers > cope, > >> but I don't really understand the comment since this patch is actually > >> removing a case where the backend transitions directly to closed. > >> > >> I'm not speaking of blkfront/blkback only, but of net, tpm, scsi, > pvcall > >> etc. frontends/backends. After all you are modifying a function common > >> to all PV driver pairs. > >> > >> You are removing a state switc to "closed" in case the state was _not_ > >> "closing" before. > > > > Yes, which AFAIK is against the intention of the generic PV protocol > such that it ever existed anyway. > > While this might be the case we should _not_ break any guests > running now. So this kind of reasoning is dangerous. > > > > >> So any PV driver reacting to "closed" of the other end > >> in case the previous state might not have been "closing" before is at > >> risk to misbehave with your patch. > > > > Well, they will see nothing now. If the state was not closing, it gets > left alone, so the frontend shouldn't do anything. The only risk that I > can see is that some frontend/backend pair needed a direct 4 -> 6 > transition to support 'unbind' before but AFAIK nothing has ever supported > that, and blk and net crash'n'burn if you try that on upstream as it > stands. A clean unplug would always set state to 5 first, since that's > part of the unplug protocol. > > That was my question: are you sure all current and previous > guest frontends and backends are handling unplug this way? > > Not "should handle", but "do handle".
That depends on the toolstack. IIUC the only 'supported' toolstack is xl/libxl, which will set 'state' to 5 and 'online' to 0 to initiate an unplug. Paul > > > Juergen _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
