On Wed, Nov 13, 2024 at 5:36 PM Numan Siddique <[email protected]> wrote:

> On Fri, Oct 18, 2024 at 4:18 PM Mark Michelson <[email protected]>
> wrote:
> >
> > Acked-by: Mark Michelson <[email protected]>
> >
> > On 10/18/24 06:09, Ales Musil wrote:
> > > The NB Chassis_Template_Var has exactly one corresponding row in SB
> database.
> > > Use the NB UUID for the SB representation which makes the processing
> easier
> > > and the link between the rows is obvious at first glance.
> > >
> > > Signed-off-by: Ales Musil <[email protected]>
> > > ---
> > >   northd/northd.c     | 30 +++++++++++++++---------------
> > >   tests/ovn-northd.at |  2 ++
> > >   2 files changed, 17 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/northd/northd.c b/northd/northd.c
> > > index fb34d0231..0dac661b0 100644
> > > --- a/northd/northd.c
> > > +++ b/northd/northd.c
> > > @@ -18283,37 +18283,37 @@ sync_template_vars(
> > >       const struct nbrec_chassis_template_var_table
> *nbrec_ch_template_var_table,
> > >       const struct sbrec_chassis_template_var_table
> *sbrec_ch_template_var_table)
> > >   {
> > > -    struct shash nb_tvs = SHASH_INITIALIZER(&nb_tvs);
> > > -
> > >       const struct nbrec_chassis_template_var *nb_tv;
> > >       const struct sbrec_chassis_template_var *sb_tv;
> > >
> > > -    NBREC_CHASSIS_TEMPLATE_VAR_TABLE_FOR_EACH (
> > > -            nb_tv, nbrec_ch_template_var_table) {
> > > -        shash_add(&nb_tvs, nb_tv->chassis, nb_tv);
> > > -    }
> > > -
> > >       SBREC_CHASSIS_TEMPLATE_VAR_TABLE_FOR_EACH_SAFE (
> > >               sb_tv, sbrec_ch_template_var_table) {
> > > -        nb_tv = shash_find_and_delete(&nb_tvs, sb_tv->chassis);
> > > +        nb_tv = nbrec_chassis_template_var_table_get_for_uuid(
> > > +            nbrec_ch_template_var_table, &sb_tv->header_.uuid);
> > >           if (!nb_tv) {
> > >               sbrec_chassis_template_var_delete(sb_tv);
> > >               continue;
> > >           }
> > > +
> There is a bug here.   If existing SB chassis_template_var row is out
> of sync for the "chassis" column,
> this patch doesn't fix it.
>
> You can test it out by
> ----
> ovn-nbctl create chassis_template_var chassis=ch-1
>
> ovn-sbctl set chassis_template_var . chassis=ch-2
>
> ovn-sbctl list chassis_template_var
> ----
>
> I think you should call : sbrec_chassis_template_var_set_chassis() here.
>
>
> Thanks
> Numan
>


Hi Numan,

that is a good point. It is fixed in v2.

Thank you,
Ales


> > >           if (!smap_equal(&sb_tv->variables, &nb_tv->variables)) {
> > > -            sbrec_chassis_template_var_set_variables(sb_tv,
> > > -
>  &nb_tv->variables);
> > > +            sbrec_chassis_template_var_set_variables(sb_tv,
> &nb_tv->variables);
> > >           }
> > >       }
> > >
> > > -    struct shash_node *node;
> > > -    SHASH_FOR_EACH (node, &nb_tvs) {
> > > -        nb_tv = node->data;
> > > -        sb_tv = sbrec_chassis_template_var_insert(ovnsb_txn);
> > > +    NBREC_CHASSIS_TEMPLATE_VAR_TABLE_FOR_EACH (
> > > +            nb_tv, nbrec_ch_template_var_table) {
> > > +        const struct uuid *nb_uuid = &nb_tv->header_.uuid;
> > > +        sb_tv = sbrec_chassis_template_var_table_get_for_uuid(
> > > +            sbrec_ch_template_var_table, nb_uuid);
> > > +        if (sb_tv) {
> > > +            continue;
> > > +        }
> > > +
> > > +        sb_tv =
> sbrec_chassis_template_var_insert_persist_uuid(ovnsb_txn,
> > > +
>  nb_uuid);
> > >           sbrec_chassis_template_var_set_chassis(sb_tv,
> nb_tv->chassis);
> > >           sbrec_chassis_template_var_set_variables(sb_tv,
> &nb_tv->variables);
> > >       }
> > > -    shash_destroy(&nb_tvs);
> > >   }
> > >
> > >   static void
> > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > > index bcee2d231..ab65ddb44 100644
> > > --- a/tests/ovn-northd.at
> > > +++ b/tests/ovn-northd.at
> > > @@ -10138,7 +10138,9 @@ check ovn-nbctl set Chassis_Template_Var hv2
> variables:tv=v2
> > >   AS_BOX([Ensure values are propagated to SB])
> > >   check ovn-nbctl --wait=sb sync
> > >   check_column "tv=v1" sb:Chassis_Template_Var variables chassis="hv1"
> > > +check_column "$(fetch_column nb:Chassis_Template_Var _uuid
> chassis=\"hv1\")" sb:Chassis_Template_Var _uuid chassis="hv1"
> > >   check_column "tv=v2" sb:Chassis_Template_Var variables chassis="hv2"
> > > +check_column "$(fetch_column nb:Chassis_Template_Var _uuid
> chassis=\"hv2\")" sb:Chassis_Template_Var _uuid chassis="hv2"
> > >
> > >   AS_BOX([Ensure SB is reconciled])
> > >   check ovn-sbctl --all destroy Chassis_Template_Var
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to