Change in vdsm[master]: net: Drop blockingdhcp persistent config on read and write
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: net: Drop blockingdhcp persistent config on read and write
......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/73106/1//COMMIT_MSG
Commit Message:
PS1, Line 12: This behaviour attribute is supose to be volatile, not saved in any
: persistent config.
> That is an indirect implication of requesting the dhcp blocking behavior.
I'm not sure I agree with Vdsm not caring if `ifup` is successful or not. As a user, I prefer synchronous APIs, that returns when dchp has finished, rather than "I got your request, but it might be bogus, ask me again some time in the future if things are fine".
But for blockingdhcp Eddy is correct: on recovery we are ALWAYS waiting for dhcp response; we behave as if blockingdhcp=True regardless of what is in running config.
So there is not reason to blockingdhcp in RunningConfig.
--
To view, visit https://gerrit.ovirt.org/73106
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7ad81ba1d342db2ed560e95c79e5e96ca3da918c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Leon Goldberg <lgoldber(a)redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 1 month
Change in vdsm[master]: virt: network: factor out update helper code
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: virt: network: factor out update helper code
......................................................................
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/72337/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:
PS5, Line 2409: specParams=None
this default value should die - different patch, maybe?
--
To view, visit https://gerrit.ovirt.org/72337
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I66e9618dbeb4280aede57e26ca389cea6fe1af31
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 1 month
Change in vdsm[ovirt-4.0]: ngn: grab OS version according with /etc/os-release
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: ngn: grab OS version according with /etc/os-release
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.ovirt.org/73180
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f35d8f2221c07a38e9d75b96324315266b0d51a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-HasComments: No
7 years, 1 month
Change in vdsm[ovirt-4.0]: after_disk_prepare: Add new hook point
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: after_disk_prepare: Add new hook point
......................................................................
Patch Set 1: Code-Review-1
I do not think we need this in 4.0
--
To view, visit https://gerrit.ovirt.org/71711
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib150682c4afd7f387eabf6873b10eb5eaa858417
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Freddy Rolland <frolland(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: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 1 month
Change in vdsm[master]: virt: network: remove move iteritems() calls
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: virt: network: remove move iteritems() calls
......................................................................
virt: network: remove move iteritems() calls
They have negligible performance impact (we are iterating over
small dicts!), but prevent the move to python3.
Change-Id: I0bb3e2e38e7daa0d150ae13c48a96c8a4114fd5a
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M lib/vdsm/virt/vmdevices/network.py
1 file changed, 2 insertions(+), 2 deletions(-)
Approvals:
Nir Soffer: Looks good to me, approved
Jenkins CI: Passed CI tests
Francesco Romani: Verified
Milan Zamazal: Looks good to me, but someone else must approve
--
To view, visit https://gerrit.ovirt.org/72338
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0bb3e2e38e7daa0d150ae13c48a96c8a4114fd5a
Gerrit-PatchSet: 6
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: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
7 years, 1 month
Change in vdsm[master]: virt: move vmxml.Domain to its own module
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has submitted this change and it was merged.
Change subject: virt: move vmxml.Domain to its own module
......................................................................
virt: move vmxml.Domain to its own module
We move vmxml.Domain into the newly introduced libvirtxml.py module.
This way we isolate the utility XML processing code from the code which
uses it to build the libvirt XML. The key benefit is that with this
patch we can reuse the utility code in the upcoming metadata handling,
module, avoiding circular dependencies.
Other lesser benefits are the slightly easier future removal of the XML
building code, and the fact that 'libvirtxml.Domain' reads a bit nicer.
Change-Id: Ie2cecd05ac50c1514045219983bac79f0ff9ec09
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M lib/vdsm/virt/Makefile.am
A lib/vdsm/virt/libvirtxml.py
M lib/vdsm/virt/vmxml.py
M lib/vdsm/virt/xmlconstants.py
M tests/hostdev_test.py
M tests/vm_test.py
M tests/vmxml_test.py
M vdsm.spec.in
M vdsm/virt/vm.py
9 files changed, 567 insertions(+), 527 deletions(-)
Approvals:
Jenkins CI: Passed CI tests
Dan Kenigsberg: Looks good to me, approved
Francesco Romani: Verified
--
To view, visit https://gerrit.ovirt.org/72015
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie2cecd05ac50c1514045219983bac79f0ff9ec09
Gerrit-PatchSet: 10
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>
7 years, 1 month
Change in vdsm[master]: virt: network: factor out update helper code
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: virt: network: factor out update helper code
......................................................................
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/72337/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:
PS5, Line 2410: vnicXML = dev.getXML()
> Missed that. So why not pass it down to the new function? It is currently r
It is quite ugly to call pass both dev and the calculated dev.getXML() into the same function call.
I don't know how expensive is the getXML() call and if it is worth to make the new function self-consistent.
--
To view, visit https://gerrit.ovirt.org/72337
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I66e9618dbeb4280aede57e26ca389cea6fe1af31
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 1 month
Change in vdsm[master]: virt: move vmxml.Domain to its own module
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: virt: move vmxml.Domain to its own module
......................................................................
Patch Set 9: Code-Review+2
raising Milan's score
--
To view, visit https://gerrit.ovirt.org/72015
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2cecd05ac50c1514045219983bac79f0ff9ec09
Gerrit-PatchSet: 9
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
7 years, 1 month
Change in vdsm[master]: vmxml: separate vmxml.Element and etree.Element
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: vmxml: separate vmxml.Element and etree.Element
......................................................................
Patch Set 9: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/72089/9/lib/vdsm/virt/vmxml.py
File lib/vdsm/virt/vmxml.py:
Line 223: :param child: child element to add to `element`
Line 224: :type child: DOM element
Line 225:
Line 226: """
Line 227: element.append(child._elem)
> That's at least confusing. Do we really want to crash if etree Element is p
I think that we do. Python allows dynamic typing, but using it when not necessary is the confusing thing imo.
However, I only had a question - whether polymorphism is needed in this case. I now see that there are two places that pass different types.
I prefer the current code, but would taken PS1 as well.
Line 228:
Line 229:
Line 230: def remove_child(element, child):
Line 231: """
--
To view, visit https://gerrit.ovirt.org/72089
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I60003e69702508e173703fcfa7d5ec2e0cffbd14
Gerrit-PatchSet: 9
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: Milan Zamazal <mzamazal(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 1 month
Change in vdsm[ovirt-4.1]: lvm: enabling movePV()
by Code Review
From Dan Kenigsberg <danken(a)redhat.com>:
Dan Kenigsberg has posted comments on this change.
Change subject: lvm: enabling movePV()
......................................................................
Patch Set 1: Code-Review+1
Trusting Nir on taking non-4.1.1-blocker now.
--
To view, visit https://gerrit.ovirt.org/72978
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a0d4a5f6e070f94f6d7a4556f01887571417824
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.1
Gerrit-Owner: Liron Aravot <laravot(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: Liron Aravot <laravot(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-HasComments: No
7 years, 1 month