| Message ID | 20220518160952.9998-1-dceara@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] ovn-controller: Consider zone 0 as a valid zone when restoring. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 5/18/22 18:09, Dumitru Ceara wrote: > 0 is a valid zone ID and some CMSs might actually use it. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2087194 > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > controller/ovn-controller.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 0efe5c5ce..c2949fa30 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -825,13 +825,18 @@ restore_ct_zones(const struct ovsrec_bridge_table *bridge_table, > } > > const char *user = node->key + 8; > - int zone = atoi(node->value); > + if (!user[0]) { > + continue; > + } > > - if (user[0] && zone) { > - VLOG_DBG("restoring ct zone %"PRId32" for '%s'", zone, user); > - bitmap_set1(ct_zone_bitmap, zone); > - simap_put(ct_zones, user, zone); > + int zone; > + if (!str_to_int(node->value, 10, &zone)) { Maybe uint? Negative values doesn't seem to be valid. > + continue; > } > + > + VLOG_DBG("restoring ct zone %"PRId32" for '%s'", zone, user); > + bitmap_set1(ct_zone_bitmap, zone); > + simap_put(ct_zones, user, zone); > } > } >
On 5/18/22 18:41, Ilya Maximets wrote: > On 5/18/22 18:09, Dumitru Ceara wrote: >> 0 is a valid zone ID and some CMSs might actually use it. >> >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2087194 >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> --- >> controller/ovn-controller.c | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index 0efe5c5ce..c2949fa30 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -825,13 +825,18 @@ restore_ct_zones(const struct ovsrec_bridge_table *bridge_table, >> } >> >> const char *user = node->key + 8; >> - int zone = atoi(node->value); >> + if (!user[0]) { >> + continue; >> + } >> >> - if (user[0] && zone) { >> - VLOG_DBG("restoring ct zone %"PRId32" for '%s'", zone, user); >> - bitmap_set1(ct_zone_bitmap, zone); >> - simap_put(ct_zones, user, zone); >> + int zone; >> + if (!str_to_int(node->value, 10, &zone)) { > > Maybe uint? Negative values doesn't seem to be valid. > Fair point. I posted v2: https://patchwork.ozlabs.org/project/ovn/patch/20220518174728.3366-1-dceara@redhat.com/ Thanks! >> + continue; >> } >> + >> + VLOG_DBG("restoring ct zone %"PRId32" for '%s'", zone, user); >> + bitmap_set1(ct_zone_bitmap, zone); >> + simap_put(ct_zones, user, zone); >> } >> } >> >
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 0efe5c5ce..c2949fa30 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -825,13 +825,18 @@ restore_ct_zones(const struct ovsrec_bridge_table *bridge_table, } const char *user = node->key + 8; - int zone = atoi(node->value); + if (!user[0]) { + continue; + } - if (user[0] && zone) { - VLOG_DBG("restoring ct zone %"PRId32" for '%s'", zone, user); - bitmap_set1(ct_zone_bitmap, zone); - simap_put(ct_zones, user, zone); + int zone; + if (!str_to_int(node->value, 10, &zone)) { + continue; } + + VLOG_DBG("restoring ct zone %"PRId32" for '%s'", zone, user); + bitmap_set1(ct_zone_bitmap, zone); + simap_put(ct_zones, user, zone); } }
0 is a valid zone ID and some CMSs might actually use it. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2087194 Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- controller/ovn-controller.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)