On 1/20/22 15:16, Lorenzo Bianconi wrote:
>> On 1/20/22 11:38, Lorenzo Bianconi wrote:
>>> Introduce the capability to specify CoPP UUID or CoPP name in order to
>>> reuse the same CoPP reference on multiple datapaths.
>>> Introduce logical_router and logical_switches columns in CoPP table in
>>> order to specify datapaths where CoPP is installed.
>>>
>>> Reported-ad: https://bugzilla.redhat.com/show_bug.cgi?id=2040852
>>> Signed-off-by: Lorenzo Bianconi <[email protected]>
>>> ---
>>
>> Hi Lorenzo,
> 
> Hi Dumitru,
> 
> thx for the review.
> 
>>
>>>  ovn-nb.ovsschema          |  15 +++++-
>>>  ovn-nb.xml                |   9 ++++
>>>  tests/ovn-northd.at       |  27 ++++++++++
>>>  utilities/ovn-nbctl.8.xml |  16 ++++--
>>>  utilities/ovn-nbctl.c     | 103 ++++++++++++++++++++++++++++++++------
>>>  5 files changed, 150 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
>>> index 55977339a..cf2947d93 100644
>>> --- a/ovn-nb.ovsschema
>>> +++ b/ovn-nb.ovsschema
>>> @@ -1,7 +1,7 @@
>>>  {
>>>      "name": "OVN_Northbound",
>>> -    "version": "5.34.1",
>>> -    "cksum": "2177334725 30782",
>>> +    "version": "5.35.0",
>>> +    "cksum": "2039436985 31434",
>>>      "tables": {
>>>          "NB_Global": {
>>>              "columns": {
>>> @@ -32,6 +32,17 @@
>>>              "isRoot": true},
>>>          "Copp": {
>>>              "columns": {
>>> +                "name": {"type": "string"},
>>
>> I think name should be an index too.  That would be a backwards
>> incompatible change but, AFAIK, there are no users of CoPP yet.
>>
>> What do you think?
> 
> if name is used as index we will need to make it mandatory. One possible 
> solution
> would be to create a random string when not provided. Any downside with this
> approach?

This still doesn't cover the case when entries already existed in the
database before the upgrade.

I don't think there's a backwards compatible way of doing this and I
don't really think it's useful to add a "name" field that's not used as
index.

However, for this feature in particular, because it's rather new and not
used (as far as I know) by any CMS, I think we can probably break
compatibility and add the missing index.

I'm curious about the maintainers' opinion on this too.

Regards,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to