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

[Jason Andryuk <jandryuk@xxxxxxxxx>]

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.

Regards,
Jason