Re: [PATCH 1/4] block-common: Fix same_vm for no targets

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Thu, Feb 8, 2024 at 2:50 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 08.02.2024 03:25, Jason Andryuk wrote:
> > On Wed, Feb 7, 2024 at 7:50 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>
> >> On 06.02.2024 12:45, Anthony PERARD wrote:
> >>> On Thu, Feb 01, 2024 at 01:30:21PM -0500, Jason Andryuk wrote:
> >>>> same_vm is broken when the two main domains do not have targets.  otvm
> >>>> and targetvm are both missing, which means they get set to -1 and then
> >>>> converted to empty strings:
> >>>>
> >>>> ++10697+ local targetvm=-1
> >>>> ++10697+ local otvm=-1
> >>>> ++10697+ otvm=
> >>>> ++10697+ othervm=/vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4
> >>>> ++10697+ targetvm=
> >>>> ++10697+ local frontend_uuid=/vm/844dea4e-44f8-4e3e-8145-325132a31ca5
> >>>>
> >>>> The final comparison returns true since the two empty strings match:
> >>>>
> >>>> ++10697+ '[' /vm/844dea4e-44f8-4e3e-8145-325132a31ca5 = 
> >>>> /vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4 -o '' = 
> >>>> /vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4 -o 
> >>>> /vm/844dea4e-44f8-4e3e-8145-325132a31ca5 = '' -o '' = '' ']'
> >>>>
> >>>> Replace -1 with distinct strings indicating the lack of a value and
> >>>> remove the collescing to empty stings.  The strings themselves will no
> >>>> longer match, and that is correct.
> >>>>
> >>>> ++12364+ '[' /vm/844dea4e-44f8-4e3e-8145-325132a31ca5 = 
> >>>> /vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4 -o 'No target' = 
> >>>> /vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4 -o 
> >>>> /vm/844dea4e-44f8-4e3e-8145-325132a31ca5 = 'No other target' -o 'No 
> >>>> target' = 'No other target' ']'
> >>>>
> >>>> Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> >>>
> >>> Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> >>
> >> I've committed this, but I take the absence of a Fixes: tag as indication
> >> that this doesn't want/need backporting.
> >
> > Hmmm, maybe this should have a Fixes.  Sorry I didn't investigate that
> > better before submission.
> >
> > Looks like this would be the commit:
> > https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=f3a7ca02400d1c416e97451b4aebfaf608fc8192
> >
> > f3a7ca02400d1 ("hotplug/Linux: fix same_vm check in block script")
> >
> > I need to circle back on this.  IIRC, when I set up a conflicting
> > assignment of a writable disk to two VMs with block-tap, it was
> > allowed and not denied.  That is what prompted this change.
> >
> > I'll have to double check there isn't something in the regular block
> > that might prevent that.
>
> Okay, I'll wait for a result here before deciding whether to queue.

Yes, it should be backported.  This patch prevents sharing writable
block devs.  Files are still broken because of issues identified
previously here:
https://lore.kernel.org/xen-devel/CAKf6xpv-U91nF2Fik7GRN3SFeOWWcdR5R+ZcK5fgojE+-D43sg@xxxxxxxxxxxxxx/

Regards,
Jason