Re: [PATCH 2/3] [SCSI] scst: Move SCST device documentation to Documentation/ABI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(resending as plain text)

On Thu, Dec 23, 2010 at 1:20 AM, Greg KH <gregkh@xxxxxxx> wrote:
>
> On Wed, Dec 22, 2010 at 09:46:28PM +0100, Bart Van Assche wrote:
> > Move the documentation about the sysfs attributes of the SCST
> > virtual device to Documentation/ABI/stable.
> >
> > Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxx>
> > Cc: Vladislav Bolkhovitin <vst@xxxxxxxx>
> > ---
> >  Documentation/ABI/stable/sysfs-devices-scst |  150 +++++++++++++++++++++++++++
> >  Documentation/scst/README.scst              |   71 -------------
> >  2 files changed, 150 insertions(+), 71 deletions(-)
> >  create mode 100644 Documentation/ABI/stable/sysfs-devices-scst
> >
> > diff --git a/Documentation/ABI/stable/sysfs-devices-scst b/Documentation/ABI/stable/sysfs-devices-scst
> > new file mode 100644
> > index 0000000..80bc258
> > --- /dev/null
> > +++ b/Documentation/ABI/stable/sysfs-devices-scst
> > @@ -0,0 +1,150 @@
> > +What:           /sys/devices/scst/mgmt
>
> How did you get a file into this directory?  Are you sure that's
> acceptable?  Same for the other files you added here.

Hello Greg,

All that was needed to create the entry /sys/devices/scst/mgmt was to
call device_register() for the device with the name "scst" and
device_create_file() for the attribute called "mgmt". We are not aware
of any documentation that disallows creating files there. So whether
or not creating a file there is acceptable is something we can't know
if the kernel maintainers do not speak up.

> > +Date:           December 2010
> > +KernelVersion:  2.6.38
>
> Really?  That's pretty presumptious, isn't it?
>
> > +Contact:        linux-scsi@xxxxxxxxxxxxxxx
>
> Why not the "owner" of this code, not the scsi mailing list.
>
> > +Description:
> > +             Interface through which SCST management commands can be issued.
>
> You mix tabs and spaces above and in all of your other files.  Please
> make them only tabs.

Will address the above comments.

> > +             The documentation of the syntax of these commands can be
> > +             obtained by reading this file:
> > +
> > +             # cat mgmt
> > +             in device/<dev> <dev_cmd>
> > +             in device_driver/<devt> <devt_cmd>
> > +             in target_driver/<tgtt> <tgtt_cmd>
> > +             in target_driver/<tgtt>/<target>/luns <tgt_cmd>
> > +             in target_driver/<tgtt>/<target>/luns <luns_cmd>
> > +             in target_driver/<tgtt>/<target>/ini_groups <acg_mgmt_cmd>
> > +             in target_driver/<tgtt>/<target>/ini_groups/<acg> <acg_cmd>
> > +             in target_driver/<tgtt>/<target>/ini_groups/<acg>/luns <luns_cmd>
> > +             in target_driver/<tgtt>/<target>/ini_groups/<acg>/initiators <acg_ini_cmd>
>
> What?  sysfs is one value per file, why would the mgmt file return more
> than one line?  That's not acceptable, sorry.

As far as I understood the sysfs philosophy, the intention of the one
value per file rule is to avoid that software reading data from sysfs
files has to parse what it reads. The above data is documentation only
and is not intended to be parsed by software. So that file could be
left empty, but that would not be very helpful for users who configure
SCST by issuing shell commands instead of using the scstadmin tool.
I'm not sure though how to proceed here.

> > +
> > +             dev_cmd syntax:
> > +
> > +             set_filename <filename>
> > +             set_threads_num <n>
> > +             set_thread_pool_type <thread_pool_type>
> > +
> > +             devt_cmd syntax:
> > +
> > +             add_device device_name [parameters]
> > +             del_device device_name
> > +             add_attribute <attribute> <value>
> > +             del_attribute <attribute> <value>
> > +             add_device_attribute device_name <attribute> <value>
> > +             del_device_attribute device_name <attribute> <value>
> > +
> > +             devt_cmd syntax for pass-through device types:
> > +
> > +             add_device H:C:I:L
> > +             del_device H:C:I:L
> > +
> > +             tgtt_cmd syntax:
> > +
> > +             add_target target_name [parameters]
> > +             del_target target_name
> > +             add_attribute <attribute> <value>
> > +             del_attribute <attribute> <value>
> > +             add_target_attribute target_name <attribute> <value>"
> > +             del_target_attribute target_name <attribute> <value>"
> > +
> > +             where parameters is one or more <name>=<value> pairs separated by ';'
> > +
> > +             tgt_cmd syntax:
> > +
> > +             enable
> > +             disable
> > +             set_cpu_mask <mask>
> > +
> > +             luns_cmd syntax:
> > +
> > +             add|del H:C:I:L lun [parameters]
> > +             add VNAME lun [parameters]
> > +             del lun
> > +             replace H:C:I:L lun [parameters]
> > +             replace VNAME lun [parameters]
> > +             clear
> > +
> > +             where parameters is either 'read_only' or empty.
> > +
> > +             acg_mgmt_cmd syntax:
> > +
> > +             create <group_name>
> > +             del <group_name>
> > +
> > +             acg_cmd syntax:
> > +             set_cpu_mask <mask>
> > +
> > +             acg_ini_cmd syntax:
> > +
> > +             add <initiator_name>
> > +             del <initiator_name>
> > +             move <initiator_name> <dest_group_name>
> > +             clear
>
> That's quite complex, are you sure it's correct, and actually describes
> it in a manner that anyone else can use?

More detailed documentation of these commands is present in
Documentation/scst/README.scst. Should that documentation be moved to
Documentation/ABI/stable/sysfs-devices-scst too ?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux