On 3/18/25 5:20 PM, Lorenzo Bianconi via dev wrote: >> Allow extra option in the Open_vSwitch external_ids called >> "ovn-cleanup-on-exit", this flag allows to specify whether >> ovn-controller should cleanup the resources during exit. The >> "--restart" exit flag still has priority over the new option in order >> to keep the backward compatibility. > > Hi Ales, > > it looks fine to me, just a nit inline. > Acked-by: Lorenzo Bianconi <[email protected]> >
Hi Ales, Lorenzo, Thanks for the patch and review! > Regards, > Lorenzo > >> >> Reported-at: https://issues.redhat.com/browse/FDP-959 >> Signed-off-by: Ales Musil <[email protected]> >> --- >> NEWS | 4 +++ >> controller/ovn-controller.8.xml | 7 ++++ >> controller/ovn-controller.c | 21 ++++++++++-- >> tests/ovn-controller.at | 57 +++++++++++++++++++++++++++++++++ >> 4 files changed, 86 insertions(+), 3 deletions(-) >> >> diff --git a/NEWS b/NEWS >> index d1767e5b3..7023bbfe3 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -1,5 +1,9 @@ >> Post v25.03.0 >> ------------- >> + - Added support for "ovn-cleanup-on-exit" config option in Open_vSwitch >> + external-ids, this option allows to specify if ovn-controller should >> + perform cleanup when exiting. The "--restart" exit has always priority Nit: I think it reads better if we say "always has priority". I changed it like that. >> + to keep the backward compatibility. >> >> OVN v25.03.0 - xx xxx xxxx >> -------------------------- >> diff --git a/controller/ovn-controller.8.xml >> b/controller/ovn-controller.8.xml >> index eace2c5cf..31f790875 100644 >> --- a/controller/ovn-controller.8.xml >> +++ b/controller/ovn-controller.8.xml >> @@ -423,6 +423,13 @@ >> See the <code>ovn-nb</code>(5) for more details. >> </p> >> </dd> >> + >> + <dt><code>external_ids:ovn-cleanup-on-exit</code></dt> >> + <dd> >> + The boolean flag indicates if ovn-controller should perform cleanup >> on >> + exit. In order to keep backward compatibility the >> + <code>--restart</code> exit flag has priority over this flag. >> + </dd> >> </dl> >> >> <p> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index 081411cba..56fa0e540 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -884,6 +884,16 @@ get_transport_zones(const struct >> ovsrec_open_vswitch_table *ovs_table) >> "ovn-transport-zones", ""); >> } >> >> +static bool >> +get_ovn_cleanup_on_exit(const struct ovsrec_open_vswitch_table *ovs_table) >> +{ >> + const struct ovsrec_open_vswitch *cfg = >> + ovsrec_open_vswitch_table_first(ovs_table); >> + const char *chassis_id = get_ovs_chassis_id(ovs_table); 'chassis_id' and 'cfg' can be NULL here. If 'cfg' is NULL we'll perform an invalid memory access below. I added: if (!cfg || !chassis_id) { return false; } To be extra sure that both chassis_id and cfg are not NULL. >> + return get_chassis_external_id_value_bool(&cfg->external_ids, >> chassis_id, >> + "ovn-cleanup-on-exit", true); >> +} >> + >> static void >> ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl) >> { >> @@ -6522,8 +6532,15 @@ loop_done: >> engine_set_context(NULL); >> engine_cleanup(); >> >> + > unnecessary new-line here. > I took care of this. [...] And then I applied this patch to main. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
