bmr-cymru / dmioscope

Device-mapper IO visualisation tools
GNU General Public License v2.0
5 stars 1 forks source link

dmioscope: make command execution more robust #3

Closed bmr-cymru closed 7 years ago

bmr-cymru commented 7 years ago

Currently dmioscope uses a cut-down version of the sos sos_get_command_output() function: the original uses a dictionary to pass back multiple items of status to the caller (status code, stdout and stderr streams, etc.).

In adapting this to dmioscope only the (merged) stderr and stdout streams are returned: this means that it is not possible to tell whether a command that produces no output returned success or failure (since the if not output test will be True in any case that output=="").

This leads to mysterious backtraces in the event that dmstats fails unexpectedly, for example, due to incompatible DSO versions:

# dmstats list
dmstats: /lib64/libdevmapper.so.1.02: version `DM_1_02_131' not found (required by dmstats)
dmstats: /lib64/libdevmapper.so.1.02: version `DM_1_02_129' not found (required by dmstats)
_get_cmd_output('dmstats report --noheadings -oregion_id,read_count,write_count vg_hex/root')

Updating 8 histogram bins
Traceback (most recent call last):
  File "dmioscope.py", line 804, in <module>
    status = main(sys.argv[1:])
  File "dmioscope.py", line 787, in main
    ioh.update(out, test=_test)
  File "dmioscope.py", line 338, in update
    for line in data.splitlines():
AttributeError: 'NoneType' object has no attribute 'splitlines'
bmr-cymru commented 7 years ago

The robustness is addressed in three commits from Friday:

commit 0a0df954ae388afd90a48647a0164f44877e463a
Author: Bryn M. Reeves <bmr@redhat.com>
Date:   Fri Sep 23 17:58:34 2016 +0100

    dmioscope: raise DmstatsException on failed operations

    Raise a DmstatsException on failed create, delete, list, and report
    operations.

commit 069f043b2c14585db9948eee7128d840fa58e6c8
Author: Bryn M. Reeves <bmr@redhat.com>
Date:   Fri Sep 23 17:59:31 2016 +0100

    dmioscope: catch DmstatsException in __main__ try block

    Catch DmstatsExceptions at the top level in the script and set the
    exit status appropriately and move region cleanup into a finally
    block.

commit 463b90727046ce658dac86ff441bf266cbfb54c5
Author: Bryn M. Reeves <bmr@redhat.com>
Date:   Fri Sep 23 17:52:58 2016 +0100

    dmioscope: add DmstatsException class

    Add a generic dmstats exception class: this represents any type
    of problem executing a dmstats command (command not found or not
    executable, bad parameters etc.).

    Functions that throw DmstatsException should log a descriptive
    error message with log_error(), as well as command output with
    log_verbose() to allow the detailed cause of the error to be
    inspected.

Remaining work to clean up and improve the dmstats command execution abstraction is tracked in Issue #7.