Change in vdsm[master]: mount: Perform mount and umount in supervdsm
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: mount: Perform mount and umount in supervdsm
......................................................................
mount: Perform mount and umount in supervdsm
We used to run the mount and umount commands using sudo, which requires
error prone sudoers configuration, and allows any process running as
vdsm (e.g. a vdsm hook) to perform commands, instead of only vdsmd
daemon itself.
This patch changes Mount class to perform mount and umount using
supervdsm, and remove the now unneeded sudoers configuration for mount,
umount, and systemd-run.
Change-Id: I38fb0eed0ba3e2c36aba8ca4ec262032cb012fc2
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M tests/mountTests.py
M vdsm/storage/mount.py
M vdsm/sudoers.vdsm.in
M vdsm/supervdsmServer
4 files changed, 93 insertions(+), 46 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/43969/1
diff --git a/tests/mountTests.py b/tests/mountTests.py
index 16a98b9..b7b0c34 100644
--- a/tests/mountTests.py
+++ b/tests/mountTests.py
@@ -62,7 +62,21 @@
os.unlink(path)
+class FakeSupervdsm(object):
+
+ def getProxy(self):
+ return self
+
+ def mount(self, *a, **kw):
+ return mount._mount(*a, **kw)
+
+ def umount(self, *a, **kw):
+ return mount._umount(*a, **kw)
+
+
class MountTests(TestCaseBase):
+
+ @monkeypatch.MonkeyPatch(mount, "supervdsm", FakeSupervdsm())
def testLoopMount(self):
checkSudo(["mount", "-o", "loop", "somefile", "target"])
checkSudo(["umount", "target"])
@@ -76,6 +90,7 @@
finally:
m.umount()
+ @monkeypatch.MonkeyPatch(mount, "supervdsm", FakeSupervdsm())
def testSymlinkMount(self):
checkSudo(["mount", "-o", "loop", "somefile", "target"])
checkSudo(["umount", "target"])
diff --git a/vdsm/storage/mount.py b/vdsm/storage/mount.py
index 4786ccb..e579450 100644
--- a/vdsm/storage/mount.py
+++ b/vdsm/storage/mount.py
@@ -27,6 +27,8 @@
from vdsm import cmdutils
from vdsm import constants
+
+import supervdsm
import misc
# Common vfs types
@@ -214,51 +216,14 @@
return hsh
def mount(self, mntOpts=None, vfstype=None, timeout=None, cgroup=None):
- cmd = [constants.EXT_MOUNT]
-
- if vfstype is not None:
- cmd.extend(("-t", vfstype))
-
- if mntOpts:
- cmd.extend(("-o", mntOpts))
-
- cmd.extend((self.fs_spec, self.fs_file))
-
- if cgroup:
- cmd = cmdutils.systemd_run(cmd, scope=True, slice=cgroup)
-
- return self._runcmd(cmd, timeout)
-
- def _runcmd(self, cmd, timeout):
- isRoot = os.geteuid() == 0
- p = misc.execCmd(cmd, sudo=not isRoot, sync=False)
- if not p.wait(timeout):
- p.kill()
- raise OSError(errno.ETIMEDOUT,
- "%s operation timed out" % os.path.basename(cmd[0]))
-
- out, err = p.communicate()
- rc = p.returncode
-
- if rc == 0:
- return
-
- raise MountError(rc, ";".join((out, err)))
+ p = supervdsm.getProxy()
+ return p.mount(self.fs_spec, self.fs_file, mntOpts=mntOpts,
+ vfstype=vfstype, timeout=timeout, cgroup=cgroup)
def umount(self, force=False, lazy=False, freeloop=False, timeout=None):
- cmd = [constants.EXT_UMOUNT]
- if force:
- cmd.append("-f")
-
- if lazy:
- cmd.append("-l")
-
- if freeloop:
- cmd.append("-d")
-
- cmd.append(self.fs_file)
-
- return self._runcmd(cmd, timeout)
+ p = supervdsm.getProxy()
+ return p.umount(self.fs_file, force=force, lazy=lazy,
+ freeloop=freeloop, timeout=timeout)
def isMounted(self):
try:
@@ -287,3 +252,58 @@
def __repr__(self):
return ("<Mount fs_spec='%s' fs_file='%s'>" %
(self.fs_spec, self.fs_file))
+
+
+def _mount(fs_spec, fs_file, mntOpts=None, vfstype=None, timeout=None, cgroup=None):
+ """
+ Called from supervdsm for running the mount command as root.
+ """
+ cmd = [constants.EXT_MOUNT]
+
+ if vfstype is not None:
+ cmd.extend(("-t", vfstype))
+
+ if mntOpts:
+ cmd.extend(("-o", mntOpts))
+
+ cmd.extend((fs_spec, fs_file))
+
+ if cgroup:
+ cmd = cmdutils.systemd_run(cmd, scope=True, slice=cgroup)
+
+ return _runcmd(cmd, timeout)
+
+
+def _umount(fs_file, force=False, lazy=False, freeloop=False, timeout=None):
+ """
+ Called from supervdsm for running the umount command as root.
+ """
+ cmd = [constants.EXT_UMOUNT]
+ if force:
+ cmd.append("-f")
+
+ if lazy:
+ cmd.append("-l")
+
+ if freeloop:
+ cmd.append("-d")
+
+ cmd.append(fs_file)
+
+ return _runcmd(cmd, timeout)
+
+
+def _runcmd(cmd, timeout):
+ p = misc.execCmd(cmd, sync=False)
+ if not p.wait(timeout):
+ p.kill()
+ raise OSError(errno.ETIMEDOUT,
+ "%s operation timed out" % os.path.basename(cmd[0]))
+
+ out, err = p.communicate()
+ rc = p.returncode
+
+ if rc == 0:
+ return
+
+ raise MountError(rc, ";".join((out, err)))
diff --git a/vdsm/sudoers.vdsm.in b/vdsm/sudoers.vdsm.in
index 924cc87..76b2386 100644
--- a/vdsm/sudoers.vdsm.in
+++ b/vdsm/sudoers.vdsm.in
@@ -1,7 +1,7 @@
Cmnd_Alias VDSM_LIFECYCLE = \
@DMIDECODE_PATH@, \
@VDSMDIR@/mk_sysprep_floppy
-Cmnd_Alias VDSM_STORAGE = @MOUNT_PATH@, @UMOUNT_PATH@, \
+Cmnd_Alias VDSM_STORAGE = \
@FSCK_PATH@ -p *, \
@TUNE2FS_PATH@ -j *, \
@MKFS_PATH@ -q -j *, \
@@ -27,8 +27,7 @@
@MULTIPATH_PATH@, \
@SETSID_PATH@ @IONICE_PATH@ -c ? -n ? @SU_PATH@ vdsm -s /bin/sh -c /usr/libexec/vdsm/spmprotect.sh*, \
@SERVICE_PATH@ vdsmd *, \
- @REBOOT_PATH@ -f, \
- @SYSTEMD_RUN_PATH@ --scope --slice=vdsm-glusterfs @MOUNT_PATH@ *
+ @REBOOT_PATH@ -f
vdsm ALL=(ALL) NOPASSWD: VDSM_LIFECYCLE, VDSM_STORAGE
Defaults:vdsm !requiretty
diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer
index 119c2ba..4b728aa 100755
--- a/vdsm/supervdsmServer
+++ b/vdsm/supervdsmServer
@@ -70,6 +70,7 @@
from storage.iscsi import readSessionInfo as _readSessionInfo
from supervdsm import _SuperVdsmManager
from storage import hba
+from storage import mount
from storage import multipath
from storage.fileUtils import chown, resolveGid, resolveUid
from storage.fileUtils import validateAccess as _validateAccess
@@ -150,6 +151,18 @@
return _getScsiSerial(*args, **kwargs)
@logDecorator
+ def mount(self, fs_spec, fs_file, mntOpts=None, vfstype=None, timeout=None,
+ cgroup=None):
+ return mount._mount(fs_spec, fs_file, mntOpts=mntOpts, vfstype=vfstype,
+ timeout=timeout, cgroup=cgroup)
+
+ @logDecorator
+ def umount(self, fs_file, force=False, lazy=False, freeloop=False,
+ timeout=None):
+ return mount._umount(fs_file, force=force, lazy=lazy,
+ freeloop=freeloop, timeout=timeout)
+
+ @logDecorator
def resizeMap(self, devName):
return multipath._resize_map(devName)
--
To view, visit https://gerrit.ovirt.org/43969
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I38fb0eed0ba3e2c36aba8ca4ec262032cb012fc2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
8 years, 7 months
Change in vdsm[master]: volume: fix failing metadata parsing
by ishaby＠redhat.com
Idan Shaby has uploaded a new change for review.
Change subject: volume: fix failing metadata parsing
......................................................................
volume: fix failing metadata parsing
The call to "split" in file and block volumes expects only one "=" per
line. This is not a good behavior since there might be fields that
contain some more "=" (like Description).
This patch limits the number of splits to 1, so that the fields values
can contains some other "=".
Change-Id: Ie84bf5e42d2c4949944a5f348615d6fe80f72fa2
Bug-Url: https://bugzilla.redhat.com/1258835
Signed-off-by: Idan Shaby <ishaby(a)redhat.com>
---
M vdsm/storage/blockVolume.py
M vdsm/storage/fileVolume.py
2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/85/45585/1
diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py
index 9ab5ca4..ac29f40 100644
--- a/vdsm/storage/blockVolume.py
+++ b/vdsm/storage/blockVolume.py
@@ -595,7 +595,7 @@
return out
if l.find("=") < 0:
continue
- key, value = l.split("=")
+ key, value = l.split("=", 1)
out[key.strip()] = value.strip()
except Exception as e:
diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index 20573bd..1259472 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -300,7 +300,7 @@
return out
if l.find("=") < 0:
continue
- key, value = l.split("=")
+ key, value = l.split("=", 1)
out[key.strip()] = value.strip()
except Exception as e:
--
To view, visit https://gerrit.ovirt.org/45585
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie84bf5e42d2c4949944a5f348615d6fe80f72fa2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby(a)redhat.com>
8 years, 7 months
Change in vdsm[ovirt-3.6]: jsonrpc: fd leak
by Piotr Kliczewski
Piotr Kliczewski has uploaded a new change for review.
Change subject: jsonrpc: fd leak
......................................................................
jsonrpc: fd leak
We need to make sure that there is no reference to objects which holds
a socket because m2c closes socket only when its object is garbage
collected.
This patch fixes two places where we had direct or indirect references
to socket object. In _StompConnection we kept a reference to it but now
we use dispatcher.socket. We passed connection object to JsonRpcServer
in order to get get local address which slowed down collection of socket
object (different thread) which could lead to reaching fd limit allowed
for the process. We removed this issue by passing local address to the
server instead.
Bug-Url: https://bugzilla.redhat.com/1256446
Change-Id: Ib15cca4659553b320babf928b603a0a75013ba99
Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com>
---
M lib/yajsonrpc/__init__.py
M lib/yajsonrpc/stompreactor.py
2 files changed, 14 insertions(+), 15 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/14/45714/1
diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py
index 76d97b8..03c49ed 100644
--- a/lib/yajsonrpc/__init__.py
+++ b/lib/yajsonrpc/__init__.py
@@ -245,10 +245,10 @@
class _JsonRpcServeRequestContext(object):
- def __init__(self, client, conn):
+ def __init__(self, client, addr):
self._requests = []
self._client = client
- self._conn = conn
+ self._addr = addr
self._counter = 0
self._requests = {}
self._responses = []
@@ -266,8 +266,8 @@
return self._counter
@property
- def connection(self):
- return self._conn
+ def address(self):
+ return self._addr
def sendReply(self):
if len(self._requests) > 0:
@@ -514,8 +514,7 @@
try:
params = req.params
- server_address = ctx.connection.get_local_address()
- self._bridge.register_server_address(server_address)
+ self._bridge.register_server_address(ctx.address)
if isinstance(req.params, list):
res = method(*params)
else:
@@ -541,11 +540,11 @@
if obj is None:
break
- client, conn, msg = obj
- self._parseMessage(client, conn, msg)
+ client, addr, msg = obj
+ self._parseMessage(client, addr, msg)
- def _parseMessage(self, client, conn, msg):
- ctx = _JsonRpcServeRequestContext(client, conn)
+ def _parseMessage(self, client, addr, msg):
+ ctx = _JsonRpcServeRequestContext(client, addr)
try:
rawRequests = json.loads(msg)
diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py
index 92d1dca..87cab3f 100644
--- a/lib/yajsonrpc/stompreactor.py
+++ b/lib/yajsonrpc/stompreactor.py
@@ -243,14 +243,13 @@
class _StompConnection(object):
def __init__(self, server, aclient, sock, reactor):
- self._socket = sock
self._reactor = reactor
self._server = server
self._messageHandler = None
self._async_client = aclient
- adisp = self._adisp = stomp.AsyncDispatcher(self, aclient)
- self._dispatcher = Dispatcher(adisp, sock=sock, map=reactor._map)
+ self._dispatcher = reactor.create_dispatcher(
+ sock, stomp.AsyncDispatcher(self, aclient))
def send_raw(self, msg):
self._async_client.queue_frame(msg)
@@ -268,7 +267,7 @@
self._async_client.remove_subscriptions()
def get_local_address(self):
- return self._socket.getsockname()[0]
+ return self._dispatcher.socket.getsockname()[0]
def set_message_handler(self, msgHandler):
self._messageHandler = msgHandler
@@ -276,7 +275,8 @@
def handleMessage(self, data):
if self._messageHandler is not None:
- self._messageHandler((self._server, self, data))
+ self._messageHandler((self._server, self.get_local_address(),
+ data))
def is_closed(self):
return not self._dispatcher.connected
--
To view, visit https://gerrit.ovirt.org/45714
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib15cca4659553b320babf928b603a0a75013ba99
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
8 years, 7 months
Change in vdsm[master]: jsonrpc: fd leak
by Piotr Kliczewski
Piotr Kliczewski has uploaded a new change for review.
Change subject: jsonrpc: fd leak
......................................................................
jsonrpc: fd leak
We need to make sure that there is no reference to objects which holds
a socket because m2c closes socket when its object is garbage collected.
Bug-Url: https://bugzilla.redhat.com/1256446
Change-Id: Ib15cca4659553b320babf928b603a0a75013ba99
Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com>
---
M lib/yajsonrpc/__init__.py
M lib/yajsonrpc/stompreactor.py
2 files changed, 17 insertions(+), 15 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/45615/1
diff --git a/lib/yajsonrpc/__init__.py b/lib/yajsonrpc/__init__.py
index 76d97b8..03c49ed 100644
--- a/lib/yajsonrpc/__init__.py
+++ b/lib/yajsonrpc/__init__.py
@@ -245,10 +245,10 @@
class _JsonRpcServeRequestContext(object):
- def __init__(self, client, conn):
+ def __init__(self, client, addr):
self._requests = []
self._client = client
- self._conn = conn
+ self._addr = addr
self._counter = 0
self._requests = {}
self._responses = []
@@ -266,8 +266,8 @@
return self._counter
@property
- def connection(self):
- return self._conn
+ def address(self):
+ return self._addr
def sendReply(self):
if len(self._requests) > 0:
@@ -514,8 +514,7 @@
try:
params = req.params
- server_address = ctx.connection.get_local_address()
- self._bridge.register_server_address(server_address)
+ self._bridge.register_server_address(ctx.address)
if isinstance(req.params, list):
res = method(*params)
else:
@@ -541,11 +540,11 @@
if obj is None:
break
- client, conn, msg = obj
- self._parseMessage(client, conn, msg)
+ client, addr, msg = obj
+ self._parseMessage(client, addr, msg)
- def _parseMessage(self, client, conn, msg):
- ctx = _JsonRpcServeRequestContext(client, conn)
+ def _parseMessage(self, client, addr, msg):
+ ctx = _JsonRpcServeRequestContext(client, addr)
try:
rawRequests = json.loads(msg)
diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py
index fa6994b..e2869df 100644
--- a/lib/yajsonrpc/stompreactor.py
+++ b/lib/yajsonrpc/stompreactor.py
@@ -243,14 +243,13 @@
class _StompConnection(object):
def __init__(self, server, aclient, sock, reactor):
- self._socket = sock
self._reactor = reactor
self._server = server
self._messageHandler = None
self._async_client = aclient
- adisp = self._adisp = stomp.AsyncDispatcher(self, aclient)
- self._dispatcher = Dispatcher(adisp, sock=sock, map=reactor._map)
+ self._dispatcher = reactor.create_dispatcher(
+ sock, stomp.AsyncDispatcher(self, aclient))
def send_raw(self, msg):
self._async_client.queue_frame(msg)
@@ -268,7 +267,7 @@
self._async_client.remove_subscriptions()
def get_local_address(self):
- return self._socket.getsockname()[0]
+ return self._dispatcher.socket.getsockname()[0]
def set_message_handler(self, msgHandler):
self._messageHandler = msgHandler
@@ -276,11 +275,15 @@
def handleMessage(self, data):
if self._messageHandler is not None:
- self._messageHandler((self._server, self, data))
+ self._messageHandler((self._server, self.get_local_address(),
+ data))
def is_closed(self):
return not self._dispatcher.connected
+ def unsubscribe(self, sub):
+ pass
+
class StompServer(object):
log = logging.getLogger("yajsonrpc.StompServer")
--
To view, visit https://gerrit.ovirt.org/45615
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib15cca4659553b320babf928b603a0a75013ba99
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
8 years, 7 months
Change in vdsm[master]: network: test: test if bonding is successfully removed
by phoracek＠redhat.com
Petr Horáček has uploaded a new change for review.
Change subject: network: test: test if bonding is successfully removed
......................................................................
network: test: test if bonding is successfully removed
Change-Id: I2fd6052676c20904e8f104616dc4e25d4616f71e
Signed-off-by: Petr Horáček <phoracek(a)redhat.com>
---
M tests/functional/networkTests.py
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/82/37882/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index aac1f01..55b6f3f 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -480,6 +480,7 @@
self.assertNetworkDoesntExist(vlan_net)
self.assertVlanDoesntExist(BONDING_NAME + '.' +
networks[vlan_net]['vlan'])
+ self.assertBondDoesntExist(BONDING_NAME, nics)
@cleanupNet
@permutations([[True], [False]])
@@ -498,6 +499,7 @@
{BONDING_NAME: {'remove': True}}, NOCHK)
self.assertEqual(status, SUCCESS, msg)
self.assertNetworkDoesntExist(NETWORK_NAME)
+ self.assertBondDoesntExist(BONDING_NAME, nics)
@cleanupNet
@permutations([[True], [False]])
@@ -532,6 +534,7 @@
{},
{BONDING_NAME: {'remove': True}}, NOCHK)
self.assertEqual(status, SUCCESS, msg)
+ self.assertBondDoesntExist(BONDING_NAME, nics)
@cleanupNet
def testSetupNetworksDelOneOfBondNets(self):
@@ -568,6 +571,7 @@
{NETA_NAME: {'remove': True}},
{BONDING_NAME: {'remove': True}}, NOCHK)
self.assertEqual(status, SUCCESS, msg)
+ self.assertBondDoesntExist(BONDING_NAME, nics)
@cleanupNet
@permutations([[True], [False]])
@@ -2263,3 +2267,4 @@
status, msg = self.vdsm_net.setupNetworks(
{}, {BONDING_NAME: {'remove': True}}, NOCHK)
self.assertEqual(status, SUCCESS, msg)
+ self.assertBondDoesntExist(BONDING_NAME, nics)
--
To view, visit http://gerrit.ovirt.org/37882
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2fd6052676c20904e8f104616dc4e25d4616f71e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <phoracek(a)redhat.com>
8 years, 7 months
Change in vdsm[master]: stomp: broken unsubscribe process
by Piotr Kliczewski
Piotr Kliczewski has uploaded a new change for review.
Change subject: stomp: broken unsubscribe process
......................................................................
stomp: broken unsubscribe process
There is a logic common for incoming and outgoing connection which
cleans the subscription. In case on incoming connection the code is
broken due missing unsubscribe method on _StompConnection.
Change-Id: I45c5741f4d94b7af138e98661cfe418cbb0a2b6b
Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com>
---
M lib/yajsonrpc/stompreactor.py
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/45617/1
diff --git a/lib/yajsonrpc/stompreactor.py b/lib/yajsonrpc/stompreactor.py
index fa6994b..59bdcc0 100644
--- a/lib/yajsonrpc/stompreactor.py
+++ b/lib/yajsonrpc/stompreactor.py
@@ -281,6 +281,9 @@
def is_closed(self):
return not self._dispatcher.connected
+ def unsubscribe(self, sub):
+ pass
+
class StompServer(object):
log = logging.getLogger("yajsonrpc.StompServer")
--
To view, visit https://gerrit.ovirt.org/45617
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I45c5741f4d94b7af138e98661cfe418cbb0a2b6b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
8 years, 7 months
Change in vdsm[master]: mount: Log info message when mounting or unmounting
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: mount: Log info message when mounting or unmounting
......................................................................
mount: Log info message when mounting or unmounting
Mounting and unmounting are important modifications to the machine state
and must be logged. Since actual mount command run now in supervdsm, we
have no clue about mounting and unmounting operations in vdsm log.
To help debugging issues with unresponsive mounts, we log the time it
took to mount or unmount.
Change-Id: Idc89d6a1bfa0e9d117344d8870d81fed140d03c6
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M vdsm/storage/mount.py
1 file changed, 21 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/43974/1
diff --git a/vdsm/storage/mount.py b/vdsm/storage/mount.py
index 1da15c6..b4e069d 100644
--- a/vdsm/storage/mount.py
+++ b/vdsm/storage/mount.py
@@ -19,6 +19,7 @@
#
from collections import namedtuple
import errno
+import logging
from os.path import normpath
import re
import os
@@ -27,6 +28,7 @@
from vdsm import cmdutils
from vdsm import constants
+from vdsm import utils
import supervdsm
import misc
@@ -195,6 +197,9 @@
class Mount(object):
+
+ log = logging.getLogger("storage.Mount")
+
def __init__(self, fs_spec, fs_file):
self.fs_spec = normpath(fs_spec)
self.fs_file = normpath(fs_file)
@@ -216,14 +221,24 @@
return hsh
def mount(self, mntOpts=None, vfstype=None, timeout=None, cgroup=None):
- mount = supervdsm.getProxy().mount if os.geteuid() != 0 else _mount
- mount(self.fs_spec, self.fs_file, mntOpts=mntOpts, vfstype=vfstype,
- timeout=timeout, cgroup=cgroup)
+ self.log.info("mounting %s at %s", self.fs_spec, self.fs_file)
+ with utils.stopwatch("%s mounted" % self.fs_file, log=self.log):
+ if os.geteuid() != 0:
+ mount = supervdsm.getProxy().mount
+ else:
+ mount = _mount
+ mount(self.fs_spec, self.fs_file, mntOpts=mntOpts, vfstype=vfstype,
+ timeout=timeout, cgroup=cgroup)
def umount(self, force=False, lazy=False, freeloop=False, timeout=None):
- umount = supervdsm.getProxy().umount if os.geteuid() != 0 else _umount
- umount(self.fs_file, force=force, lazy=lazy, freeloop=freeloop,
- timeout=timeout)
+ self.log.info("unmounting %s", self.fs_file)
+ with utils.stopwatch("%s unmounted" % self.fs_file, log=self.log):
+ if os.geteuid() != 0:
+ umount = supervdsm.getProxy().umount
+ else:
+ umount = _umount
+ umount(self.fs_file, force=force, lazy=lazy, freeloop=freeloop,
+ timeout=timeout)
def isMounted(self):
try:
--
To view, visit https://gerrit.ovirt.org/43974
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idc89d6a1bfa0e9d117344d8870d81fed140d03c6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
8 years, 7 months
Change in vdsm[master]: mount: Remove unneeded return
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: mount: Remove unneeded return
......................................................................
mount: Remove unneeded return
Remove unneeded return in mount._mount() and mount._umount() since
mount._runcmd() does not return any value.
Change-Id: I594c733b9c6230bf225bdc425b2ee58972003600
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M vdsm/storage/mount.py
M vdsm/supervdsmServer
2 files changed, 10 insertions(+), 10 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/73/43973/1
diff --git a/vdsm/storage/mount.py b/vdsm/storage/mount.py
index a09199d..1da15c6 100644
--- a/vdsm/storage/mount.py
+++ b/vdsm/storage/mount.py
@@ -217,13 +217,13 @@
def mount(self, mntOpts=None, vfstype=None, timeout=None, cgroup=None):
mount = supervdsm.getProxy().mount if os.geteuid() != 0 else _mount
- return mount(self.fs_spec, self.fs_file, mntOpts=mntOpts,
- vfstype=vfstype, timeout=timeout, cgroup=cgroup)
+ mount(self.fs_spec, self.fs_file, mntOpts=mntOpts, vfstype=vfstype,
+ timeout=timeout, cgroup=cgroup)
def umount(self, force=False, lazy=False, freeloop=False, timeout=None):
umount = supervdsm.getProxy().umount if os.geteuid() != 0 else _umount
- return umount(self.fs_file, force=force, lazy=lazy, freeloop=freeloop,
- timeout=timeout)
+ umount(self.fs_file, force=force, lazy=lazy, freeloop=freeloop,
+ timeout=timeout)
def isMounted(self):
try:
@@ -271,7 +271,7 @@
if cgroup:
cmd = cmdutils.systemd_run(cmd, scope=True, slice=cgroup)
- return _runcmd(cmd, timeout)
+ _runcmd(cmd, timeout)
def _umount(fs_file, force=False, lazy=False, freeloop=False, timeout=None):
@@ -290,7 +290,7 @@
cmd.append(fs_file)
- return _runcmd(cmd, timeout)
+ _runcmd(cmd, timeout)
def _runcmd(cmd, timeout):
diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer
index 4b728aa..be3f3b4 100755
--- a/vdsm/supervdsmServer
+++ b/vdsm/supervdsmServer
@@ -153,14 +153,14 @@
@logDecorator
def mount(self, fs_spec, fs_file, mntOpts=None, vfstype=None, timeout=None,
cgroup=None):
- return mount._mount(fs_spec, fs_file, mntOpts=mntOpts, vfstype=vfstype,
- timeout=timeout, cgroup=cgroup)
+ mount._mount(fs_spec, fs_file, mntOpts=mntOpts, vfstype=vfstype,
+ timeout=timeout, cgroup=cgroup)
@logDecorator
def umount(self, fs_file, force=False, lazy=False, freeloop=False,
timeout=None):
- return mount._umount(fs_file, force=force, lazy=lazy,
- freeloop=freeloop, timeout=timeout)
+ mount._umount(fs_file, force=force, lazy=lazy, freeloop=freeloop,
+ timeout=timeout)
@logDecorator
def resizeMap(self, devName):
--
To view, visit https://gerrit.ovirt.org/43973
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I594c733b9c6230bf225bdc425b2ee58972003600
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
8 years, 7 months
Change in vdsm[master]: VolumeMetadata: Introduce class structure
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: VolumeMetadata: Introduce class structure
......................................................................
Patch Set 17: Code-Review-1
(1 comment)
Partial review
https://gerrit.ovirt.org/#/c/41844/17/vdsm/storage/blockVolume.py
File vdsm/storage/blockVolume.py:
Line 113:
Line 114: class BlockVolume(volume.Volume):
Line 115: """ Actually represents a single volume (i.e. part of virtual disk).
Line 116: """
Line 117: MetadataClass = BlockVolumeMetadata
metadataClass
Line 118:
Line 119: def __init__(self, repoPath, sdUUID, imgUUID, volUUID):
Line 120: md = self.MetadataClass(repoPath, sdUUID, imgUUID, volUUID)
Line 121: self.metaoff = None
--
To view, visit https://gerrit.ovirt.org/41844
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0552bea23b04b9e58e5d2cc7e016103d03194d1a
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Ala Hino <ahino(a)redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 7 months
Change in vdsm[master]: StorageDomainManifest: Move clusterLock into Manifest
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: StorageDomainManifest: Move clusterLock into Manifest
......................................................................
Patch Set 12: Code-Review-1
(8 comments)
Generally ok, added few questions and minor cleanups.
https://gerrit.ovirt.org/#/c/43549/12/tests/manifest_tests.py
File tests/manifest_tests.py:
Line 64: self.assertEquals(mdpath, manifest.getMDPath())
Line 65:
Line 66: def test_getmetaparam(self):
Line 67: with namedTemporaryDir() as tmpdir:
Line 68: metadata = {sd.DMDK_VERSION: 3}
Why not use FakeMetadata object for consistency? (for another patch?)
Line 69: manifest = make_filesd_manifest(tmpdir, metadata)
Line 70: metadata[sd.DMDK_SDUUID] = manifest.sdUUID
Line 71: self.assertEquals(manifest.sdUUID,
Line 72: manifest.getMetaParam(sd.DMDK_SDUUID))
https://gerrit.ovirt.org/#/c/43549/12/tests/storagetestlib.py
File tests/storagetestlib.py:
Line 38: def make_blocksd_manifest(tmpdir=None, metadata=None, sduuid=None):
Line 39: if sduuid is None:
Line 40: sduuid = str(uuid.uuid4())
Line 41: if metadata is None:
Line 42: metadata = FakeMetadata([(sd.DMDK_VERSION, 3)])
You can use:
FakeMetadata({sd.DMDK: 3})
Line 43: manifest = blockSD.BlockStorageDomainManifest(sduuid, metadata)
Line 44: if tmpdir is not None:
Line 45: manifest.domaindir = tmpdir
Line 46: os.makedirs(os.path.join(manifest.domaindir, sduuid, sd.DOMAIN_IMAGES))
Line 77: sduuid = str(uuid.uuid4())
Line 78: domain_path = os.path.join(tmpdir, sduuid)
Line 79: make_file(get_metafile_path(domain_path))
Line 80: if metadata is None:
Line 81: metadata = FakeMetadata([(sd.DMDK_VERSION, 3)])
You can use:
FakeMetadata({sd.DMDK: 3})
Line 82: manifest = fileSD.FileStorageDomainManifest(domain_path, metadata)
Line 83: os.makedirs(os.path.join(manifest.domaindir, sduuid, sd.DOMAIN_IMAGES))
Line 84: return manifest
Line 85:
https://gerrit.ovirt.org/#/c/43549/12/vdsm/storage/imageRepository/format...
File vdsm/storage/imageRepository/formatConverter.py:
Line 117: log.debug("Setting permissions for domain %s", domain.sdUUID)
Line 118: domain.setMetadataPermissions()
Line 119:
Line 120: log.debug("Initializing the new cluster lock for domain %s", domain.sdUUID)
Line 121: newClusterLock = domain._manifest._makeDomainLock(targetVersion)
Why no not use domain._makeClusterLock()?
Line 122: newClusterLock.initLock()
Line 123:
Line 124: log.debug("Acquiring the host id %s for domain %s", hostId, domain.sdUUID)
Line 125: newClusterLock.acquireHostId(hostId, async=False)
https://gerrit.ovirt.org/#/c/43549/12/vdsm/storage/sd.py
File vdsm/storage/sd.py:
Line 555
Line 556
Line 557
Line 558
Line 559
Lets move it to the manifest, so we don't have to make the manifest domainLock public. In the menifest, this should be called initDomainLock()
Line 304: def __init__(self, sdUUID, domaindir, metadata):
Line 305: self.sdUUID = sdUUID
Line 306: self.domaindir = domaindir
Line 307: self.replaceMetadata(metadata)
Line 308: self.domainLock = self._makeDomainLock()
Why public? old lock was private.
Line 309:
Line 310: @property
Line 311: def oop(self):
Line 312: return oop.getProcessPool(self.sdUUID)
Line 428: DEFAULT_LEASE_PARAMS[DMDK_LOCK_RENEWAL_INTERVAL_SEC],
Line 429: DEFAULT_LEASE_PARAMS[DMDK_LEASE_TIME_SEC],
Line 430: DEFAULT_LEASE_PARAMS[DMDK_LEASE_RETRIES],
Line 431: DEFAULT_LEASE_PARAMS[DMDK_IO_OP_TIMEOUT_SEC],
Line 432: )
For another patch, move leaseParams after lock class selection.
Line 433:
Line 434: try:
Line 435: clusterLockClass = self._domainLockTable[domVersion]
Line 436: except KeyError:
Line 431: DEFAULT_LEASE_PARAMS[DMDK_IO_OP_TIMEOUT_SEC],
Line 432: )
Line 433:
Line 434: try:
Line 435: clusterLockClass = self._domainLockTable[domVersion]
If you renamed _makeClusterLock to _makeDomainLock, you should also rename the class name to domainLockClass, or just lockClass.
Line 436: except KeyError:
Line 437: raise se.UnsupportedDomainVersion(domVersion)
Line 438:
Line 439: return clusterLockClass(self.sdUUID, self.getIdsFilePath(),
--
To view, visit https://gerrit.ovirt.org/43549
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2c019a7fed6ba793d2b3a2459af221b1abbff2a3
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 7 months