On Wed, Jul 24, 2019 at 02:41:14PM +0200, Jan Tluka wrote:
Wed, Jul 24, 2019 at 01:29:08PM CEST, csfakian(a)redhat.com wrote:
>
>
>----- Original Message -----
>> From: "Jan Tluka" <jtluka(a)redhat.com>
>> To: csfakian(a)redhat.com
>> Cc: lnst-developers(a)lists.fedorahosted.org
>> Sent: Wednesday, July 24, 2019 12:05:08 PM
>> Subject: Re: [PATCH-next v2 8/8] lnst.Recipes.ENRT: rework recipes to adopt to
334e2c..7bde5c
>>
>> Tue, Jul 23, 2019 at 06:19:26PM CEST, csfakian(a)redhat.com wrote:
>> >From: Christos Sfakianakis <csfakian(a)redhat.com>
>> >
>> >Rewrite recipes to be compatible with the above patches.
>> >Rename SimplePerfRecipe to SimpleNetworkRecipe.
>> >
>> >Signed-off-by: Christos Sfakianakis <csfakian(a)redhat.com>
>> >---
>> > lnst/Recipes/ENRT/BondRecipe.py | 137 +++++---
>> > lnst/Recipes/ENRT/DoubleBondRecipe.py | 140 +++++---
>> > lnst/Recipes/ENRT/DoubleTeamRecipe.py | 159 ++++++----
>> > lnst/Recipes/ENRT/IpsecEspAeadRecipe.py | 280 ++++++++--------
>> > lnst/Recipes/ENRT/IpsecEspAhCompRecipe.py | 297 ++++++++---------
>> > lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py | 130 +++++---
>> > lnst/Recipes/ENRT/PingFloodRecipe.py | 21 +-
>> > .../ENRT/ShortLivedConnectionsRecipe.py | 87 ++---
>> > lnst/Recipes/ENRT/SimpleMacsecRecipe.py | 252 +++++++--------
>> > ...lePerfRecipe.py => SimpleNetworkRecipe.py} | 41 +--
>> > lnst/Recipes/ENRT/TeamRecipe.py | 134 +++++---
>> > lnst/Recipes/ENRT/TeamVsBondRecipe.py | 162 ++++++----
>> > lnst/Recipes/ENRT/VirtOvsVxlanRecipe.py | 230 +++++++++++---
>> > .../VirtualBridgeVlanInGuestMirroredRecipe.py | 191 ++++++-----
>> > .../ENRT/VirtualBridgeVlanInGuestRecipe.py | 167 ++++++----
>> > .../VirtualBridgeVlanInHostMirroredRecipe.py | 178 +++++++----
>> > .../ENRT/VirtualBridgeVlanInHostRecipe.py | 155 ++++++---
>> > .../ENRT/VirtualBridgeVlansOverBondRecipe.py | 298 +++++++++++++-----
>> > ...rtualOvsBridgeVlanInGuestMirroredRecipe.py | 193 +++++++-----
>> > .../ENRT/VirtualOvsBridgeVlanInGuestRecipe.py | 164 ++++++----
>> > ...irtualOvsBridgeVlanInHostMirroredRecipe.py | 155 +++++----
>> > .../ENRT/VirtualOvsBridgeVlanInHostRecipe.py | 144 ++++++---
>> > .../VirtualOvsBridgeVlansOverBondRecipe.py | 277 ++++++++++------
>> > lnst/Recipes/ENRT/VlansOverBondRecipe.py | 267 +++++++++++-----
>> > lnst/Recipes/ENRT/VlansOverTeamRecipe.py | 269 +++++++++++-----
>> > lnst/Recipes/ENRT/VlansRecipe.py | 230 ++++++++++----
>> > lnst/Recipes/ENRT/VxlanMulticastRecipe.py | 155 ++++++---
>> > lnst/Recipes/ENRT/VxlanRemoteRecipe.py | 115 +++++--
>> > lnst/Recipes/ENRT/__init__.py | 2 +-
>> > 29 files changed, 3195 insertions(+), 1835 deletions(-)
>> > rename lnst/Recipes/ENRT/{SimplePerfRecipe.py =>
SimpleNetworkRecipe.py}
>> > (73%)
>>
>> A note on the commit description ($subj). When we push the changes
>> the commit ids will be different or am I wrong? I'd remove them or
>> replace with 'to adopt previous changes'.
>>
>Ok, will remove these hashes from the subject line
>> I have one more comment. I asked Ondrej why following pattern is in
>> SimpleNetworkRecipe:
>>
>> + @property
>> + def offload_nics(self):
>> + return [self.matched.host1.bond0, self.matched.host2.bond0]
>> +
>> + @property
>> + def mtu_hw_config_dev_list(self):
>> + return self.offload_nics
>> +
>>
>> I meant why we use offload_nics for mtu_hwconfig_dev_list. Ondrej said
>> that the idea is to avoid copy-paste errors. I'm fine with that.
>>
>> But when I checked the rest of the code, there's a mixture of approach
>> there.
>>
>> E.g. following recipes does not follow this pattern:
>> lnst/Recipes/ENRT/IpsecEspAeadRecipe.py
>> lnst/Recipes/ENRT/IpsecEspAhCompRecipe.py
>> lnst/Recipes/ENRT/NoVirtOvsVxlanRecipe.py
>> lnst/Recipes/ENRT/SimpleMacsecRecipe.py
>>
>> I'd like to have this consistent.
>Ok, I will avoid referencing one property via another to avoid confusion and
>misleads.
Actually Ondrej wanted the other approach. It is okay to reference
one property via another.
I need to think about this a bit more, I'll send a proper reply
tomorrow.
Thanks,
-Ondrej