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

On July 12th, 2013, 11:33 a.m. UTC, Jan Safranek wrote:

cli-tools/lmi/lmi_client_shell.py (Diff revision 2)
751
        unique_str = ''.join(random.choice(string.ascii_uppercase + string.digits) for x in range(6))
There is risk of clashing with other listeners. Will add_handler() throw an error in this case?
Very low chance to clash. Currently the handler callback will be overwritten. Ok, maybe I could add some checks.

On July 12th, 2013, 11:33 a.m. UTC, Jan Safranek wrote:

cli-tools/lmi/lmi_client_shell.py (Diff revision 2)
753
        destination_port = 10240
This port should be probably configurable.
(maybe in next releases)
OK.

On July 12th, 2013, 11:33 a.m. UTC, Jan Safranek wrote:

cli-tools/lmi/lmi_client_shell.py (Diff revision 2)
764
        filter_name = "LMI:%s:Changed" % job_inst.classname
Expected name of the indication filter must be documented somewhere.
BTW, there is no lmishell documentation so far.
The indication filter name comes from (eg.) your provider. But I can add some comment, or something, how the filters' names should look like.

On July 12th, 2013, 11:33 a.m. UTC, Jan Safranek wrote:

cli-tools/lmi/lmi_client_shell.py (Diff revision 2)
840
                cond.wait(15)
There is a lot of magic numbers in this file, '15' is just an example. Please use constants with meaningful names.
On purspose, I was expecting an opinion, if it is a suitable constant. Will be switched to a constant, of course.

On July 12th, 2013, 11:33 a.m. UTC, Jan Safranek wrote:

cli-tools/lmi/lmi_client_shell.py (Diff revision 2)
852
                    if _LmiInstanceMethod.is_job_finished(job_inst):
853
                        job_out_params.value = job_inst.JobOutParameters
854
                        listener.stop()
855
                        break
If I understand it correctly, this checks every minute that the job instance is still in progress by calling GetInstance().

The GetInstance() is useful only to check that the job had not finished before we subscribed for job indications. Therefore the lmishell should check much earlier than one minute, probably in 0.1-0.5 seconds, and check only once.
There is a check before the "waiting loop". If a job has already finished before entering the loop, the loop body will not be executed.

On July 12th, 2013, 11:33 a.m. UTC, Jan Safranek wrote:

cli-tools/lmi/lmi_client_shell.py (Diff revision 2)
def __call__(self, *args, **kwargs):
905
            filter_classnames = (
906
                "LMI_StorageJob",
907
                "LMI_SoftwareInstallationJob",
908
                "LMI_NetworkJob"
909
            )
This list should be configurable in later versions. Or you can check for CIM_ConcreteJob subclasses (if it is simple).
I would stick with a configuration.

On July 12th, 2013, 11:33 a.m. UTC, Jan Safranek wrote:

cli-tools/lmi/lmi_client_shell.py (Diff revision 2)
def __call__(self, *args, **kwargs):
929
                            sleep_time = 1
930
                            while not _LmiInstanceMethod.is_job_finished(job_inst):
931
                                # Sleep, a bit longer in every iteration
932
                                time.sleep(sleep_time)
933
                                if sleep_time < 128:
934
                                    sleep_time *= 2
935
                                (refreshed, _, errorstr) = job_inst.refresh()
936
                                if not refreshed:
937
                                    if LmiBaseClient._get_use_exceptions():
938
                                        raise LmiSynchroMethodCallError(errorstr)
939
                                    return LmiReturnValue(
940
                                        rval = -1,
941
                                        errorstr = errorstr,
942
                                        hostname = self._client.hostname,
943
                                        use_display_hook = True)
944
                            call_rparams = self.__transform_return_values({"out" : job_inst.JobOutParameters})
Cut it to separate method? 
OK.

- Peter


On July 12th, 2013, 11:01 a.m. UTC, Peter Hatina wrote:

Review request for OpenLMI Developers.
By Peter Hatina.

Updated July 12, 2013, 11:01 a.m.

Repository: openlmi-tools

Description

By this patch, we introduce a feature, which enables LMIShell to perform synchronous method calls. In other words, when there is a method call executed, which returns a CIM_ConcretJob object (derived from), LMIShell is able to either subscribe an indication and then wait for the job to finish and return job output parameters. If the shell is not able to use indications (can not start tcp server), then it uses "blind" polling as the waiting method. Indication method of waiting also checks the JobState value of the object to prevent the shell from infinite waiting.

To invoke a "synchro" method, see following (example with filesystem):
> ... some stuff to get necessary objects ...
> conf_service.SyncLMI_CreateFileSystem(FileSystemType = 11, InExtents = [sda1])
               ^^^^

The prefix "Sync" is also added to interactive interface (tab-completion).

Question 1) 
As the patch of a version 1, there is only unsecure http used for indication delivery. The question is: what kind of certificate to use, do we generate some for the shell at install time, or use some existing? Please, tell me your opinion.

Question 2)
Can we setup Pegasus to permit other users to create objects in the namespace PG_InterOp? In the example below, there is root account used, other users for me with default Pegasus setup can not create indication handler and subscription objects. Now, the shell will fail to get necessary static filter instance (used for the subscription) and will fail to call "synchro" method.

Testing

Tested with storage provider.

> c = connect("hostname", "root", "root")
> sda = c.root.cimv2.LMI_StorageExtent.first_instance({"ElementName" : "sda"})
> basedon = sda.first_reference(ResultClass = "LMI_PartitionBasedOn")
> sda1 = basedon.Dependent.to_instance()
> conf_service = c.root.cimv2.LMI_FileSystemConfigurationService.first_instance()
> (ret, out, err) = conf_service.SyncLMI_CreateFileSystem(FileSystemType = 11, InExtents = [sda1])
> print "%s\n%s\n%s" % (ret, out, err)
4096
NocaseDict({'out': _LmiInstance(classname='CIM_ManagedElement', ...)})

Note: see return value and output parameters (ret == 4096, out['out'] == _LmiInstance(...)) <- this will be changed once we have Pegasus fixed. Now synchro method does not modify return value of the method, therefore you can find 4096 in the result. Once we have proper values in output parameters (Job.JobOutParameters), return code and return parameters will be modified respectively.

Diffs

  • cli-tools/lmi/__init__.py (e69de29bb2d1d6434b8b29ae775ad8c2e48c5391)
  • cli-tools/lmi/lmi_client_shell.py (9a6788cc17582f7c4419df909cd4322d9e6b23f9)
  • cli-tools/lmi/lmi_listener.py (6670b204a84485b6e1f8bef5dfa2893098f0a9da)
  • cli-tools/lmishell (a12233eedc159a6f6c570ca31dfff20ac6d17161)

View Diff