Hi Chaitanya & Christoph,

I changed the patch I posted this morning and added the changes suggested.

I also wrote a patch for nvmetcli and verified that nvmetcli can set and save the model description:

dev:/usr/src/nvmetcli # ./nvmetcli save /tmp/test.json
dev:/usr/src/nvmetcli # cat /sys/kernel/config/nvmet/subsystems/loop/attr_model 
mark
dev:/usr/src/nvmetcli # grep model /tmp/test.json
        "model": "mark", 

dev:/usr/src/nvmetcli # ./nvmetcli 
/subsystems/loop> ls  
o- loop ............................................................ [version=1.3, allow_any=1, serial=198f07c757eb114d, model=mark]
  o- allowed_hosts ........................................................................................................... [...]
  o- namespaces .............................................................................................................. [...]
    o- 1 ............................................ [path=/dev/loop0, uuid=d808c1db-3507-40cf-8ade-aa4b1a3b290b, grpid=1, enabled]
/subsystems/loop> set attr model=coolmodel
Parameter model is now 'coolmodel'.
/subsystems/loop> ls
o- loop ....................................................... [version=1.3, allow_any=1, serial=198f07c757eb114d, model=coolmodel]
  o- allowed_hosts ........................................................................................................... [...]
  o- namespaces .............................................................................................................. [...]
    o- 1 ............................................ [path=/dev/loop0, uuid=d808c1db-3507-40cf-8ade-aa4b1a3b290b, grpid=1, enabled]
/subsystems/loop> exit
dev:/usr/src/nvmetcli # ./nvmetcli save /tmp/testcool.jsonl 
dev:/usr/src/nvmetcli # grep model /tmp/testcool.json
        "model": "coolmodel", 
dev:/usr/src/nvmetcli #

I also verified that writing illegal characters in the model fails and that the initiator sees the model description that was set:
 echo -e "\t failit" > attr_model 
-bash: echo: write error: Invalid argument

echo this~works > attr_model

# nvme list
Node             SN                   Model                                    Namespace Usage                      Format           FW Rev  
---------------- -------------------- ---------------------------------------- --------- -------------------------- ---------------- --------
/dev/nvme0n1     4d11eb57c7078f19     this~works                               1         104,86  MB / 104,86  MB    512   B +  0 B   5.3.5   

Thanks,

Mark Ruijter
 

Op 11-11-19 11:35 heeft Linux-nvme namens Christoph Hellwig <linux-nvme-bounces@lists.infradead.org namens hch@infradead.org> geschreven:

    > +	const char *model = ctrl->subsys->model;
    
    Can we have a little nvme_controller_mode() helper that uses
    subsystem->model if it is set, and otherwise the default?  That saves
    memory by not duplicating the default name for every subsystem.
    
    > +	kfree(subsys->model);
    > +	subsys->model = kstrndup(page, len, GFP_KERNEL);
    > +	if (!subsys->model)
    > +		ret = -ENOMEM;
    
    I don't think we should free the old model until the new one
    is allocated.
    
    Otherwise this looks good to me, but can someone also prepare a
    nvmetcli patch as well?
    
    _______________________________________________
    Linux-nvme mailing list
    Linux-nvme@lists.infradead.org
    http://lists.infradead.org/mailman/listinfo/linux-nvme