Make persistent discovery controller configurable for the subsystem#3536
Make persistent discovery controller configurable for the subsystem#3536hreinecke wants to merge 1 commit into
Conversation
|
This is a somewhat competing implementation to #3472; I'm not trying to change the default, but rather control the creation of a PDC on a per-subsystem basis. |
b8ac076 to
390bd9f
Compare
| disconnect = true; | ||
| else | ||
| disconnect = false; | ||
| disconnect = libnvme_subsystem_get_pdc_enabled(s); |
There was a problem hiding this comment.
yeah, this makes a lot of sense to me.
There was a problem hiding this comment.
everything in this for loop only runs if _nvmf_discovery gets called with connect = true so running nvme connect. The initial goal was to not only support default pdc for nvme connect, but also nvme discover. The goal post then moved a bit to support default EPCSD for nvme discover as nvme connect supports it already. If the goal of this PR is to support default pdc for nvme connect and not nvme discover then this is fine either way its a step in the right direction.
Sorry for the late reply i somehow missed this until now
255f151 to
2a3dd3d
Compare
Some subsystems / vendors might choose to enable or disable persistent discovery controllers for particular subsystems, independent on the global option to enable PDCs. This patch adds a new subsystem configuration option 'persistent_discovery_controller', which controls whether a PDC should be created for this subsystem. The value defaults to the host PDC setting if not specified in the configuration file. Signed-off-by: Hannes Reinecke <hare@suse.de>
|
As far I understand the implementation it makes sense to handle the PDC on subsys level. @Mr-Bossman, @johnmeneghini WDYT? |
| disconnect = true; | ||
| else | ||
| disconnect = false; | ||
| disconnect = libnvme_subsystem_get_pdc_enabled(s); |
There was a problem hiding this comment.
everything in this for loop only runs if _nvmf_discovery gets called with connect = true so running nvme connect. The initial goal was to not only support default pdc for nvme connect, but also nvme discover. The goal post then moved a bit to support default EPCSD for nvme discover as nvme connect supports it already. If the goal of this PR is to support default pdc for nvme connect and not nvme discover then this is fine either way its a step in the right direction.
Sorry for the late reply i somehow missed this until now
Oh I've missed this important fact. Sorry about that. It's kind of obvious now you mentioned it. I blame my distraction due the 3.x work Ah, this case it I think Hannes patch should just be updated. The if (!connect)
return 0;check should be after the PDC check. My idea is to avoid to iterate over the entries twice. The logic spread across the function. I suppose it would help to refactor this into smaller functions with a clear scope what they do. We have |
3 participants
Some subsystems / vendors might choose to enable or disable persistent discovery controllers for particular subsystems, independent on the global option to enable PDCs.
This patch adds a new subsystem configuration option 'persistent_discovery_controller', which controls whether a PDC should be created for this subsystem. The value defaults to the host PDC setting if not specified in the configuration file.