Re: [PATCH for-4.19] hotplug: Restore block-tap phy compatibility (again)

[oleksii.kurochko@xxxxxxxxx]

On Tue, 2024-07-23 at 14:05 -0400, Jason Andryuk wrote:
> On 2024-07-23 13:34, Andrew Cooper wrote:
> > On 23/07/2024 6:31 pm, oleksii.kurochko@xxxxxxxxx wrote:
> > > On Tue, 2024-07-23 at 11:04 -0400, Jason Andryuk wrote:
> > > > On 2024-07-23 11:04, Anthony PERARD wrote:
> > > > > On Mon, Jul 15, 2024 at 07:46:31PM -0400, Jason Andryuk
> > > > > wrote:
> > > > > > "$dev" needs to be set correctly for backendtype=phy as
> > > > > > well as
> > > > > > backendtype=tap.  Move the setting into the conditional, so
> > > > > > it
> > > > > > can be
> > > > > > handled properly for each.
> > > > > > 
> > > > > > (dev could be captured during tap-ctl allocate for blktap
> > > > > > module,
> > > > > > but it
> > > > > > would not be set properly for the find_device case.  The
> > > > > > backendtype=tap
> > > > > > case would need to be handled regardless.)
> > > > > > 
> > > > > > Fixes: 6fcdc84927 ("hotplug: Restore block-tap phy
> > > > > > compatibility")
> > > > > Do you mean f16ac12bd418 ("hotplug: Restore block-tap phy
> > > > > compatibility") ?
> > > > Yes!  Thanks for checking that - I must have grabbed the hash
> > > > from a
> > > > local branch.
> > > > 
> > > > > > Fixes: 76a484193d ("hotplug: Update block-tap")
> > > > > > 
> > > > > > Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> > > > > With the fixes tag fix:
> > > > > Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > > > Thanks again.
> > > > 
> > > > Oleksii, this is a fix (for an incomplete fix) for 4.19. 
> > > > 76a484193d
> > > > broke compatibility for block-tap with the blktap2 kernel model
> > > > (when
> > > > adding support for tapback).  This finishes restoring blktap2
> > > > support.
> > > > 
> > > > I realize it's late in the release if you don't want to take
> > > > it.
> > > It's pretty late but I just wanted to clarify:
> > > 1. Is so critical that we should have this in 4.19?
> > > 2. If we won't take it now, then will it be backported anyway?
> > 
> > 2) Yes it will get backported.  In fact I'm about to commit it to
> > staging.
> > 
> > 1) It's a bug in a new feature for 4.19, so if we don't take this
> > bugfix
> > then we'll have to strip it from the release notes.
> 
> It's a bug in the old feature.  The new feature - tapback daemon 
> support, backendtype=tap - works with what's in the 4.19 tree.  It's
> the 
> old kernel module support - backendtype=phy,script=block-tap - that
> was 
> broken when adding tapback support.  This patch fixes the old
> support.
> 
> The change is localized in the block-tap script and requires explicit
> configuration (script=block-tap) to use.  So it seems to me to be a 
> lower risk to take it even though it is late in the cycle.
Agree, if it is by default is disabled then I think we can have this
patch in 4.19:
 Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

~ Oleksii