From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: network: report dpdk ports as nics
......................................................................
Patch Set 10:
(3 comments)
https://gerrit.ovirt.org/#/c/73987/10/lib/vdsm/network/ipwrapper.py
File lib/vdsm/network/ipwrapper.py:
PS10, Line 277: getLinks
this function is not tested yet, but the Right Thing to do is to add a test for it.
https://gerrit.ovirt.org/#/c/73987/10/lib/vdsm/network/link/dpdk.py
File lib/vdsm/network/link/dpdk.py:
Line 52:
Line 53:
Line 54: def link_info(dpdk_link):
Line 55: """Returns a dictionary with the information of the link object."""
Line 56: info = {}
to repeat Eddy's comment:
info = {
'address': _get_hw_addr(dpdk_link),
'flags': '',
'index': '',
...
}
is more compact
Line 57: info['address'] = _get_hw_addr(dpdk_link)
Line 58: info['flags'] = ''
Line 59: info['index'] = ''
Line 60: info['mtu'] = ''
https://gerrit.ovirt.org/#/c/73987/10/lib/vdsm/network/netinfo/cache.py
File lib/vdsm/network/netinfo/cache.py:
PS10, Line 106: dev.isDPDK()
frankly, testing
if dev.name in dpdk_devices
is safer than isDPDK()'s implementation, and probably a bit faster.
--
To view, visit https://gerrit.ovirt.org/73987
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I67b2083e9c6f032472fd75f6800c864083ffdef5
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: nics: list dpdk ports
......................................................................
Patch Set 17: Code-Review-1
(4 comments)
https://gerrit.ovirt.org/#/c/72308/17/lib/vdsm/network/link/dpdk.py
File lib/vdsm/network/link/dpdk.py:
PS17, Line 54: isDPDK
why's the sudden uppercase? new module should have pep8-conforming names.
PS17, Line 56: NET_SYSFS + dev_name
os.path.join is slightly safer
PS17, Line 92: not dev_name or not
why is this special protection agains dev_name in (0, None, False) ?
https://gerrit.ovirt.org/#/c/72308/17/tests/network/dpdk_test.py
File tests/network/dpdk_test.py:
Line 93: "configuration": {
Line 94: "driver": "igb"}}]}]}
Line 95:
Line 96: num_ports = sum(1 for x in dpdk._device_ports(lshw_output))
Line 97: self.assertEqual(0, num_ports)
we care about the content of the items, not only their number.
self.assertEqual([], list(dpdk._device_ports(lshw_output)))
is much clearer, and can be replaced in the other two places with the actual value expected from _device_ports.
--
To view, visit https://gerrit.ovirt.org/72308
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia07f5a1052146c0eea6f0e19aa22d68ff4148646
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Peřina <mperina(a)redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: virt: metadata: get rid of _XMLWrapper
......................................................................
virt: metadata: get rid of _XMLWrapper
We added _XMLWrapper out of necessity, because we possibly
need to *replace* (or at very least update) the object
returned by the interal _domain_xml context manager.
In this patch we reduce the amount of cerimony replacing
the class with much simpler plain list wrapper.
Also, we rename the _domain_xml helper to a more correct
_metadata_xml.
No changes in behaviour for the clients of the metadata
module, tests included.
Change-Id: I2e454495b4189e1656f9357e1d26bda105c7ad50
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M lib/vdsm/virt/metadata.py
1 file changed, 12 insertions(+), 26 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Dan Kenigsberg: Looks good to me, approved
Francesco Romani: Verified
Martin Polednik: Looks good to me, but someone else must approve
--
To view, visit https://gerrit.ovirt.org/74861
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2e454495b4189e1656f9357e1d26bda105c7ad50
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: virt: metadata: get rid of _XMLWrapper
......................................................................
Patch Set 1: Code-Review+2
raising
--
To view, visit https://gerrit.ovirt.org/74861
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e454495b4189e1656f9357e1d26bda105c7ad50
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: virt: metadata: extract helper for dom metadata
......................................................................
virt: metadata: extract helper for dom metadata
Extract one helper context manager to take care
of the get-update-set cycle of the domain metadata.
It will be used to implement the per-device metadata
handler code.
No change in (the current) behaviour.
Change-Id: I2ece6973823a721bc5ec756e25649b1cf33dec90
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M lib/vdsm/virt/metadata.py
1 file changed, 29 insertions(+), 9 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Dan Kenigsberg: Looks good to me, approved
Francesco Romani: Verified
Milan Zamazal: Looks good to me, but someone else must approve
--
To view, visit https://gerrit.ovirt.org/74738
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I2ece6973823a721bc5ec756e25649b1cf33dec90
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: containers: fetch image name from metadata
......................................................................
containers: fetch image name from metadata
In commit 46ba88dc557ee53adb8b3d20709a02a678910f6b
we started to remove smartness from the container domain parser.
The original plan was to get the image from the outside;
With the current plans to overhaul the domain metadata, it's
just simpler to add a new helper which parses it from the metadata.
This patch effectively restores part of the code removed in 46ba88dc55,
but now we call this explicitely, so we complete the removal
of unwanted and implicit smartness.
Change-Id: I78946d0ed8328d3196d8df1c461313258b5e25f0
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M lib/vdsm/virt/containers/docker.py
M lib/vdsm/virt/containers/xmlfile.py
M tests/containers/data/full_dom.xml
M tests/containers/data/metadata_drive_map.xml
M tests/containers/data/minimal_dom.xml
M tests/containers/docker_test.py
6 files changed, 48 insertions(+), 31 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Dan Kenigsberg: Looks good to me, approved
Francesco Romani: Verified
Martin Polednik: Looks good to me, but someone else must approve
Milan Zamazal: Looks good to me, but someone else must approve
--
To view, visit https://gerrit.ovirt.org/74322
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I78946d0ed8328d3196d8df1c461313258b5e25f0
Gerrit-PatchSet: 14
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: virt: metadata: extract helper for dom metadata
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/74738
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2ece6973823a721bc5ec756e25649b1cf33dec90
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: containers: fetch image name from metadata
......................................................................
Patch Set 13: Code-Review+2
--
To view, visit https://gerrit.ovirt.org/74322
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I78946d0ed8328d3196d8df1c461313258b5e25f0
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No