hostnqn is not used from config.json file

[igaw]

nvme connect-all will only use the hostnqn from /etc/nvme/hostnqn.

With given configuration nvme connect-all will not connect to anything:

# cat /etc/nvme/hostnqn
nqn.2014-08.org.nvmexpress:uuid:31333937-3136-584d-5135-323430365637

# cat config.json 
[
  {
    "hostnqn":"nqn.2014-08.org.nvmexpress:uuid:1a9e23dd-466e-45ca-9f43-a29aaf47cb21",
    "hostid":"1a9e23dd-466e-45ca-9f43-a29aaf47cb21",
   ...
  }
]

Only if we provide the hostnqn and hostid it will connect:

# nvme connect-all --hostnqn nqn.2014-08.org.nvmexpress:uuid:1a9e23dd-466e-45ca-9f43-a29aaf47cb21 --hostid 1a9e23dd-466e-45ca-9f43-a29aaf47cb21

I am not sure if this is the desired behavior. What do people expect with such a configuration?

[igaw]

@martin-gpy @hreinecke @johnmeneghini

[johnmeneghini]

I am not sure if this is the desired behavior. What do people expect with such a configuration?

nvme-cli has always taken the host-nqn and hostid from /etc/nvme/hostnqn and /etc/nvme/hostid and historically users were not required to add either one to the command line. My preferred way to handle this problem would be:

1. take host-nqn and hostid from /etc/nvme files
2. override host-nqn or hostid with values from discovery.conf
3. override host-nqn or hostid with values from json.config
4. override host-nqn or hostid with values from the command line

I think this would provide a fully backwards compatible method of supporting the previous precedence.

[igaw]

Make sense to me.

BTW, should we insist on that the user providing both hostnqn and hostid on the command line?

The current code expects that the user provides both. And with the recent discussion/decision upstream on this topic, newer kernel will only allow a 1:1 mapping. So should we be smart here as well and warn if they do not match and if only hostnqn is provided we figure out hostid?

[martin-gpy]

I think this would provide a fully backwards compatible method of supporting the previous precedence.

Yes, this approach looks good.

[martin-gpy]

BTW, should we insist on that the user providing both hostnqn and hostid on the command line?

I think hostnqn alone is sufficient. Honestly it would be an overkill if we insist on both hostnqn and hostid here.

The current code expects that the user provides both. And with the recent discussion/decision upstream on this topic, newer kernel will only allow a 1:1 mapping. So should we be smart here as well and warn if they do not match and if only hostnqn is provided we figure out hostid?

I think so yes. We should able to warn if they don't match. And figure out hostid when only hostnqn is provided.

[hreinecke]

hostid is only used for reservations, and really should not be of a concern here. IE we should only be using hostnqn as an identifier for the host itself, and allow the hostid to change.

[johnmeneghini]

On 5/26/23 03:23, Daniel Wagner wrote: BTW, should we insist on that the user providing both |hostnqn| and |hostid|?

According to the latest NVMe specification, the hostid can be 0. We changed this with TP-4110, which was ratified on 01/24/2022. So I'd like to support TP-4110 with nvme-cli by NOT enforcing any policy for hostid. I suggest that, if the user provides a hostid in any of historical sources, that nvme-cli use the user provided value for the hostid. However, if no hostid is proivded then nvme-cli should use the value "0" for the hostid. This should provide a backwards compatible solution. /John

[igaw]

Thanks for the feedback. Highly appreciated!

[igaw]

There is also a related problem. The connect-all command will only connect all controllers from one (first) host defined in the JSON config file.

[igaw]

Merged #2394. I've added a few unit tests to make sure I don't break anything. Anyway, please give it a run and see if there are any regressions. Thanks.