[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 3/4] hotplug: Update block-tap
On Thu, Feb 01, 2024 at 01:30:23PM -0500, Jason Andryuk wrote: > Implement a sharing check like the regular block script. > > Checking tapback inside block-tap is too late since it needs to be > running to transition the backend to InitWait before block-tap is run. > > tap-ctl check will be removed when the requirement for the blktap kernel > driver is removed. Remove it now as it is of limited use. > > find_device() needs to be non-fatal allow a sharing check. > > Only write physical-device-path because that is all that tapback needs. > Also write_dev doesn't handled files and would incorrectly store > physical-device as 0:0 which would confuse the minor inside tapback Missing SOB. Is `block-tap` still going to work with the example given in the script header? That is: "script=block-tap,vdev=xvda,target=<type>:<file>" Or maybe, that example is already broken? > --- > diff --git a/tools/hotplug/Linux/block-tap b/tools/hotplug/Linux/block-tap > index 89247921b9..5eca09f0f6 100755 > --- a/tools/hotplug/Linux/block-tap > +++ b/tools/hotplug/Linux/block-tap > +count_using() > +{ > + local file="$1" > + local f > + > + local i=0 > + local base_path="$XENBUS_BASE_PATH/$XENBUS_TYPE" > + for dom in $(xenstore-list "$base_path") > + do > + for dev in $(xenstore-list "$base_path/$dom") This function is probably missing "local dom dev". > + do > + f=$(xenstore_read_default "$base_path/$dom/$dev/params" "") > + f=$(echo "$f" | cut -d ":" -f 2) > + > + if [ -n "$f" ] && [ "$file" = $f ] ; then > + i=$(( i + 1 )) > + fi > + done > + done > + > + echo "$i" > +} > + > +check_tap_sharing() > +{ > + local file="$1" > + local mode="$2" > + local dev > + > + local base_path="$XENBUS_BASE_PATH/$XENBUS_TYPE" > + for dom in $(xenstore-list "$base_path") ; do Should we add "local dom" to the function? > + for dev in $(xenstore-list "$base_path/$dom") ; do > + f=$(xenstore_read_default "$base_path/$dom/$dev/params" "") Same here, maybe "local f" would be good to have too. > @@ -89,15 +183,57 @@ find_device() > # the device > add() > { > - dev=$(tap-ctl create -a $target) > - write_dev $dev > + local minor > + local pid > + local res > + > + claim_lock "block" > + > + if find_device; then > + result=$( check_tap_sharing "$file" "$mode" ) > + if [ "$result" != "ok" ] ; then > + do_ebusy "tap $type file $file in use " "$mode" "${result%% *}" > + fi > + else > + tap_create The new function tap_create() is doing something similar to the replace `tap-ctl create` call, right? > # Disconnects the device > remove() > { > - find_device > - do_or_die tap-ctl destroy -p ${pid} -m ${minor} > /dev/null > + local minor > + local pid > + > + claim_lock "block" > + > + if tap_shared ; then > + return > + fi > + > + minor=$( xenstore_read "$XENBUS_PATH/minor" ) > + pid=$( xenstore_read "$XENBUS_PATH/pid" ) > + > + [ -n "$minor" ] || fatal "minor missing" > + [ -n "$pid" ] || fatal "pid missing" > + do_or_die tap-ctl close -p "$pid" -m "$minor" > /dev/null > + do_or_die tap-ctl detach -p "$pid" -m "$minor" > /dev/null Should we also call `tap-ctl free`, like `tap-ctl destroy` seems to do? > + > + release_lock "block" > } > > command=$1 Thanks, -- Anthony PERARD
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |