[REVIEW] Storage mounting API
Jan Safranek
jsafrane at redhat.com
Thu Mar 14 09:10:48 UTC 2013
On 03/13/2013 04:29 PM, Jan Synacek wrote:
> class LMI_Mount : CIM_Mount
> {
> [ Description (
> "Type of filesystem to create."),
> ValueMap { "0", "2", "3", "4", "5", "6", "7", "8", "9", "10",
> "11", "12", "13", "..",
> "32769", "32770", "32771", "32772", "32773", "32768" },
> Values { "Unknown", "UFS", "HFS", "FAT", "FAT16", "FAT32",
> "NTFS4", "NTFS5", "XFS", "AFS", "EXT2", "EXT3",
> "REISERFS", "DMTF Reserved",
> "EXT4", "BTRFS", "JFS", "TMPFS", "VFAT", "OTHER"} ]
> uint16 FileSystemType;
>
> [ Description (
> "Specifies filesystem type other than listed in FileSystemType.") ]
Probably "Specifies filesystem type when FileSystemType property is set
to Other." And please use Other instead of OTHER, it's not an abbreviation.
> class LMI_MountSetting : CIM_SettingData
> {
> [ Description (
> "Other mount options that can be filesystem specific. This property is "
> "also used to specify options with values (e.g. uid=0 or gid=100). "
> "Other options are appended to the basic options."),
> string OtherOptions[];
I would explicitly note that the mount options are used in the same
order as in this array. Also add ArrayType("Indexed") qualifier to
emphasize this.
> class LMI_MountService : CIM_Service
> {
> uint32 CreateMount(
> [ IN, Description (
> "Existing filesystem that should be mounted.") ]
> CIM_FileSystem REF FileSystem,
>
> [ IN, Description (
> "Type of filesystem."),
> ValueMap{ "0", "2", "3", "4", "5", "6", "7", "8", "9", "10",
> "11", "12", "13", "..",
> "32769", "32770", "32771", "32772", "32773", "32768" },
> Values { "Unknown", "UFS", "HFS", "FAT", "FAT16", "FAT32",
> "NTFS4", "NTFS5", "XFS", "AFS", "EXT2", "EXT3",
> "REISERFS", "DMTF Reserved",
> "EXT4", "BTRFS", "JFS", "TMPFS", "VFAT", "OTHER"} ]
> uint16 FileSystemType,
We already know the FS type from FileSystem parameter, FileSystemType
seems to be redundant.
>
> [ IN, Description (
> "Directory where the mounted filesystem should be attached at.") ]
> string MountPoint,
>
> [ IN, Description (
> "Desired mount settings. If NULL, defaults will be used. Default "
> "mount options are 'rw, suid, dev, exec, auto, nouser, async'.") ]
> LMI_MountSettings REF Goal,
>
> [ IN(false), OUT, Description (
> "Reference to the created job.") ]
> CIM_ConcreteJob REF Job
There should be output parameter Mount with the created mount.
> );
>
> [ Description (
> "Modifies (remounts) an existing mount." ),
> ValueMap { "0", "1", "2", "3", "4", "5", "6", "..", "4096",
> "4097", "4098..32767", "32768..65535" },
> Values { "Job Completed with No Error", "Not Supported",
> "Unknown", "Timeout", "Failed", "Invalid Parameter",
> "In Use", "DMTF Reserved",
> "Method Parameters Checked - Job Started",
> "Size Not Supported", "Method Reserved", "Vendor Specific" } ]
> uint32 ModifyMount(
> [ IN, Description (
> "An existing mount.") ]
> LMI_Mount REF Mount,
Shouldn't it be also output parameter? That's how we do it in CIM.
>
> [ IN, Description (
> "Desired mount settings. If NULL, defaults will be used. Default "
> "mount options are 'rw, suid, dev, exec, auto, nouser, async'.") ]
> LMI_MountSettings REF Goal,
>
> [ IN(false), OUT, Description (
> "Reference to the created job.") ]
> CIM_ConcreteJob REF Job
> );
Otherwise it looks well, good job!
Jan
More information about the openlmi-devel
mailing list