Change in vdsm[master]: fileUtils: Normalize initial double slashes
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: fileUtils: Normalize initial double slashes
......................................................................
fileUtils: Normalize initial double slashes
POSIX allows both /path and //path. The second slash may be interpreted
in an implementation-defined manner. The Linux interpretation seems to
be to ignore the double slash, so it seems to be safe to remove it.
See https://bugs.python.org/issue26329 for more info.
Change-Id: Ifa3f8724e9a423263b5676d691308d0188b26935
Bug-Url: XXX
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/vdsm/storage/fileUtils.py
M tests/fileUtilTests.py
2 files changed, 20 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/78/61578/1
diff --git a/lib/vdsm/storage/fileUtils.py b/lib/vdsm/storage/fileUtils.py
index fcd8829..49ee937 100644
--- a/lib/vdsm/storage/fileUtils.py
+++ b/lib/vdsm/storage/fileUtils.py
@@ -36,8 +36,6 @@
import types
import warnings
-from os.path import normpath
-
import six
from vdsm import constants
@@ -106,6 +104,22 @@
return address.hosttail_join(host, tail)
+def normpath(path):
+ """
+ Normalize file system path.
+
+ POSIX allows both /path and //path. The second slash may be interpreted in
+ an implementation-defined manner. The Linux interpretation seems to be to
+ ignore the double slash, so it seems to be safe to remove it.
+
+ See https://bugs.python.org/issue26329 for more info.
+ """
+ path = os.path.normpath(path)
+ if path.startswith('//'):
+ path = path[1:]
+ return path
+
+
def is_port(port_str):
if port_str.startswith('0'):
return False
diff --git a/tests/fileUtilTests.py b/tests/fileUtilTests.py
index a9a7270..ec4c278 100644
--- a/tests/fileUtilTests.py
+++ b/tests/fileUtilTests.py
@@ -180,10 +180,14 @@
@permutations([
# Remote paths without a port
("server:/path", "server:/path"),
+ ("server://path", "server:/path"),
+ ("server:///path", "server:/path"),
("server:/path/", "server:/path"),
("server:/pa:th", "server:/pa:th"),
("server:/path//", "server:/path"),
("server:/", "server:/"),
+ ("server://", "server:/"),
+ ("server:///", "server:/"),
("12.34.56.78:/", "12.34.56.78:/"),
("[2001:db8::60fe:5bf:febc:912]:/", "[2001:db8::60fe:5bf:febc:912]:/"),
("server:01234:/", "server:01234:"),
--
To view, visit https://gerrit.ovirt.org/61578
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifa3f8724e9a423263b5676d691308d0188b26935
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
7 years, 9 months
Change in vdsm[master]: ifcfg: pass a NetDevice to _createConfFile
by osvoboda＠redhat.com
Ondřej Svoboda has uploaded a new change for review.
Change subject: ifcfg: pass a NetDevice to _createConfFile
......................................................................
ifcfg: pass a NetDevice to _createConfFile
If we want to handle DNS1/2 in this function, we should give
it all information stored in a NetDevice instance.
Change-Id: I67db38cb7253bff14f86019a52fef3d736a65c1f
Signed-off-by: Ondřej Svoboda <osvoboda(a)redhat.com>
---
M lib/vdsm/network/configurators/ifcfg.py
1 file changed, 20 insertions(+), 16 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/61471/1
diff --git a/lib/vdsm/network/configurators/ifcfg.py b/lib/vdsm/network/configurators/ifcfg.py
index e9f67e1..3487103 100644
--- a/lib/vdsm/network/configurators/ifcfg.py
+++ b/lib/vdsm/network/configurators/ifcfg.py
@@ -517,8 +517,9 @@
if self.unifiedPersistence and utils.isOvirtNode():
node_fs.Config().persist(fileName)
- def _createConfFile(self, conf, name, ipv4, ipv6, mtu=None, **kwargs):
+ def _createConfFile(self, conf, device):
""" Create ifcfg-* file with proper fields per device """
+ name, ipv4, ipv6 = device.name, device.ipv4, device.ipv6
cfg = 'DEVICE=%s\n' % pipes.quote(name)
cfg += conf
if ipv4.address:
@@ -536,8 +537,8 @@
# Ask dhclient to stop any dhclient running for the device
dhclient.kill(name)
address.flush(name, family=4)
- if mtu:
- cfg += 'MTU=%d\n' % mtu
+ if device.mtu:
+ cfg += 'MTU=%d\n' % device.mtu
if ipv4.defaultRoute is not None:
cfg += 'DEFROUTE=%s\n' % _to_ifcfg_bool(ipv4.defaultRoute)
cfg += 'NM_CONTROLLED=no\n'
@@ -575,8 +576,7 @@
if 'custom' in opts and 'bridge_opts' in opts['custom']:
conf += 'BRIDGING_OPTS="%s"\n' % opts['custom']['bridge_opts']
- self._createConfFile(conf, bridge.name, bridge.ipv4, bridge.ipv6,
- bridge.mtu, **opts)
+ self._createConfFile(conf, bridge)
def addVlan(self, vlan, **opts):
""" Create ifcfg-* file with proper fields for VLAN """
@@ -585,8 +585,7 @@
if vlan.bridge:
conf += 'BRIDGE=%s\n' % pipes.quote(vlan.bridge.name)
conf += 'ONBOOT=yes\n'
- self._createConfFile(conf, vlan.name, vlan.ipv4, vlan.ipv6, vlan.mtu,
- **opts)
+ self._createConfFile(conf, vlan)
def addBonding(self, bond, **opts):
""" Create ifcfg-* file with proper fields for bond """
@@ -598,8 +597,8 @@
conf += 'BRIDGE=%s\n' % pipes.quote(bond.bridge.name)
conf += 'ONBOOT=yes\n'
- ipv4, ipv6, mtu = self._getIfaceConfValues(bond)
- self._createConfFile(conf, bond.name, ipv4, ipv6, mtu, **opts)
+ bond_updated_from_ifcfg = self._update_from_ifcfg(bond)
+ self._createConfFile(conf, bond_updated_from_ifcfg)
# create the bonding device to avoid initscripts noise
with open(netinfo_bonding.BONDING_MASTERS) as info:
@@ -624,15 +623,15 @@
if ethtool_opts:
conf += 'ETHTOOL_OPTS=%s\n' % pipes.quote(ethtool_opts)
- ipv4, ipv6, mtu = self._getIfaceConfValues(nic)
- self._createConfFile(conf, nic.name, ipv4, ipv6, mtu, **opts)
+ nic_updated_from_ifcfg = self._update_from_ifcfg(nic)
+ self._createConfFile(conf, nic_updated_from_ifcfg)
@staticmethod
- def _getIfaceConfValues(iface):
- ipv4 = copy.deepcopy(iface.ipv4)
- ipv6 = copy.deepcopy(iface.ipv6)
- mtu = iface.mtu
+ def _update_from_ifcfg(iface):
if ifaceUsed(iface.name):
+ ipv4 = copy.deepcopy(iface.ipv4)
+ ipv6 = copy.deepcopy(iface.ipv6)
+ mtu = iface.mtu
confParams = misc.getIfaceCfg(iface.name)
if not ipv4.address and ipv4.bootproto != 'dhcp':
ipv4.address = confParams.get('IPADDR')
@@ -651,7 +650,12 @@
mtu = confParams.get('MTU')
if mtu:
mtu = int(mtu)
- return ipv4, ipv6, mtu
+ iface_updated_from_ifcfg = copy.deepcopy(iface)
+ iface_updated_from_ifcfg.ipv4 = ipv4
+ iface_updated_from_ifcfg.ipv6 = ipv6
+ return iface_updated_from_ifcfg
+ else:
+ return iface
def removeNic(self, nic):
cf = NET_CONF_PREF + nic
--
To view, visit https://gerrit.ovirt.org/61471
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I67db38cb7253bff14f86019a52fef3d736a65c1f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda(a)redhat.com>
7 years, 9 months
Change in vdsm[master]: configs: move sudoers to static
by Martin Polednik
Martin Polednik has posted comments on this change.
Change subject: configs: move sudoers to static
......................................................................
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/61603/5/static/Makefile.am
File static/Makefile.am:
PS5, Line 60: chmod
> before we also created sudoers.d dir - please keep the same behavior from h
Agreed on the install; unsure about the dir: it is created somewhere in the process anyway - would love to eventually find out where.
--
To view, visit https://gerrit.ovirt.org/61603
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fd006ff4fdafbe436974f868e461c105ae0dba1
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(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: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 9 months
Change in vdsm[master]: configs: move sudoers to static
by ybronhei＠redhat.com
Yaniv Bronhaim has posted comments on this change.
Change subject: configs: move sudoers to static
......................................................................
Patch Set 5: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/61603/5/static/Makefile.am
File static/Makefile.am:
PS5, Line 60: chmod
> perhaps just `install`?
before we also created sudoers.d dir - please keep the same behavior from https://gerrit.ovirt.org/#/c/61603/5/vdsm/Makefile.am
use $(MKDIR_P) and $(INSTALL_DATA)
--
To view, visit https://gerrit.ovirt.org/61603
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fd006ff4fdafbe436974f868e461c105ae0dba1
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(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: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 9 months
Change in vdsm[master]: configs: move limits to static
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: configs: move limits to static
......................................................................
Patch Set 5: Code-Review-1
another instance of the `cp` vs `mv` question, -1 for visibility. Once this is discussed, I'm fine with this patch.
--
To view, visit https://gerrit.ovirt.org/61605
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd6f78eb2feadaeb73ed688b4c41af5093c928c0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(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: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 9 months
Change in vdsm[master]: configs: move rwtab to static
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: configs: move rwtab to static
......................................................................
Patch Set 5: Code-Review-1
(1 comment)
-1 for visibility, please check inline question. Conceptually ok.
https://gerrit.ovirt.org/#/c/61604/5/static/Makefile.am
File static/Makefile.am:
PS5, Line 68: mv
same question as parent patch: why not `cp` ?
--
To view, visit https://gerrit.ovirt.org/61604
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I44befe2591c42b6c636f2db2b5b13c170af5ce17
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(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: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 9 months
Change in vdsm[master]: configs: move sudoers to static
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: configs: move sudoers to static
......................................................................
Patch Set 5: Code-Review-1
(2 comments)
conceptually fine, a couple of questions about Makefile.am, -1 for visibility
https://gerrit.ovirt.org/#/c/61603/5/static/Makefile.am
File static/Makefile.am:
PS5, Line 58: mv
why mv and not cp?
(Milan already commented this)
PS5, Line 60: chmod
perhaps just `install`?
--
To view, visit https://gerrit.ovirt.org/61603
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fd006ff4fdafbe436974f868e461c105ae0dba1
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(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: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 9 months
Change in vdsm[master]: configs: move mom to static
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: configs: move mom to static
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit https://gerrit.ovirt.org/61602
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibaeffdb167f9f406106efa187ed48e9103648267
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(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: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: No
7 years, 9 months
Change in vdsm[master]: configs: move logger to static
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: configs: move logger to static
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://gerrit.ovirt.org/61601
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7894e7d48047a6cb8b1f2482fd44c89b9260797e
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(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: 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: No
7 years, 9 months
Change in vdsm[master]: ovs: early IP+link setup
by edwardh＠redhat.com
Edward Haas has posted comments on this change.
Change subject: ovs: early IP+link setup
......................................................................
Patch Set 4:
(5 comments)
https://gerrit.ovirt.org/#/c/60371/4/configure.ac
File configure.ac:
PS4, Line 169: OVS_NET_SERVICE
I do not think this needs to change at all, only the OPENVSWITCHSERVICE requirement needs to be placed in the vdsm-network-init service.
https://gerrit.ovirt.org/#/c/60371/4/init/systemd/vdsm-network-ovs-init.s...
File init/systemd/vdsm-network-ovs-init.service.in:
I think that 'ovs' can be dropped from the file name and description.
Currently, internal implementation will cover only ovs typed networks, later on we will probably want to use it for legacy as well.
Line 1: [Unit]
Line 2: Description=Virtual Desktop Server Manager OVS network IP+link restoration
Line 3: Wants=network.target
Line 4: Requires=openvswitch.service
Line 1: [Unit]
Line 2: Description=Virtual Desktop Server Manager OVS network IP+link restoration
Line 3: Wants=network.target
Line 4: Requires=openvswitch.service
Line 5: After=openvswitch.service
We need it Before libvirt as well.
We may want it Before network manager, not sure.
Line 6:
Line 7: [Service]
Line 8: Type=oneshot
Line 9: EnvironmentFile=-/etc/sysconfig/vdsm
https://gerrit.ovirt.org/#/c/60371/4/init/systemd/vdsm-network.service.in
File init/systemd/vdsm-network.service.in:
PS4, Line 4: @OVS_NET_SERVICE@
Original just needs to be moved to vdsm-network-init and here depend (always) on vdsm-network-init
https://gerrit.ovirt.org/#/c/60371/4/lib/vdsm/tool/restore_ovs_nets.py
File lib/vdsm/tool/restore_ovs_nets.py:
PS4, Line 44: _NETS_RESTORED_MARK
Who creates it?
--
To view, visit https://gerrit.ovirt.org/60371
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0555a61c9709be54bfb2587e3020d3046db10ec6
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes
7 years, 9 months