Change in vdsm[master]: net: mtu should be reported as a string through caps

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5: Code-Review+2

Let us unbreak master first.

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[danken]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: net: mtu should be reported as a string through caps
..


net: mtu should be reported as a string through caps

Previous change (commit 59ede0b) migrated all mtu values to
integers internally, including at the API entry point.
One point has been missed though, the caps reoport to the
engine: engine (3.6) expects a string mtu, however vdsm reports
an integer.

This change reports the mtu to engine as a string for
compatibility with engine ver < 3.7.

Note: Several functional tests had to be cleaned up to support
this change:
- mtu assertion.
- running vs kernel config validation should use netinfo (and
  not caps).

Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Signed-off-by: Dan Kenigsberg 
Signed-off-by: Edward Haas 
Reviewed-on: https://gerrit.ovirt.org/51164
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer 
---
M lib/vdsm/netinfo/__init__.py
M tests/functional/networkTests.py
M vdsm/caps.py
3 files changed, 44 insertions(+), 20 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved
  Edward Haas: Verified



-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 6:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/5/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 104: 
Line 105: return networking
Line 106: 
Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
> So lets use mtu_as_string=True, it is good enough for now.
get(mtu_as_string=True) is strange, I prefer what we have now.
I don't see why the caller needs to know about this compatibility issue 
specifically, it just needs to know about compatibility in general. 
What is the benefit in changing the method signature on every expansion? 
(defaults included)

I expect caps to require more such conversions as we clean-up the code (STP is 
next).
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 30700:
Line 112: # REQUIRED_FOR engine < 3.7


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/4/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 370:
> 3 * 10**4 = 3
Lets add utility function for creating version number from version string:

"3.6.1" -> 30601
"3.6"   -> 30600
"4" -> 4

(In another patch)
Line 112: # REQUIRED_FOR engine < 3.7
Line 113: return _stringify_mtus(_get(vdsmnets))
Line 114: 
Line 115: return _get(vdsmnets)


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 4: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 4: Code-Review-1

(3 comments)

https://gerrit.ovirt.org/#/c/51164/4/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 370:
370 is version 3.70, will fail for version 3.7 (major * 100 + minor).
Line 112: # REQUIRED_FOR engine < 3.7
Line 113: return _stringify_mtus(_get(vdsmnets))
Line 114: 
Line 115: return _get(vdsmnets)


https://gerrit.ovirt.org/#/c/51164/4/tests/functional/networkTests.py
File tests/functional/networkTests.py:

Line 381: 
Line 382: def assertMtu(self, mtu, *elems):
Line 383: # Due to compatibility with engine, the expected mtu type is 
string
Line 384: # REQUIRED_FOR engine < 3.7
Line 385: mtu = str(mtu)
Why getMtu should return a string? We have backward compatibility layer 
(netinfo.get()) to stringify muts, so this test should test that mtu is integer.
Line 386: for elem in elems:
Line 387: self.assertEquals(mtu, self.vdsm_net.getMtu(elem))
Line 388: 
Line 389: def assert_active_slave_exists(self, bondName, nics):


https://gerrit.ovirt.org/#/c/51164/4/vdsm/caps.py
File vdsm/caps.py:

Line 630: 
Line 631: caps.update(_getVersionInfo())
Line 632: 
Line 633: # TODO: Version requests by engine to ease handling of 
compatibility.
Line 634: netinfo_data = netinfo.get(compatibility=360)
Should be 306
Line 635: caps.update(netinfo_data)
Line 636: 
Line 637: try:
Line 638: caps['hooks'] = hooks.installed()


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/4/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 370:
> How about 30700? (I would not mind either)
I think you mean 300700 (major * 10**4 + minor * 10**2 + patch).

Better if we plan to change api on patch versions, but doing such changes 
should be avoided. However seems that we are doing this already (see "new in 
version" in the schema).
Line 112: # REQUIRED_FOR engine < 3.7
Line 113: return _stringify_mtus(_get(vdsmnets))
Line 114: 
Line 115: return _get(vdsmnets)


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 4:

(3 comments)

https://gerrit.ovirt.org/#/c/51164/4/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 370:
> How about 30700? (I would not mind either)
Done
Line 112: # REQUIRED_FOR engine < 3.7
Line 113: return _stringify_mtus(_get(vdsmnets))
Line 114: 
Line 115: return _get(vdsmnets)


Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 370:
> I think you mean 300700 (major * 10**4 + minor * 10**2 + patch).
3 * 10**4 = 3
7 * 10**2 = 00700
Line 112: # REQUIRED_FOR engine < 3.7
Line 113: return _stringify_mtus(_get(vdsmnets))
Line 114: 
Line 115: return _get(vdsmnets)


https://gerrit.ovirt.org/#/c/51164/4/vdsm/caps.py
File vdsm/caps.py:

Line 630: 
Line 631: caps.update(_getVersionInfo())
Line 632: 
Line 633: # TODO: Version requests by engine to ease handling of 
compatibility.
Line 634: netinfo_data = netinfo.get(compatibility=360)
> Should be 306
Changing to 30600
Line 635: caps.update(netinfo_data)
Line 636: 
Line 637: try:
Line 638: caps['hooks'] = hooks.installed()


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 4: Code-Review+2

Let's unbreak master

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 4: -Code-Review

(2 comments)

https://gerrit.ovirt.org/#/c/51164/4/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 370:
> 370 is version 3.70, will fail for version 3.7 (major * 100 + minor).
How about 30700? (I would not mind either)
Line 112: # REQUIRED_FOR engine < 3.7
Line 113: return _stringify_mtus(_get(vdsmnets))
Line 114: 
Line 115: return _get(vdsmnets)


https://gerrit.ovirt.org/#/c/51164/4/tests/functional/networkTests.py
File tests/functional/networkTests.py:

Line 381: 
Line 382: def assertMtu(self, mtu, *elems):
Line 383: # Due to compatibility with engine, the expected mtu type is 
string
Line 384: # REQUIRED_FOR engine < 3.7
Line 385: mtu = str(mtu)
> Why getMtu should return a string? We have backward compatibility layer (ne
getMtu() should keep returning the MTU as reported by Vdsm's API. And we would 
like assert that this is a string.
Line 386: for elem in elems:
Line 387: self.assertEquals(mtu, self.vdsm_net.getMtu(elem))
Line 388: 
Line 389: def assert_active_slave_exists(self, bondName, nics):


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/5/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 104: 
Line 105: return networking
Line 106: 
Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
> That means we will need to change the default each time we add, remove of m
Current we have 2 options - 3.7 format (mtu is int), and 3.6 format (mut is 
string). The code should reflect these options. The additional None option is 
same as 3.7, so I don't see a need this option.
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 30700:
Line 112: # REQUIRED_FOR engine < 3.7


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/5/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 104: 
Line 105: return networking
Line 106: 
Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
> Current we have 2 options - 3.7 format (mtu is int), and 3.6 format (mut is
I see it slightly differently. We have master Engine (with no compatibility 
demands) and 3.6 Engine (that needs strings).

And in any case, I consider this a minor issue (I wanted an explicit 
mtu_as_string=True flag) relative to the need to unbreak master.
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 30700:
Line 112: # REQUIRED_FOR engine < 3.7


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/4/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 370:
> Lets add utility function for creating version number from version string:
There is one, will fix it in another patch.
Line 112: # REQUIRED_FOR engine < 3.7
Line 113: return _stringify_mtus(_get(vdsmnets))
Line 114: 
Line 115: return _get(vdsmnets)


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5:

(2 comments)

https://gerrit.ovirt.org/#/c/51164/5/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 104: 
Line 105: return networking
Line 106: 
Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
Lets make the default 30700
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 30700:
Line 112: # REQUIRED_FOR engine < 3.7


Line 105: return networking
Line 106: 
Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
Line 109: if compatibility is None:
And now we can remove this branch.
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 30700:
Line 112: # REQUIRED_FOR engine < 3.7
Line 113: return _stringify_mtus(_get(vdsmnets))


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/5/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 104: 
Line 105: return networking
Line 106: 
Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
> Lets make the default 30700
That means we will need to change the default each time we add, remove of 
modify compatibility issues.
I prefer the default to be "no compatibility' explicitly.
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 30700:
Line 112: # REQUIRED_FOR engine < 3.7


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5: Code-Review+1

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 5:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/5/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 104: 
Line 105: return networking
Line 106: 
Line 107: 
Line 108: def get(vdsmnets=None, compatibility=None):
> I see it slightly differently. We have master Engine (with no compatibility
So lets use mtu_as_string=True, it is good enough for now.
Line 109: if compatibility is None:
Line 110: return _get(vdsmnets)
Line 111: elif compatibility < 30700:
Line 112: # REQUIRED_FOR engine < 3.7


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/3/vdsm/caps.py
File vdsm/caps.py:

Line 634: netinfo_data
> I don't see benefit of defining this temporary variable.
I did it to express what data is being updated.
It also fits (in my mind) to the ability to choose the correct netinfo data 
based on the client version in the future.


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

Verified by running all networking functional tests.
They cover these points:
- setupNetwork using string and integer mtu values.
- Validating (per setupNetwork) that the running and kernel mtu/s are equal 
(and integers).
- The mtu in the caps report is always a string, no matter the input.

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/51164/3/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 97: 
Line 98: return networking
Line 99: 
Line 100: 
Line 101: def get_with_engine36_compatibility():
> This returns info from the netinfo module, so netinfo.get() is not that bad
I agree that adding the compatibility into get() guts is unwanted, so lets do 
ti like this:

def get(compatibility=None):
if compatibility is None:
return _get()
else:
return _stringify_mtus(_get())
Line 102: # REQUIRED_FOR engine < 3.7
Line 103: return _stringify_mtus(get())
Line 104: 
Line 105: 


Line 98: return networking
Line 99: 
Line 100: 
Line 101: def get_with_engine36_compatibility():
Line 102: # REQUIRED_FOR engine < 3.7
We don't what next version will be (3.7, 4.0), so better mention version we 
know:

engine <= 3.6
Line 103: return _stringify_mtus(get())
Line 104: 
Line 105: 
Line 106: def _stringify_mtus(netinfo_data):


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

(2 comments)

could you state how this was verified?

https://gerrit.ovirt.org/#/c/51164/3/vdsm/caps.py
File vdsm/caps.py:

Line 629: _getCompatibleCpuModels())
Line 630: 
Line 631: caps.update(_getVersionInfo())
Line 632: 
Line 633: # TODO: Version requests by engine to ease handling of 
compatability.
please see current plans in https://gerrit.ovirt.org/#/c/50032/
Line 634: netinfo_data = netinfo.get_with_engine36_compatibility()
Line 635: caps.update(netinfo_data)
Line 636: 
Line 637: try:


Line 634: netinfo_data
I don't see benefit of defining this temporary variable.
(note that in another place in this patchset you *drop* such a temporary 
variable).


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/51164/3/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 97: 
Line 98: return networking
Line 99: 
Line 100: 
Line 101: def get_with_engine36_compatibility():
> Too ugly, we don't want such apis.
I don't like the compatibility logic to enter into the main method logic, that 
should be handled one layer up.
This will also be a good opportunity to replace the name "get" which does not 
describe what the method really does.

Anyway, if dan agrees as well to the api change, I will do it and add a layer: 
get(compatibility=None) calling get() and adding the if.
Line 102: # REQUIRED_FOR engine < 3.7
Line 103: return _stringify_mtus(get())
Line 104: 
Line 105: 


https://gerrit.ovirt.org/#/c/51164/3/tests/functional/networkTests.py
File tests/functional/networkTests.py:

Line 406: return status, msg
Line 407: 
Line 408: def _assert_kernel_config_matches_running_config(self):
Line 409: bare_kernel_config = kernelconfig.KernelConfig(
Line 410: vdsm.netinfo.CachingNetInfo())
> Why is this needed?
This is the 'real' netinfo data, the previous was the caps data (although the 
naming does not hint it).
The kernel config needs to be compared to the netinfo data.
Line 411: bare_running_config = self.vdsm_net.config
Line 412: normalized_running_config = 
kernelconfig.normalize(bare_running_config)
Line 413: # Unify strings to unicode instances so differences are 
easier to
Line 414: # understand. This won't be needed once we move to Python 3.


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/3/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 97: 
Line 98: return networking
Line 99: 
Line 100: 
Line 101: def get_with_engine36_compatibility():
> I don't like the compatibility logic to enter into the main method logic, t
This returns info from the netinfo module, so netinfo.get() is not that bad 
name. I think info() would be a better name, and also match other names (e.g. 
bonding.info())

But this patch is not about improving names but about restoring backward 
compatibility. Lets handle the name in another patch?
Line 102: # REQUIRED_FOR engine < 3.7
Line 103: return _stringify_mtus(get())
Line 104: 
Line 105: 


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/51164/3/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 97: 
Line 98: return networking
Line 99: 
Line 100: 
Line 101: def get_with_engine36_compatibility():
> I agree that adding the compatibility into get() guts is unwanted, so lets 
Done
Line 102: # REQUIRED_FOR engine < 3.7
Line 103: return _stringify_mtus(get())
Line 104: 
Line 105: 


Line 98: return networking
Line 99: 
Line 100: 
Line 101: def get_with_engine36_compatibility():
Line 102: # REQUIRED_FOR engine < 3.7
> We don't what next version will be (3.7, 4.0), so better mention version we
I have no opinion on this, Dan?
I see no real difference.
Line 103: return _stringify_mtus(get())
Line 104: 
Line 105: 
Line 106: def _stringify_mtus(netinfo_data):


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[nsoffer]

Nir Soffer has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/51164/3/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 97: 
Line 98: return networking
Line 99: 
Line 100: 
Line 101: def get_with_engine36_compatibility():
Too ugly, we don't want such apis.

Lets move this into get(compatibility=306)

At the end of get():

   if compatibility <= 306:
   networking = _strinfify_muts(networking)
Line 102: # REQUIRED_FOR engine < 3.7
Line 103: return _stringify_mtus(get())
Line 104: 
Line 105: 


https://gerrit.ovirt.org/#/c/51164/3/tests/functional/networkTests.py
File tests/functional/networkTests.py:

Line 406: return status, msg
Line 407: 
Line 408: def _assert_kernel_config_matches_running_config(self):
Line 409: bare_kernel_config = kernelconfig.KernelConfig(
Line 410: vdsm.netinfo.CachingNetInfo())
Why is this needed?
Line 411: bare_running_config = self.vdsm_net.config
Line 412: normalized_running_config = 
kernelconfig.normalize(bare_running_config)
Line 413: # Unify strings to unicode instances so differences are 
easier to
Line 414: # understand. This won't be needed once we move to Python 3.


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[danken]

Dan Kenigsberg has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/51164/3/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 98: return networking
Line 99: 
Line 100: 
Line 101: def get_with_engine36_compatibility():
Line 102: # REQUIRED_FOR engine < 3.7
> I have no opinion on this, Dan?
Neither do I see a difference.
Line 103: return _stringify_mtus(get())
Line 104: 
Line 105: 
Line 106: def _stringify_mtus(netinfo_data):


https://gerrit.ovirt.org/#/c/51164/3/vdsm/caps.py
File vdsm/caps.py:

Line 634: netinfo_data
> I did it to express what data is being updated.
sorry, I don't understand the explanation.


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/3/vdsm/caps.py
File vdsm/caps.py:

Line 634: netinfo_data
> sorry, I don't understand the explanation.
Something like caps.update(netinfo.get()) gives no clue on what data is being 
'updated' into caps.


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/1/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 101: def stringify_mtus(networking):
Line 102: # REQUIRED_FOR engine < 3.7
Line 103: for devtype in ('bondings', 'bridges', 'networks', 'nics', 
'vlans'):
Line 104: for dev in networking[devtype]:
Line 105: dev['mtu'] = str(dev['mtu'])
> Testing that this returns a string mtu is not enough, we should test that i
Nir, the netinfo data used here is a copy, not cached or internal.
We have tests that cover this anyway, part of the functional tests. They have 
been just type agnostic, so I changed that in patch 2.
Line 106: return networking
Line 107: 
Line 108: 
Line 109: def libvirtNets2vdsm(nets, running_config=None, routes=None, 
ipAddrs=None,


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/51164/2/lib/vdsm/netinfo/__init__.py
File lib/vdsm/netinfo/__init__.py:

Line 56: , engine36_compatibility=False
remove


-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[automation]

gerrit-hooks has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' 
and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Change in vdsm[master]: net: mtu should be reported as a string through caps

[edwardh]

Edward Haas has posted comments on this change.

Change subject: net: mtu should be reported as a string through caps
..


Patch Set 3: Verified+1

-- 
To view, visit https://gerrit.ovirt.org/51164
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I719ed889bfae763ce7cbed4f2aab3f6134ba2865
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches