You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Daan Hoogland <da...@gmail.com> on 2014/02/19 16:03:03 UTC
Re: cidrs in acls
Kishan,
Can you have a look at the branch acl-item-cidrs. I made some code to
handle the cidrs from a separate table. I hardly think this can be
enough and would like to create a checklist on what I need to do next.
(item one is use the new transaction model;)
thanks,
Daan
On Fri, Jan 17, 2014 at 1:19 PM, Daan Hoogland <da...@gmail.com> wrote:
> That was what I thought as well. What was the retionale to join them
> into one field?
>
> On Fri, Jan 17, 2014 at 8:32 AM, Kishan Kavala <Ki...@citrix.com> wrote:
>> Daan,
>> Similar to firewall_rules_cidrs, separate table can be used to store acl cidrs. Maybe in network_acl_item_cidrs.
>>
>> Regards,
>> kishan
>>
>>> -----Original Message-----
>>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>>> Sent: Friday, 17 January 2014 1:05 AM
>>> To: Kishan Kavala
>>> Cc: dev
>>> Subject: cidrs in acls
>>>
>>> H Kishan,
>>>
>>> I see you implemented CLOUDSTACK-763. it merges a lot of cidrs into one field.
>>> The api doesn't check the field length. I enlarged the field in the create table
>>> statement to 2048 for the 4.3 branch. Can you help me think about a more solid
>>> solution, please. It seems to me those cidrs shouldn't be joint into one field.
>>>
>>> regards,
>>> Daan
--
Daan
Re: cidrs in acls
Posted by Daan Hoogland <da...@gmail.com>.
H Kishan,
I implemented point 1, even though I had some doubts at what you
meant. If you have a minute please look at the last commit in
acl-item-cidrs again. I will work on the migration code as time falls
free.
thanks,
Daan
On Wed, Feb 26, 2014 at 9:12 AM, Daan Hoogland <da...@gmail.com> wrote:
> thanks, will find some time to add those.
>
> On Wed, Feb 26, 2014 at 7:36 AM, Kishan Kavala <Ki...@citrix.com> wrote:
>> Daan,
>> I looked at the code in acl-item-cidrs. Persisting cidrs in separate table looks good.
>> Pending items:
>>
>> 1. All references to NetworkACLItemVO.getSourceCidrList() should call NetworkACLItemDao.loadCidrs. Cidr list won't be available otherwise.
>> 2. Migration code should be added to upgrade path to move existing cidrs to new network_acl_item_cidr table
>>
>> Regards,
>> kishan
>>
>>> -----Original Message-----
>>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>>> Sent: Wednesday, 19 February 2014 8:33 PM
>>> To: dev; Kishan Kavala
>>> Subject: Re: cidrs in acls
>>>
>>> Kishan,
>>>
>>> Can you have a look at the branch acl-item-cidrs. I made some code to
>>> handle the cidrs from a separate table. I hardly think this can be enough and
>>> would like to create a checklist on what I need to do next.
>>> (item one is use the new transaction model;)
>>>
>>> thanks,
>>> Daan
>>>
>>> On Fri, Jan 17, 2014 at 1:19 PM, Daan Hoogland
>>> <da...@gmail.com> wrote:
>>> > That was what I thought as well. What was the retionale to join them
>>> > into one field?
>>> >
>>> > On Fri, Jan 17, 2014 at 8:32 AM, Kishan Kavala <Ki...@citrix.com>
>>> wrote:
>>> >> Daan,
>>> >> Similar to firewall_rules_cidrs, separate table can be used to store acl
>>> cidrs. Maybe in network_acl_item_cidrs.
>>> >>
>>> >> Regards,
>>> >> kishan
>>> >>
>>> >>> -----Original Message-----
>>> >>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>>> >>> Sent: Friday, 17 January 2014 1:05 AM
>>> >>> To: Kishan Kavala
>>> >>> Cc: dev
>>> >>> Subject: cidrs in acls
>>> >>>
>>> >>> H Kishan,
>>> >>>
>>> >>> I see you implemented CLOUDSTACK-763. it merges a lot of cidrs into
>>> one field.
>>> >>> The api doesn't check the field length. I enlarged the field in the
>>> >>> create table statement to 2048 for the 4.3 branch. Can you help me
>>> >>> think about a more solid solution, please. It seems to me those cidrs
>>> shouldn't be joint into one field.
>>> >>>
>>> >>> regards,
>>> >>> Daan
>>>
>>>
>>>
>>> --
>>> Daan
>
>
>
> --
> Daan
--
Daan
Re: cidrs in acls
Posted by Daan Hoogland <da...@gmail.com>.
thanks, will find some time to add those.
On Wed, Feb 26, 2014 at 7:36 AM, Kishan Kavala <Ki...@citrix.com> wrote:
> Daan,
> I looked at the code in acl-item-cidrs. Persisting cidrs in separate table looks good.
> Pending items:
>
> 1. All references to NetworkACLItemVO.getSourceCidrList() should call NetworkACLItemDao.loadCidrs. Cidr list won't be available otherwise.
> 2. Migration code should be added to upgrade path to move existing cidrs to new network_acl_item_cidr table
>
> Regards,
> kishan
>
>> -----Original Message-----
>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>> Sent: Wednesday, 19 February 2014 8:33 PM
>> To: dev; Kishan Kavala
>> Subject: Re: cidrs in acls
>>
>> Kishan,
>>
>> Can you have a look at the branch acl-item-cidrs. I made some code to
>> handle the cidrs from a separate table. I hardly think this can be enough and
>> would like to create a checklist on what I need to do next.
>> (item one is use the new transaction model;)
>>
>> thanks,
>> Daan
>>
>> On Fri, Jan 17, 2014 at 1:19 PM, Daan Hoogland
>> <da...@gmail.com> wrote:
>> > That was what I thought as well. What was the retionale to join them
>> > into one field?
>> >
>> > On Fri, Jan 17, 2014 at 8:32 AM, Kishan Kavala <Ki...@citrix.com>
>> wrote:
>> >> Daan,
>> >> Similar to firewall_rules_cidrs, separate table can be used to store acl
>> cidrs. Maybe in network_acl_item_cidrs.
>> >>
>> >> Regards,
>> >> kishan
>> >>
>> >>> -----Original Message-----
>> >>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>> >>> Sent: Friday, 17 January 2014 1:05 AM
>> >>> To: Kishan Kavala
>> >>> Cc: dev
>> >>> Subject: cidrs in acls
>> >>>
>> >>> H Kishan,
>> >>>
>> >>> I see you implemented CLOUDSTACK-763. it merges a lot of cidrs into
>> one field.
>> >>> The api doesn't check the field length. I enlarged the field in the
>> >>> create table statement to 2048 for the 4.3 branch. Can you help me
>> >>> think about a more solid solution, please. It seems to me those cidrs
>> shouldn't be joint into one field.
>> >>>
>> >>> regards,
>> >>> Daan
>>
>>
>>
>> --
>> Daan
--
Daan
RE: cidrs in acls
Posted by Kishan Kavala <Ki...@citrix.com>.
Daan,
I looked at the code in acl-item-cidrs. Persisting cidrs in separate table looks good.
Pending items:
1. All references to NetworkACLItemVO.getSourceCidrList() should call NetworkACLItemDao.loadCidrs. Cidr list won't be available otherwise.
2. Migration code should be added to upgrade path to move existing cidrs to new network_acl_item_cidr table
Regards,
kishan
> -----Original Message-----
> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
> Sent: Wednesday, 19 February 2014 8:33 PM
> To: dev; Kishan Kavala
> Subject: Re: cidrs in acls
>
> Kishan,
>
> Can you have a look at the branch acl-item-cidrs. I made some code to
> handle the cidrs from a separate table. I hardly think this can be enough and
> would like to create a checklist on what I need to do next.
> (item one is use the new transaction model;)
>
> thanks,
> Daan
>
> On Fri, Jan 17, 2014 at 1:19 PM, Daan Hoogland
> <da...@gmail.com> wrote:
> > That was what I thought as well. What was the retionale to join them
> > into one field?
> >
> > On Fri, Jan 17, 2014 at 8:32 AM, Kishan Kavala <Ki...@citrix.com>
> wrote:
> >> Daan,
> >> Similar to firewall_rules_cidrs, separate table can be used to store acl
> cidrs. Maybe in network_acl_item_cidrs.
> >>
> >> Regards,
> >> kishan
> >>
> >>> -----Original Message-----
> >>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
> >>> Sent: Friday, 17 January 2014 1:05 AM
> >>> To: Kishan Kavala
> >>> Cc: dev
> >>> Subject: cidrs in acls
> >>>
> >>> H Kishan,
> >>>
> >>> I see you implemented CLOUDSTACK-763. it merges a lot of cidrs into
> one field.
> >>> The api doesn't check the field length. I enlarged the field in the
> >>> create table statement to 2048 for the 4.3 branch. Can you help me
> >>> think about a more solid solution, please. It seems to me those cidrs
> shouldn't be joint into one field.
> >>>
> >>> regards,
> >>> Daan
>
>
>
> --
> Daan