| Message ID | 20240201183024.145424-2-jandryuk@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libxl: blktap/tapback support | expand |
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@gmail.com> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Thanks,
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@gmail.com> > > Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> I've committed this, but I take the absence of a Fixes: tag as indication that this doesn't want/need backporting. Jan
On Wed, Feb 7, 2024 at 7:50 AM Jan Beulich <jbeulich@suse.com> 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@gmail.com> > > > > Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> > > 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
On 08.02.2024 03:25, Jason Andryuk wrote: > On Wed, Feb 7, 2024 at 7:50 AM Jan Beulich <jbeulich@suse.com> 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@gmail.com> >>> >>> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> >> >> 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. Jan
On Thu, Feb 8, 2024 at 2:50 AM Jan Beulich <jbeulich@suse.com> wrote: > > On 08.02.2024 03:25, Jason Andryuk wrote: > > On Wed, Feb 7, 2024 at 7:50 AM Jan Beulich <jbeulich@suse.com> 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@gmail.com> > >>> > >>> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> > >> > >> 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@mail.gmail.com/ Regards, Jason
diff --git a/tools/hotplug/Linux/block-common.sh b/tools/hotplug/Linux/block-common.sh index f86a88c4eb..5c80237d99 100644 --- a/tools/hotplug/Linux/block-common.sh +++ b/tools/hotplug/Linux/block-common.sh @@ -112,14 +112,12 @@ same_vm() "$FRONTEND_UUID") local target=$(xenstore_read_default "/local/domain/$FRONTEND_ID/target" \ "-1") - local targetvm=$(xenstore_read_default "/local/domain/$target/vm" "-1") + local targetvm=$(xenstore_read_default "/local/domain/$target/vm" "No Target") local otarget=$(xenstore_read_default "/local/domain/$otherdom/target" \ "-1") local otvm=$(xenstore_read_default "/local/domain/$otarget/vm" \ - "-1") - otvm=${otvm%-1} - othervm=${othervm%-1} - targetvm=${targetvm%-1} + "No Other Target") + local frontend_uuid=${FRONTEND_UUID%-1} [ "$frontend_uuid" = "$othervm" -o "$targetvm" = "$othervm" -o \
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@gmail.com> --- tools/hotplug/Linux/block-common.sh | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)