[PATCH v2 2/4] nvmet: make sn stable once connection was established

[Chaitanya Kulkarni]

Max,

On 6/7/21 02:23, Max Gurtovoy wrote:
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index 0ae809ca428c..bd0a0b91d843 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -231,6 +231,7 @@ struct nvmet_subsys {
>  
>  	u64			ver;
>  	char			serial[NVMET_SN_MAX_SIZE];
> +	bool			subsys_discovered;

When subsystem is discovered (connected to the host) we will have non empty
controller list, instead of adding a new bool variable why not use
something like following tested patch on the top of this one ?
(that also fixes the indentation of
nvmet_subsys_attr_serial_store_locked()).

diff --git a/drivers/nvme/target/admin-cmd.c
b/drivers/nvme/target/admin-cmd.c
index bec84c231d37..897f419be618 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -366,12 +366,6 @@ static void nvmet_execute_identify_ctrl(struct
nvmet_req *req)
        u32 cmd_capsule_size;
        u16 status = 0;
 
-       if (!subsys->subsys_discovered) {
-               mutex_lock(&subsys->lock);
-               subsys->subsys_discovered = true;
-               mutex_unlock(&subsys->lock);
-       }
-
        /*
         * If there is no model number yet, set it now.  It will then remain
         * stable for the life time of the subsystem.
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index a13da86fb374..94e23dca763f 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -1046,11 +1046,11 @@ static ssize_t
nvmet_subsys_attr_serial_show(struct config_item *item,
 
 static ssize_t
 nvmet_subsys_attr_serial_store_locked(struct nvmet_subsys *subsys,
-               const char *page, size_t count)
+                                     const char *page, size_t count)
 {
        int pos, len = strcspn(page, "\n");
 
-       if (subsys->subsys_discovered) {
+       if (!list_empty(&subsys->ctrls)) {
                pr_err("Can't set serial number. %s is already assigned\n",
                       subsys->serial);
                return -EINVAL;
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index f7fbf9690ce8..73c6c4dc8997 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -232,7 +232,6 @@ struct nvmet_subsys {
 
        u64                     ver;
        char                    serial[NVMET_SN_MAX_SIZE];
-       bool                    subsys_discovered;
        char                    *subsysnqn;
        bool                    pi_support;
 

# nvme list
Node                  SN                  
Model                                    Namespace
Usage                      Format           FW Rev 
--------------------- --------------------
---------------------------------------- ---------
-------------------------- ---------------- --------
/dev/nvme0n1          foo                  QEMU NVMe
Ctrl                           1           1.07  GB /   1.07  GB   
512   B +  0 B   1.0    
/dev/nvme1n1          bf854da611a264d448c2
Linux                                    1           2.15  GB /   2.15 
GB      4 KiB +  0 B   5.13.0-r
# echo 1 > /sys/kernel/config/nvmet/subsystems/nqn/
allowed_hosts/       attr_cntlid_min      attr_serial          passthru/
attr_allow_any_host  attr_model           attr_version        
attr_cntlid_max      attr_pi_enable       namespaces/         
 # echo "1234567890" > /sys/kernel/config/nvmet/subsystems/nqn/attr_serial
bash: echo: write error: Invalid argument
# dmesg  -c
[311081.155148] nvmet: Can't set serial number. bf854da611a264d448c2 is
already assigned