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.
