Abstract getting unique status result into a single method #430

mulkieran · 2019-02-26T21:03:06Z

Related: https://github.com/stratis-storage/stratisd/issues/794.
Related: #221.

The goals of this PR are:

Remove some assertions, and replace them with errors, since the assertions are not logically necessary.
Make reading the target type when parsing the ioctl a bit safer in order to
Extend the implementation of FromStr for the various status types by having it parse the target type. This bring FromStr for status types in line with FromStr for param types.
Get rid of some unnecessary code duplication.

The changes in behavior are:

Where previously an error in parsing might result in an assertion failure, now the error would result in an error return with a more helpful message.
If it was ever possible for a target type to have a trailing space at the end, now it is not. This makes checking equality of target types a less chancy proposition.
If a status line for an unexpected target type is parsed, a failure will occur when the target type is compared with the expected one, rather than later, when the actual fields of the status are parsed with incorrect expectations.

EDIT: Comments:

tasleson · 2019-02-27T19:50:38Z

I just built a local copy of stratisd using this branch. I created a pool with 3 devices, started IO on the FS created from it and then took the block devices offline. The stratsd process printed:

ERROR libstratis::engine::strat_engine::thinpool::thinpool: Thinpool status is fail -> Failed

Running the command line results in exception:

# stratis pool
Traceback (most recent call last):
  File "/home/tasleson/projects/stratis/dbus-python-client-gen/src/dbus_python_client_gen/_invokers.py", line 319, in dbus_func
    return dbus_method(*xformed_args, timeout=timeout)
  File "/usr/lib64/python3.6/site-packages/dbus/proxies.py", line 145, in __call__
    **keywords)
  File "/usr/lib64/python3.6/site-packages/dbus/connection.py", line 651, in call_blocking
    message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.Failed: no total physical size computed for pool with uuid 3b8e5c85-a0d8-4b04-ab5e-826689e7d020

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tasleson/projects/stratis/stratis-cli/src/stratis_cli/_main.py", line 48, in the_func
    result.func(result)
  File "/home/tasleson/projects/stratis/stratis-cli/src/stratis_cli/_actions/_top.py", line 68, in list_pools
    managed_objects = ObjectManager.Methods.GetManagedObjects(proxy, {})
  File "/home/tasleson/projects/stratis/dbus-python-client-gen/src/dbus_python_client_gen/_invokers.py", line 327, in dbus_func
    name, xformed_args)) from err
dbus_python_client_gen._errors.DPClientInvocationError: Error while invoking method "GetManagedObjects" belonging to interface "org.freedesktop.DBus.ObjectManager" with arguments ()

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tasleson/projects/stratis/stratis-cli/bin/stratis", line 33, in <module>
    main()
  File "/home/tasleson/projects/stratis/stratis-cli/bin/stratis", line 29, in main
    return run()(sys.argv[1:])
  File "/home/tasleson/projects/stratis/stratis-cli/src/stratis_cli/_main.py", line 52, in the_func
    raise StratisCliActionError(command_line_args, result) from err
stratis_cli._errors.StratisCliActionError: Action selected by command-line arguments ['--propagate', 'pool'] which were parsed to Namespace(func=<function TopActions.list_pools at 0x7fb4e364e6a8>, propagate=True) failed

So we aren't panicing the stratisd process, but the CLI is not usable.

mulkieran · 2019-02-27T20:02:51Z

~~I think the problem is flaky; hopefully it will recur. Maybe it depends on calling the command line at just the correct time.~~ Whoops. Look at the error for the first exception. dbus.exceptions.DBusException: org.freedesktop.DBus.Error.Failed: no total physical size computed for pool with uuid 3b8e5c85-a0d8-4b04-ab5e-826689e7d020. stratisd is now taking the error from devicemapper, throwing it away, and stuffing in a generic error. We need to edit stratisd to send back the real info, somehow.

mulkieran · 2019-02-27T20:17:49Z

@tasleson: Please add stratis-storage/stratisd#1449 to your stratisd install and see what the D-Bus tells us then.

tasleson · 2019-02-27T20:48:17Z

@mulkieran As requested

# stratis pool list
Traceback (most recent call last):
  File "/home/tasleson/projects/stratis/dbus-python-client-gen/src/dbus_python_client_gen/_invokers.py", line 319, in dbus_func
    return dbus_method(*xformed_args, timeout=timeout)
  File "/usr/lib64/python3.6/site-packages/dbus/proxies.py", line 145, in __call__
    **keywords)
  File "/usr/lib64/python3.6/site-packages/dbus/connection.py", line 651, in call_blocking
    message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.Failed: no total physical size computed for pool with uuid b9e2b3f8-c085-44d4-9493-8636c2801947, cause: DM error: DM error: Invalid: Insufficient number of fields for status; requires at least 8, found only 1 in status line "Error"

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tasleson/projects/stratis/stratis-cli/src/stratis_cli/_main.py", line 48, in the_func
    result.func(result)
  File "/home/tasleson/projects/stratis/stratis-cli/src/stratis_cli/_actions/_top.py", line 68, in list_pools
    managed_objects = ObjectManager.Methods.GetManagedObjects(proxy, {})
  File "/home/tasleson/projects/stratis/dbus-python-client-gen/src/dbus_python_client_gen/_invokers.py", line 327, in dbus_func
    name, xformed_args)) from err
dbus_python_client_gen._errors.DPClientInvocationError: Error while invoking method "GetManagedObjects" belonging to interface "org.freedesktop.DBus.ObjectManager" with arguments ()

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tasleson/projects/stratis/stratis-cli/bin/stratis", line 33, in <module>
    main()
  File "/home/tasleson/projects/stratis/stratis-cli/bin/stratis", line 29, in main
    return run()(sys.argv[1:])
  File "/home/tasleson/projects/stratis/stratis-cli/src/stratis_cli/_main.py", line 52, in the_func
    raise StratisCliActionError(command_line_args, result) from err
stratis_cli._errors.StratisCliActionError: Action selected by command-line arguments ['--propagate', 'pool', 'list'] which were parsed to Namespace(func=<function TopActions.list_pools at 0x7f2c82f7b6a8>, propagate=True) failed

mulkieran · 2019-02-27T20:56:25Z

Bingo! I think this needs a separate issue since the problem that has been located has nothing to do with this PR. See #431.

mulkieran · 2019-03-01T15:32:59Z

rebased.

mulkieran · 2019-03-01T21:51:18Z

rebased.

mulkieran · 2019-03-01T21:58:45Z

I think there's potential for a test, so pulling out of review...

This brings it into line with params, and is a bit better encapsulated. Signed-off-by: mulhern <amulhern@redhat.com>

mulkieran · 2019-05-28T12:59:57Z

rebased.

mulkieran self-assigned this Feb 26, 2019

mulkieran force-pushed the master-status branch from 8cbf1df to 9d204d9 Compare February 26, 2019 21:03

mulkieran added the do not merge label Feb 26, 2019

mulkieran force-pushed the master-status branch 3 times, most recently from 5417386 to 64d66a1 Compare February 27, 2019 18:01

mulkieran removed the do not merge label Feb 27, 2019

mulkieran requested review from tasleson and trgill February 27, 2019 18:10

tasleson mentioned this pull request Feb 27, 2019

Need to understand recovery when a device goes away and comes back stratis-storage/stratisd#1448

Open

mulkieran mentioned this pull request Feb 27, 2019

Devicemapper status ioctl sometimes returns a single word status "Error" #431

Closed

mulkieran force-pushed the master-status branch from 64d66a1 to e75e5da Compare March 1, 2019 15:32

mulkieran force-pushed the master-status branch from e75e5da to ebe7915 Compare March 1, 2019 21:51

mulkieran closed this Mar 1, 2019

mulkieran reopened this Mar 1, 2019

mulkieran force-pushed the master-status branch 2 times, most recently from 05e2ae3 to feea886 Compare March 1, 2019 22:06

mulkieran mentioned this pull request Mar 5, 2019

Trim the end of the target type string #435

Merged

mulkieran force-pushed the master-status branch from feea886 to ab182d8 Compare March 5, 2019 16:20

mulkieran force-pushed the master-status branch from ab182d8 to bb06967 Compare May 21, 2019 21:08

Combine target type value with status to be tested

19e5e46

This brings it into line with params, and is a bit better encapsulated. Signed-off-by: mulhern <amulhern@redhat.com>

mulkieran force-pushed the master-status branch from bb06967 to 19e5e46 Compare May 28, 2019 12:59

mulkieran marked this pull request as draft March 10, 2021 13:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Abstract getting unique status result into a single method #430

Abstract getting unique status result into a single method #430

mulkieran commented Feb 26, 2019 •

edited

Loading

tasleson commented Feb 27, 2019

mulkieran commented Feb 27, 2019 •

edited

Loading

mulkieran commented Feb 27, 2019

tasleson commented Feb 27, 2019

mulkieran commented Feb 27, 2019 •

edited

Loading

mulkieran commented Mar 1, 2019

mulkieran commented Mar 1, 2019

mulkieran commented Mar 1, 2019

mulkieran commented May 28, 2019

Abstract getting unique status result into a single method #430

Are you sure you want to change the base?

Abstract getting unique status result into a single method #430

Conversation

mulkieran commented Feb 26, 2019 • edited Loading

tasleson commented Feb 27, 2019

mulkieran commented Feb 27, 2019 • edited Loading

mulkieran commented Feb 27, 2019

tasleson commented Feb 27, 2019

mulkieran commented Feb 27, 2019 • edited Loading

mulkieran commented Mar 1, 2019

mulkieran commented Mar 1, 2019

mulkieran commented Mar 1, 2019

mulkieran commented May 28, 2019

mulkieran commented Feb 26, 2019 •

edited

Loading

mulkieran commented Feb 27, 2019 •

edited

Loading

mulkieran commented Feb 27, 2019 •

edited

Loading