[PATCH blktests v3 1/3] nvme/rc: introduce remote target support

[Daniel Wagner]

On Thu, Jun 27, 2024 at 11:59:11AM GMT, Hannes Reinecke wrote:
 > +	if [[ -n "${nvme_target_control}" ]]; then
> > +		eval "${nvme_target_control}" cleanup \
> > +			--subsysnqn "${subsysnqn}" \
> > +			> /dev/null
> > +		return
> > +	fi
> > +
> >   	_get_nvmet_ports "${subsysnqn}" ports
> >   	for port in "${ports[@]}"; do
> 
> Hmm. This wasn't quite what I had in mind; I think it'd be simpler
> if we could just check if the requested controller is visible to the
> host already (ie checking sysfs here), and then skip all the setup
> steps at they are obviously not required anymore.
> That would save quite a lot of issues, and we wouldn't need to specify
> a target setup script (or whatever).

What I like at the current approach is that it allows to support the
use case where the external target can be configured. For example this
works with a remote VM running Linux pretty well. With your idea only
static setups are supported.

I don't know what you mean with 'quite a lot of issues'. I've running
this against a VM which gets dynamically configured and it works well.

> Quite a bit of churn, I agree, as that would mean we have to roll
>
> 	_setup_nvmet
>
> 	_nvmet_target_setup
>
> 	_nvme_connect_subsys
>
> all into one function. But it might be easier in the long run.
> Hmm?

Not all tests have this pattern, e.g. nvme/031. This test works with
the approach in the version of the series though.

Sure, I'll don't see a problem to refactor a bit more and reduce the
boiler plate even more but I see this a different task.