Devices/Device.py: Fix for broken cleanup() with missing tc tool

[SirPhuttel]

If no tc utility is present on the agent system, exec_cmd() raises an exception. If uncaught, the previous address flush request will also not complete. The address remaining in place will cause an EEXIST error upon next test run, requiring manual intervention to resolve the situation.

Fixes: 49e523d ("Slave: Add basic support for tc.")

[SirPhuttel]

@jtluka my bet was right, this fixes the problem. Could you please review?

[jtluka]

The fix looks ok in general, however I'd like to enhance it a bit so that we don't silently discard potential issues when iproute-tc is installed but fails when executed.

In particular I'd like to have error message check for "command not found" similar to what we have in

lnst/lnst/Devices/Device.py

Line 926 in bd86ed0

except ExecCmdFail as e:

Maybe using function decorator similar to sriov_capable, would be best. Decorator definition is here

Anyway, I'm ok with the patch as is, @olichtne what's your opinion?

[olichtne]

Anyway, I'm ok with the patch as is, @olichtne what's your opinion?

i'm it should be ok to simply do

except ExecCmdFail:
    logging.exception("tc command error during cleanup")

this will log both the traceback and some basic message instead of silently passing the exception.

[jtluka]

Anyway, I'm ok with the patch as is, @olichtne what's your opinion?

i'm it should be ok to simply do

except ExecCmdFail:
    logging.exception("tc command error during cleanup")

this will log both the traceback and some basic message instead of silently passing the exception.

The exception is already logged in the exec_cmd code, so this is not necessary.

[olichtne]

ah, fair point, in that case this should be ok as is.

wrt. sriov_capable i'm not sure what that would add in this case?

[jtluka]

wrt. sriov_capable i'm not sure what that would add in this case?

Avoiding duplication of the code. If we were to add error message parsing, we'd have to include that in both _clear_tc_qdisc and _clear_tc_filters. Using wrapper that would do the check of iproute-tc availability would keep the code at single place.

But it's not worth it. Let's merge this.