On 19 Mar 2024, at 11:24, Jakob Meng wrote:
> On 15.03.24 11:16, Eelco Chaudron wrote:
>> Hi Jakob,
>>
>> See some comments below.
>>
>> //Eelco
>
> Thanks and comments below again 😄
>
> Cheers,
> Jakob
>
>>> ...
>>> --- a/tests/appctl.py
>>> +++ b/tests/appctl.py
>>> @@ -49,13 +49,30 @@ def main():
>>> help="Arguments to the command.")
>>> parser.add_argument("-T", "--timeout", metavar="SECS",
>>> help="wait at most SECS seconds for a response")
>>> + parser.add_argument("-f", "--format", metavar="FMT",
>>> + help="Output format.", default="text",
>>> + choices=[fmt.name.lower()
>>> + for fmt in ovs.util.OutputFormat])
>>> args = parser.parse_args()
>>>
>>> signal_alarm(int(args.timeout) if args.timeout else None)
>>>
>>> ovs.vlog.Vlog.init()
>>> target = args.target
>>> + format = ovs.util.OutputFormat[args.format.upper()]
>>> client = connect_to_target(target)
>>> +
>>> + if format != ovs.util.OutputFormat.TEXT:
>>> + err_no, error, _ = client.transact(
>>> + "set-options", ["--format", args.format])
>>> +
>>> + if err_no:
>>> + ovs.util.ovs_fatal(err_no, "%s: transaction error" % target)
>>> + elif error is not None:
>> I see this is a copy from below, but is the 'elif' ok here? Meaning can we
>> not
>> get both errors at the same time? The C code catches both.
>>
>> Also 'if err_no' we will still try the second transaction for the command.
>> I think we should exit even in this case.
>
> When we hit a transaction error ('if err_no'), the ovs.util.ovs_fatal() call
> will sys.exit(1), so the elif branch will not be executed nor the second
> command transaction?!
>
> What did I miss?🙈
You missed nothing :) It was me missign the fatal part :(
Acked-by: Eelco Chaudron <[email protected]>
>>> + sys.stderr.write(error)
>>> + ovs.util.ovs_error(0, "%s: server returned an error" % target)
>>> + sys.exit(2)
>>> +
>>> err_no, error, result = client.transact(args.command, args.argv)
>>> client.close()
>>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
