This is an automatically generated e-mail. To reply, visit: http://reviewboard-openlmi.rhcloud.com/r/254/

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
56
                raise pywbem.CIMError(pywbem.CIM_ERR_FAILED, "Empty paramater.")
It would be nice to say also *which* parameter is empty.

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
227
                  affected_elements=[param_filesystemspec, param_mountpoint],
the affected elements are CIMInstanceNames of elements, which are being used/modified during the transaction, i.e. param_filesystem (if specified).

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
242
        outparams = [pywbem.CIMParameter(
If you return 'Mount' here, it means that the instance is already available and application can call GetInstance on it. If I understand it correctly, the MountedFileSystem will be available only after the job finishes. So don't return it here, but add it to outparams when the job finishes.

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
272
                # XXX it will be nice if blivet also returned a nice error
Fill a RFE or create a patch... Adding XXX to code won't help anything :)

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
275
                err = pywbem.CIMError(pywbem.CIM_ERR_FAILED,
either return non-positive 'rval' or set error, not both. IMHO it's enough to 'raise CIMError(...), the job will handle the rest automatically.

Dtto for DeleteMount.

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
282
            # raise pywbem.CIMError(pywbem.CIM_ERR_NOT_SUPPORTED,
Feel free to raise the error, otherwise the application might get an impression that everything is OK.

Dtto for DeleteMount.

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
288
        outparams = {}
Put the created mount point here as outparam.

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
365
                  affected_elements=[param_mount['FileSystemSpec'],
Again, CIMInstanceName of the affected MountedFileSystem should be in affected_elements

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
412
        # XXX Specifying an error instance does not work. It results in strange
413
        # crashes in cmpi_pywbem_bindings
IMHO it works now

src/openlmi/storage/LMI_MountConfigurationService.py (Diff revision 1)
533
        rval = self.MountMethod.Job_Completed_with_No_Error
Throw error instead.

And again, fill RFE.

src/openlmi/storage/MountingProvider.py (Diff revision 1)
26
class MountingProvider(BaseProvider):
27
    """
28
    An auxiliary class that provides some code sharing and a common base for all
29
    mounting classes.
30
    """
OK, why not, but let other mounting classes use it too.

- Jan


On April 24th, 2013, 2:15 p.m. CEST, Jan Synacek wrote:

Review request for OpenLMI Developers.
By Jan Synacek.

Updated April 24, 2013, 2:15 p.m.

Repository: openlmi-storage

Description

Add LMI_MountConfigurationService.

Diffs

  • mof/LMI_Storage.reg (e0eb237150618f0e561294d1d5354857ddf23adf)
  • src/openlmi/storage/LMI_MountConfigurationService.py (PRE-CREATION)
  • src/openlmi/storage/MountingProvider.py (PRE-CREATION)
  • src/openlmi/storage/cimom_entry.py (7157f7d21dff4751905e9f36d115e57d2f397a9d)

View Diff