Re: [PATCH 1/3] nvmet: make the subsystem type configurable

[Christoph Hellwig <hch@lst.de>]

On Thu, Apr 07, 2022 at 12:48:06PM +0200, Hannes Reinecke wrote:
> +static ssize_t nvmet_subsys_attr_type_store(struct config_item *item,
> +					    const char *page, size_t count)
> +{
> +	struct nvmet_subsys *subsys = to_subsys(item);
> +	struct nvmet_port *p;
> +	struct nvmet_subsys_link *s;
> +	int i;
> +
> +	if (subsys->subsys_discovered)
> +		return -EACCES;
> +
> +	/*
> +	 * Do not allow to change the subsystem type if it's
> +	 * already linked to ports; the user should unlink it first.
> +	 */
> +	down_read(&nvmet_config_sem);
> +	list_for_each_entry(p, &nvmet_ports_list, global_entry) {
> +		list_for_each_entry(s, &p->subsystems, entry) {
> +			if (s->subsys == subsys) {
> +				up_read(&nvmet_config_sem);
> +				return -EACCES;
> +			}
> +		}
> +	}
> +	up_read(&nvmet_config_sem);

Does this scale?  Do we want a flag in the subsystem instead?

> +	for (i = 0; i < ARRAY_SIZE(nvmet_subsys_type_map); i++) {
> +		if (sysfs_streq(page, nvmet_subsys_type_map[i].name))
> +			goto found;
> +	}
> +
> +	pr_err("Invalid value '%s' for subsystem type\n", page);
> +	return -EINVAL;
> +
> +found:
> +	down_write(&nvmet_config_sem);
> +	if (nvmet_subsys_type_map[i].type == NVME_NQN_CURR) {
> +		if (!xa_empty(&subsys->namespaces)) {
> +			pr_err("discovery subsystem cannot have namespaces\n");
> +			return -EINVAL;
> +		}
> +	}

We can probably just check this unconditionally, as something that was
a discovery subsystem before by definition can't have namespaces.