Change in vdsm[master]: logging: Improve logging when receiving signals
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: logging: Improve logging when receiving signals
......................................................................
logging: Improve logging when receiving signals
Log INFO message when receiving signals, explaining the action taken.
Change-Id: I1462d296ca99e8eb2eee8640648db0eea039a83c
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M vdsm/vdsm
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/61289/1
diff --git a/vdsm/vdsm b/vdsm/vdsm
index a6dc727..400f542 100755
--- a/vdsm/vdsm
+++ b/vdsm/vdsm
@@ -70,12 +70,12 @@
running = [True]
def sigtermHandler(signum, frame):
- log.debug("Received signal %s" % signum)
+ log.info("Received signal %s, shutting down" % signum)
running[0] = False
def sigusr1Handler(signum, frame):
if irs:
- log.debug("Received signal %s" % signum)
+ log.info("Received signal %s, stopping SPM" % signum)
irs.spmStop(
irs.getConnectedStoragePoolsList()['poollist'][0])
--
To view, visit https://gerrit.ovirt.org/61289
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1462d296ca99e8eb2eee8640648db0eea039a83c
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]: debug: Add virt-alignment-scan.debug tool
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: debug: Add virt-alignment-scan.debug tool
......................................................................
debug: Add virt-alignment-scan.debug tool
Change-Id: I2ec4d0219ea039d57ee8809339a7e03d71980bf8
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
A vdsm/virt-alignment-scan.debug
1 file changed, 24 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/96/40896/1
diff --git a/vdsm/virt-alignment-scan.debug b/vdsm/virt-alignment-scan.debug
new file mode 100755
index 0000000..e904b45
--- /dev/null
+++ b/vdsm/virt-alignment-scan.debug
@@ -0,0 +1,24 @@
+#!/bin/sh
+#
+# Debug tool for virt-alignmnet-scan
+#
+# Installation:
+#
+# mv /usr/bin/virt-alignment-scan /usr/bin/virt-alignment-scan.real
+# ln -s /usr/share/vdsm/virt-alignment-scan.debug /usr/bin/virt-alignment-scan
+#
+# Uinstall
+#
+# mv /usr/bin/virt-alignment-scan.real /usr/bin/virt-alignment-scan
+
+log="/var/log/vdsm/virt-alignmnet-scan.debug.log"
+
+/usr/bin/virt-alignment-scan "$@"
+rc=$?
+if [ $rc -ne 0 ]; then
+ # See https://bugzilla.redhat.com/show_bug.cgi?id=1151838#c28
+ sleep 30
+ /usr/bin/libguestfs-test-tool >> $log 2>&1
+fi
+
+exit $rc
--
To view, visit https://gerrit.ovirt.org/40896
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2ec4d0219ea039d57ee8809339a7e03d71980bf8
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]: sdm: Use guarded.context in copy_data
by alitke@redhat.com
Adam Litke has uploaded a new change for review.
Change subject: sdm: Use guarded.context in copy_data
......................................................................
sdm: Use guarded.context in copy_data
Change-Id: I5c2a9527f5e3787069e0847e1bf775a60321b306
Signed-off-by: Adam Litke <alitke(a)redhat.com>
---
M tests/storage_sdm_copy_data_test.py
M vdsm/storage/sdm/api/copy_data.py
2 files changed, 71 insertions(+), 20 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/61693/1
diff --git a/tests/storage_sdm_copy_data_test.py b/tests/storage_sdm_copy_data_test.py
index def8934..45c5540 100644
--- a/tests/storage_sdm_copy_data_test.py
+++ b/tests/storage_sdm_copy_data_test.py
@@ -33,8 +33,11 @@
from vdsm import jobs
from vdsm import qemuimg
from vdsm.storage import constants as sc
+from vdsm.storage import guarded
from storage import blockVolume
+from storage import resourceManager as rm
+from storage import sd
import storage.sdm.api.copy_data
@@ -49,6 +52,7 @@
rm = FakeResourceManager()
with MonkeyPatchScope([
(storage.sdm.api.copy_data, 'sdCache', env.sdcache),
+ (guarded, 'rmanager', rm),
(blockVolume, 'rmanager', rm),
]):
src_vols = make_qemu_chain(env, self.SIZE, src_fmt,
@@ -68,6 +72,40 @@
qemuimg.create(vol.volumePath, size=self.SIZE,
format=qemuimg.FORMAT.QCOW2, backing=backing)
return vol
+
+ def check_locks(self, storage_type, src, dst):
+ src_img_ns = sd.getNamespace(sc.IMAGE_NAMESPACE, src.sdUUID)
+ dst_img_ns = sd.getNamespace(sc.IMAGE_NAMESPACE, dst.sdUUID)
+ locks = sorted({
+ guarded.ResourceManagerLock(sc.STORAGE, src.sdUUID,
+ rm.LockType.shared),
+ guarded.ResourceManagerLock(sc.STORAGE, dst.sdUUID,
+ rm.LockType.shared),
+ guarded.ResourceManagerLock(src_img_ns, src.imgUUID,
+ rm.LockType.shared),
+ guarded.ResourceManagerLock(dst_img_ns, dst.imgUUID,
+ rm.LockType.exclusive),
+ })
+ if storage_type == 'block':
+ # NOTE: LVM locks are taken implicitly as part of block volume
+ # prepare. As a result they are acquired in the order that the
+ # prepare is called (src -> dst). This probably should be changed
+ # but we'd have to add a bulk prepare function that uses guarded.
+ locks.extend([
+ guarded.ResourceManagerLock(
+ sd.getNamespace(sc.LVM_ACTIVATION_NAMESPACE, src.sdUUID),
+ src.volUUID, rm.LockType.shared),
+ guarded.ResourceManagerLock(
+ sd.getNamespace(sc.LVM_ACTIVATION_NAMESPACE, dst.sdUUID),
+ dst.volUUID, rm.LockType.exclusive),
+ ])
+
+ expected_acquires = [('acquireResource', (l.ns, l.name, l.mode), {})
+ for l in locks]
+ expected_releases = [('releaseResource', (l.ns, l.name), {})
+ for l in reversed(locks)]
+ calls = guarded.rmanager.__calls__
+ self.assertEqual(expected_acquires + expected_releases, calls)
@permutations((
('file', 'raw', 'raw'),
@@ -106,6 +144,7 @@
verify_qemu_chain(dst_chain)
self.assertEqual(sc.fmt2str(dst_fmt),
qemuimg.info(dst_vol.volumePath)['format'])
+ self.check_locks(env_type, src_vol, dst_vol)
@permutations((
('file', 'raw', 'raw', (0, 1)),
diff --git a/vdsm/storage/sdm/api/copy_data.py b/vdsm/storage/sdm/api/copy_data.py
index aff0185..b9dd555 100644
--- a/vdsm/storage/sdm/api/copy_data.py
+++ b/vdsm/storage/sdm/api/copy_data.py
@@ -26,7 +26,10 @@
from vdsm import properties
from vdsm import qemuimg
from vdsm.storage import constants as sc
+from vdsm.storage import guarded
+from storage import resourceManager as rm
+from storage import sd
from storage import volume
from storage.sdc import sdCache
@@ -42,8 +45,8 @@
def __init__(self, job_id, host_id, source, destination):
super(Job, self).__init__(job_id, 'copy_data', host_id)
- self._source = _create_endpoint(source)
- self._dest = _create_endpoint(destination)
+ self._source = _create_endpoint(source, writable=False)
+ self._dest = _create_endpoint(destination, writable=True)
self._operation = None
@property
@@ -55,26 +58,26 @@
self._operation.abort()
def _run(self):
- # TODO: LOCKING!
- with self._source.prepare(), self._dest.prepare(writable=True):
- # Do not start copying if we have already been aborted
- if self._status == jobs.STATUS.ABORTED:
- return
+ with guarded.context(self._source, self._dest):
+ with self._source.prepare(), self._dest.prepare():
+ # Do not start copying if we have already been aborted
+ if self._status == jobs.STATUS.ABORTED:
+ return
- self._operation = qemuimg.convert(
- self._source.path,
- self._dest.path,
- srcFormat=self._source.qemu_format,
- dstFormat=self._dest.qemu_format,
- backing=self._dest.backing_path,
- backingFormat=self._dest.backing_qemu_format)
- self._operation.wait_for_completion()
+ self._operation = qemuimg.convert(
+ self._source.path,
+ self._dest.path,
+ srcFormat=self._source.qemu_format,
+ dstFormat=self._dest.qemu_format,
+ backing=self._dest.backing_path,
+ backingFormat=self._dest.backing_qemu_format)
+ self._operation.wait_for_completion()
-def _create_endpoint(params):
+def _create_endpoint(params, writable):
endpoint_type = params.pop('endpoint_type')
if endpoint_type == 'div':
- return CopyDataDivEndpoint(params)
+ return CopyDataDivEndpoint(params, writable)
else:
raise ValueError("Invalid or unsupported endpoint %r" % params)
@@ -84,11 +87,20 @@
img_id = properties.UUID(required=True)
vol_id = properties.UUID(required=True)
- def __init__(self, params):
+ def __init__(self, params, writable):
self.sd_id = params.get('sd_id')
self.img_id = params.get('img_id')
self.vol_id = params.get('vol_id')
+ self._writable = writable
self._vol = None
+
+ @property
+ def locks(self):
+ img_ns = sd.getNamespace(sc.IMAGE_NAMESPACE, self.sd_id)
+ mode = rm.LockType.exclusive if self._writable else rm.LockType.shared
+ return (guarded.ResourceManagerLock(sc.STORAGE, self.sd_id,
+ rm.LockType.shared),
+ guarded.ResourceManagerLock(img_ns, self.img_id, mode))
@property
def path(self):
@@ -114,10 +126,10 @@
return sc.fmt2str(parent_vol.getFormat())
@contextmanager
- def prepare(self, writable=False):
+ def prepare(self):
dom = sdCache.produce_manifest(self.sd_id)
self._vol = dom.produceVolume(self.img_id, self.vol_id)
- self._vol.prepare(rw=writable, justme=True)
+ self._vol.prepare(rw=self._writable, justme=True)
try:
yield
finally:
--
To view, visit https://gerrit.ovirt.org/61693
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5c2a9527f5e3787069e0847e1bf775a60321b306
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
7 years, 9 months
Change in vdsm[master]: sampling: Retrieve backing chain stats
by ahino@redhat.com
Ala Hino has uploaded a new change for review.
Change subject: sampling: Retrieve backing chain stats
......................................................................
sampling: Retrieve backing chain stats
Update sampling mechanism to retrieve backing chain stats. This is
required to get the allocation value of all volumes in the chain.
These stats will be cachd in a volume UUID to allocatoin value
dictionary.
Based on the allocation value, we will find out how to extend volumes
during live merge. The logic of getting live merge ectend candidates
will be done in a separate patch.
Change-Id: I3bbb8643d1c86e90d1e2de7cb2a5b00116c71453
Signed-off-by: Ala Hino <ahino(a)redhat.com>
---
M lib/vdsm/virt/sampling.py
M lib/vdsm/virt/vmstats.py
2 files changed, 27 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/60888/1
diff --git a/lib/vdsm/virt/sampling.py b/lib/vdsm/virt/sampling.py
index 8feaf37..c92fc4a 100644
--- a/lib/vdsm/virt/sampling.py
+++ b/lib/vdsm/virt/sampling.py
@@ -31,6 +31,9 @@
import threading
import time
+# 3rd party libs imports
+import libvirt
+
from vdsm import numa
from vdsm import utils
from vdsm.constants import P_VDSM_RUN, P_VDSM_CLIENT_LOG
@@ -493,17 +496,19 @@
fast_path = acquired and not self._skip_doms
doms = [] # whitelist, meaningful only in the slow path
try:
+ flags = libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING
if fast_path:
# This is expected to be the common case.
# If everything's ok, we can skip all the costly checks.
- bulk_stats = self._conn.getAllDomainStats(self._stats)
+ bulk_stats = self._conn.getAllDomainStats(
+ self._stats, flags)
else:
# A previous call got stuck, or not every domain
# has properly recovered. Thus we must whitelist domains.
doms = self._get_responsive_doms()
if doms:
bulk_stats = self._conn.domainListGetStats(
- doms, self._stats)
+ doms, self._stats, flags)
else:
bulk_stats = []
except Exception:
diff --git a/lib/vdsm/virt/vmstats.py b/lib/vdsm/virt/vmstats.py
index 177e2a9..2f06a66 100644
--- a/lib/vdsm/virt/vmstats.py
+++ b/lib/vdsm/virt/vmstats.py
@@ -21,6 +21,7 @@
import contextlib
import logging
+import os
import six
@@ -43,6 +44,7 @@
cpu(stats, first_sample, last_sample, interval)
networks(vm, stats, first_sample, last_sample, interval)
disks(vm, stats, first_sample, last_sample, interval)
+ watermarks(stats, first_sample, last_sample, interval)
balloon(vm, stats, last_sample)
cpu_count(stats, last_sample)
tune_io(vm, stats)
@@ -352,6 +354,24 @@
return stats
+def watermarks(stats, first_sample, last_sample, interval):
+ if first_sample is None or last_sample is None:
+ return None
+
+ watermarks = {}
+ for key in last_sample:
+ if key.endswith("path"):
+ prefix, index, attr = key.split(".", 2)
+ path = last_sample[key]
+ alloc_key = "block" + "." + index + ".allocation"
+ allocation = last_sample[alloc_key]
+ vol_uuid = os.path.basename(path)
+ watermarks[vol_uuid] = allocation
+
+ if watermarks:
+ stats['watermarks'] = watermarks
+
+
def _disk_rate(first_sample, first_index, last_sample, last_index, interval):
stats = {}
--
To view, visit https://gerrit.ovirt.org/60888
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3bbb8643d1c86e90d1e2de7cb2a5b00116c71453
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino <ahino(a)redhat.com>
7 years, 9 months
Change in vdsm[master]: net: dhclient - address flush before starting and on shutdown
by edwardh@redhat.com
Edward Haas has uploaded a new change for review.
Change subject: net: dhclient - address flush before starting and on shutdown
......................................................................
net: dhclient - address flush before starting and on shutdown
Change-Id: I6e72f2aeb9f557a4cef49200fe195e6879490dcc
Signed-off-by: Edward Haas <edwardh(a)redhat.com>
---
M lib/vdsm/network/ip/dhclient.py
1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/63088/1
diff --git a/lib/vdsm/network/ip/dhclient.py b/lib/vdsm/network/ip/dhclient.py
index 6f60e65..4f76bed 100644
--- a/lib/vdsm/network/ip/dhclient.py
+++ b/lib/vdsm/network/ip/dhclient.py
@@ -32,6 +32,8 @@
from vdsm.commands import execCmd
from vdsm.utils import CommandPath, memoized, pgrep, kill_and_rm_pid
+from . import address
+
DHCLIENT_BINARY = CommandPath('dhclient', '/sbin/dhclient')
DHCLIENT_CGROUP = 'vdsm-dhclient'
LEASE_DIR = '/var/lib/dhclient'
@@ -62,6 +64,8 @@
def _dhclient(self):
if iface.exists(self.iface):
kill(self.iface, self.family)
+ address.flush(self.iface, family=self.family)
+
cmd = [DHCLIENT_BINARY.cmd, '-%s' % self.family, '-1', '-pf',
self.pidFile, '-lf', self.leaseFile]
cmd += ['-R', DhcpClient.DEFAULT_REQ_OPTIONS]
@@ -91,7 +95,10 @@
else:
raise
else:
+ logging.info('Stopping dhclient/%s on %s', self.family, self.iface)
kill_and_rm_pid(pid, self.pidFile)
+ if iface.exists(self.iface):
+ address.flush(self.iface)
def kill(device_name, family=4):
--
To view, visit https://gerrit.ovirt.org/63088
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6e72f2aeb9f557a4cef49200fe195e6879490dcc
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
7 years, 9 months
Change in vdsm[master]: net: dhclient kill - early exit if iface does not exists
by edwardh@redhat.com
Edward Haas has uploaded a new change for review.
Change subject: net: dhclient kill - early exit if iface does not exists
......................................................................
net: dhclient kill - early exit if iface does not exists
Change-Id: I9ba37ce248f1daeec8e79bd192261c1840417310
Signed-off-by: Edward Haas <edwardh(a)redhat.com>
---
M lib/vdsm/network/ip/dhclient.py
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/63087/1
diff --git a/lib/vdsm/network/ip/dhclient.py b/lib/vdsm/network/ip/dhclient.py
index c44299f..6f60e65 100644
--- a/lib/vdsm/network/ip/dhclient.py
+++ b/lib/vdsm/network/ip/dhclient.py
@@ -95,6 +95,8 @@
def kill(device_name, family=4):
+ if not iface.exists(device_name):
+ return
for pid, pid_file in _pid_lookup(device_name, family):
logging.info('Stopping dhclient -%s on %s', family, device_name)
kill_and_rm_pid(pid, pid_file)
--
To view, visit https://gerrit.ovirt.org/63087
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9ba37ce248f1daeec8e79bd192261c1840417310
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh(a)redhat.com>
7 years, 9 months
Change in vdsm[master]: storage: domainMonitor.getAllHostIds() API
by alitke@redhat.com
Adam Litke has uploaded a new change for review.
Change subject: storage: domainMonitor.getAllHostIds() API
......................................................................
storage: domainMonitor.getAllHostIds() API
SDM verbs require host_ids when working with Storage Domains. The current API
for getting a host_id is domainMonitor.getHostId(sd_id) where the domainMonitor
is accessible to functions in the HSM class. Therein lies the problem. The
dictionary arguments passed to SDM verbs should be opaque to HSM because the
validation and business logic is implemented down in the SDM code. To grant
SDM the ability to look up host_ids we could do one of the following:
- Pass the domainMonitor instance to all SDM verbs and let them call its
getHostId method when required, or
- Pass a map of sd_id : host_id to SDM verbs and let them look it up
The second option avoids proliferating the domainMonitor and keeps the layers
more separated. The second option is also easier to test. This patch
implements it by adding a getAllHostIds() API to the domainMonitor which
returns this mapping. To illustrate its usage, SDM.copy_volume is enhanced to
receive this map for eventual use when acquiring volume leases.
Change-Id: If1d89bb114eea58387cd88d9da3c27b69d6affa3
Signed-off-by: Adam Litke <alitke(a)redhat.com>
---
M tests/storage_sdm_copy_data_test.py
M vdsm/storage/hsm.py
M vdsm/storage/monitor.py
M vdsm/storage/sdm/api/copy_data.py
4 files changed, 26 insertions(+), 10 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/61997/1
diff --git a/tests/storage_sdm_copy_data_test.py b/tests/storage_sdm_copy_data_test.py
index def8934..7d56c66 100644
--- a/tests/storage_sdm_copy_data_test.py
+++ b/tests/storage_sdm_copy_data_test.py
@@ -38,6 +38,8 @@
import storage.sdm.api.copy_data
+HOST_ID = 1
+
@expandPermutations
class CopyDataTests(VdsmTestCase):
@@ -92,11 +94,13 @@
self.assertRaises(ChainVerificationError,
verify_qemu_chain, dst_chain)
+ host_id_map = {dst_vol.sdUUID: HOST_ID}
source = dict(endpoint_type='div', sd_id=src_vol.sdUUID,
img_id=src_vol.imgUUID, vol_id=src_vol.volUUID)
dest = dict(endpoint_type='div', sd_id=dst_vol.sdUUID,
img_id=dst_vol.imgUUID, vol_id=dst_vol.volUUID)
- job = storage.sdm.api.copy_data.Job(job_id, None, source, dest)
+ job = storage.sdm.api.copy_data.Job(job_id, host_id_map,
+ source, dest)
job.run()
wait_for_job(job)
@@ -125,11 +129,13 @@
job_id = str(uuid.uuid4())
src_vol = src_chain[index]
dst_vol = dst_chain[index]
+ host_id_map = {dst_vol.sdUUID: HOST_ID}
source = dict(endpoint_type='div', sd_id=src_vol.sdUUID,
img_id=src_vol.imgUUID, vol_id=src_vol.volUUID)
dest = dict(endpoint_type='div', sd_id=dst_vol.sdUUID,
img_id=dst_vol.imgUUID, vol_id=dst_vol.volUUID)
- job = storage.sdm.api.copy_data.Job(job_id, None, source, dest)
+ job = storage.sdm.api.copy_data.Job(job_id, host_id_map,
+ source, dest)
job.run()
wait_for_job(job)
verify_qemu_chain(dst_chain)
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index cfa3118..533346b 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -3521,5 +3521,6 @@
@public
def sdm_copy_data(self, job_id, source, destination):
- job = sdm.api.copy_data.Job(job_id, None, source, destination)
+ host_id_map = self.domainMonitor.getAllHostIds()
+ job = sdm.api.copy_data.Job(job_id, host_id_map, source, destination)
self.sdm_schedule(job)
diff --git a/vdsm/storage/monitor.py b/vdsm/storage/monitor.py
index 607f8af..c9232c7 100644
--- a/vdsm/storage/monitor.py
+++ b/vdsm/storage/monitor.py
@@ -214,6 +214,10 @@
def getHostId(self, sdUUID):
return self._monitors[sdUUID].hostId
+ def getAllHostIds(self):
+ return dict([(sd_id, self._monitors[sd_id].hostId)
+ for sd_id in self._monitors])
+
def shutdown(self):
"""
Called during shutdown to stop all monitors without releasing the host
diff --git a/vdsm/storage/sdm/api/copy_data.py b/vdsm/storage/sdm/api/copy_data.py
index aff0185..9bd169a 100644
--- a/vdsm/storage/sdm/api/copy_data.py
+++ b/vdsm/storage/sdm/api/copy_data.py
@@ -40,10 +40,10 @@
"""
log = logging.getLogger('storage.sdm.copy_data')
- def __init__(self, job_id, host_id, source, destination):
- super(Job, self).__init__(job_id, 'copy_data', host_id)
- self._source = _create_endpoint(source)
- self._dest = _create_endpoint(destination)
+ def __init__(self, job_id, host_id_map, source, destination):
+ self._source = _create_endpoint(source, host_id_map)
+ self._dest = _create_endpoint(destination, host_id_map)
+ super(Job, self).__init__(job_id, 'copy_data', self._dest.host_id)
self._operation = None
@property
@@ -71,10 +71,10 @@
self._operation.wait_for_completion()
-def _create_endpoint(params):
+def _create_endpoint(params, host_id_map):
endpoint_type = params.pop('endpoint_type')
if endpoint_type == 'div':
- return CopyDataDivEndpoint(params)
+ return CopyDataDivEndpoint(params, host_id_map)
else:
raise ValueError("Invalid or unsupported endpoint %r" % params)
@@ -84,13 +84,18 @@
img_id = properties.UUID(required=True)
vol_id = properties.UUID(required=True)
- def __init__(self, params):
+ def __init__(self, params, host_id_map):
self.sd_id = params.get('sd_id')
self.img_id = params.get('img_id')
self.vol_id = params.get('vol_id')
+ self._host_id = host_id_map.get(self.sd_id)
self._vol = None
@property
+ def host_id(self):
+ return self._host_id
+
+ @property
def path(self):
return self._vol.getVolumePath()
--
To view, visit https://gerrit.ovirt.org/61997
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If1d89bb114eea58387cd88d9da3c27b69d6affa3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
7 years, 9 months
Change in vdsm[master]: sdm: Pass the real hostid to copy_data
by alitke@redhat.com
Adam Litke has uploaded a new change for review.
Change subject: sdm: Pass the real hostid to copy_data
......................................................................
sdm: Pass the real hostid to copy_data
We have been passing the hostid as None to copy_data but the correct
value will need to be passed in order to support volume leases. Since
we use the same hostid for all storage domains and the that hostid is
the same as the id attribute of the connected storage pool we can add a
simple helper to supply the proper hostid to sdm verbs. This will need
to be changed if we remove storage pools or if we opt to use different
hostid values for each domain.
Change-Id: I367576fca126be695d7617477f92110941620a3b
Signed-off-by: Adam Litke <alitke(a)redhat.com>
---
M vdsm/storage/hsm.py
1 file changed, 12 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/76/62576/1
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index cfa3118..e1e8b40 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -3510,6 +3510,16 @@
self.taskMng.scheduleJob("sdm", None, vars.task,
job.description, job.run)
+ def _get_hostid(self):
+ # Currently we use the pool.id as the hostid for all storage domains.
+ # If we get rid of the storage pool then we need to add an interface
+ # to fetch the hostid from the StorageDomainManifest object.
+ try:
+ pool = self.pools.values()[0]
+ except IndexError:
+ raise se.StoragePoolNotConnected()
+ return pool.id
+
@public
def sdm_create_volume(self, job_id, vol_info):
vol_info = sdm.api.create_volume.CreateVolumeInfo(vol_info)
@@ -3521,5 +3531,6 @@
@public
def sdm_copy_data(self, job_id, source, destination):
- job = sdm.api.copy_data.Job(job_id, None, source, destination)
+ job = sdm.api.copy_data.Job(job_id, self._get_hostid(),
+ source, destination)
self.sdm_schedule(job)
--
To view, visit https://gerrit.ovirt.org/62576
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I367576fca126be695d7617477f92110941620a3b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
7 years, 9 months
Change in vdsm[master]: storage: Introduce guarded utilities
by alitke@redhat.com
Adam Litke has uploaded a new change for review.
Change subject: storage: Introduce guarded utilities
......................................................................
storage: Introduce guarded utilities
Throughout the storage code we have implicit locking rules which we follow in
order to ensure that concurrent operations will not conflict with one another.
We also require a specific locking order to prevent deadlocks. This code is
repeated throughout the storage subsystem and is error prone. To alleviate
this, introduce the guarded module with a context manager that can perform the
necessary locking. At first we will support only resourceManager locks but
this model can be easily extended to a support for volume leases.
The contextmanager accepts a list of entities which need to be guarded within
the context. It then collects all of the required locks from the entities and
locks them according to the rules. When exiting the context (or upon error)
the locks are released in reverse order.
Entities must provide the get_rm_locks() method which returns a list of ResourceManagerLocks required.
The following rules are currently enforced:
- Locks are collected from all entities and sorted
- Duplicate locks are removed
- Exclusive mode takes precedence over shared mode
Change-Id: I2b0a204818d44b6205515277f4c2834cb2b7a057
Signed-off-by: Adam Litke <alitke(a)redhat.com>
---
A lib/vdsm/storage/guarded.py
M tests/Makefile.am
A tests/storage_guarded_test.py
M vdsm.spec.in
4 files changed, 236 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/35/61435/1
diff --git a/lib/vdsm/storage/guarded.py b/lib/vdsm/storage/guarded.py
new file mode 100644
index 0000000..5614cf0
--- /dev/null
+++ b/lib/vdsm/storage/guarded.py
@@ -0,0 +1,94 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+from __future__ import absolute_import
+import logging
+
+from storage import resourceManager as rm
+
+log = logging.getLogger('storage.guarded')
+rmanager = rm.ResourceManager.getInstance()
+
+
+class operation_context(object):
+
+ def __init__(self, entities):
+ self._rm_locks = self._collect_rm_locks(entities)
+ self._held_locks = []
+
+ def __enter__(self):
+ try:
+ for lock in self._rm_locks:
+ lock.acquire()
+ self._held_locks.append(lock)
+ except:
+ self._release()
+ raise
+
+ def __exit__(self, exc_type, exc_val, exc_tb):
+ self._release()
+
+ def _collect_rm_locks(self, entities):
+ locks = []
+ for entity in entities:
+ locks.extend(entity.get_rm_locks())
+
+ # Exclusive locks always take precedence over shared
+ lock_set = set([l for l in locks if l.mode == rm.LockType.exclusive])
+ shared_locks = set([l for l in locks if l.mode == rm.LockType.shared])
+ lock_set.update(shared_locks)
+ return sorted(lock_set)
+
+ def _release(self):
+ while self._held_locks:
+ lock = self._held_locks.pop()
+ lock.release()
+
+
+class Entity(object):
+ """
+ Defines the interface that must be implemented by objects that wish to be
+ guarded by operation_context.
+ """
+
+ def get_rm_locks(self):
+ return []
+
+
+class ResourceManagerLock(object):
+ def __init__(self, ns, name, mode):
+ self.ns = ns
+ self.name = name
+ self.mode = mode
+
+ def acquire(self):
+ rmanager.acquireResource(self.ns, self.name, self.mode)
+
+ def release(self):
+ rmanager.releaseResource(self.ns, self.name)
+
+ def _key(self):
+ return self.ns, self.name
+
+ def __cmp__(self, other):
+ return cmp(self._key(), other._key())
+
+ def __hash__(self):
+ return hash(self._key())
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 467f2d4..c6e527f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -122,6 +122,7 @@
storage_asyncevent_test.py \
storage_check_test.py \
storage_directio_test.py \
+ storage_guarded_test.py \
storage_hsm_test.py \
storage_monitor_test.py \
storage_rwlock_test.py \
@@ -221,6 +222,7 @@
stompAsyncDispatcherTests.py \
stompTests.py \
storageMailboxTests.py \
+ storage_guarded_test.py \
storage_hsm_test.py \
storage_monitor_test.py \
storageServerTests.py \
diff --git a/tests/storage_guarded_test.py b/tests/storage_guarded_test.py
new file mode 100644
index 0000000..29db745
--- /dev/null
+++ b/tests/storage_guarded_test.py
@@ -0,0 +1,139 @@
+#
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+
+from __future__ import absolute_import
+from contextlib import contextmanager
+
+from monkeypatch import MonkeyPatchScope
+from storagefakelib import FakeResourceManager
+from testlib import VdsmTestCase, expandPermutations, permutations
+
+from vdsm.storage import guarded
+
+from storage import resourceManager as rm
+
+
+@expandPermutations
+class ContextTest(VdsmTestCase):
+
+ @contextmanager
+ def env(self):
+ rmanager = FakeResourceManager()
+ with MonkeyPatchScope([(guarded, 'rmanager', rmanager)]):
+ yield rmanager
+
+ def test_none(self):
+ with self.env():
+ with guarded.operation_context([]):
+ pass
+
+ def test_remove_duplicates(self):
+ resource_locks = [
+ guarded.ResourceManagerLock('A', 'a', rm.LockType.shared),
+ ]
+ src = GuardedEntity(resource_locks)
+ dst = GuardedEntity(resource_locks)
+ with self.env() as rmanager:
+ calls = [('acquireResource', ('A', 'a', rm.LockType.shared), {})]
+ with guarded.operation_context((src, dst)):
+ self.assertEqual(calls, rmanager.__calls__)
+ calls.append(('releaseResource', ('A', 'a'), {}))
+
+ def test_sorting(self):
+ src_locks = [
+ guarded.ResourceManagerLock('C', 'a', rm.LockType.shared),
+ guarded.ResourceManagerLock('A', 'b', rm.LockType.shared),
+ ]
+ src = GuardedEntity(src_locks)
+ dst_locks = [
+ guarded.ResourceManagerLock('A', 'a', rm.LockType.shared),
+ guarded.ResourceManagerLock('B', 'a', rm.LockType.shared),
+ ]
+ dst = GuardedEntity(dst_locks)
+ with self.env() as rmanager:
+ calls = [
+ ('acquireResource', ('A', 'a', rm.LockType.shared), {}),
+ ('acquireResource', ('A', 'b', rm.LockType.shared), {}),
+ ('acquireResource', ('B', 'a', rm.LockType.shared), {}),
+ ('acquireResource', ('C', 'a', rm.LockType.shared), {}),
+ ]
+ with guarded.operation_context((src, dst)):
+ self.assertEqual(calls, rmanager.__calls__)
+ calls.extend([
+ ('releaseResource', ('C', 'a'), {}),
+ ('releaseResource', ('B', 'a'), {}),
+ ('releaseResource', ('A', 'b'), {}),
+ ('releaseResource', ('A', 'a'), {}),
+ ])
+ self.assertEqual(calls, rmanager.__calls__)
+
+ def test_resource_exclusive_and_shared(self):
+ locks = [guarded.ResourceManagerLock('A', 'a', rm.LockType.shared)]
+ src = GuardedEntity(locks)
+ locks = [guarded.ResourceManagerLock('A', 'a', rm.LockType.exclusive)]
+ dst = GuardedEntity(locks)
+ with self.env() as rmanager:
+ calls = [('acquireResource',
+ ('A', 'a', rm.LockType.exclusive), {}),
+ ('releaseResource', ('A', 'a'), {})]
+ with guarded.operation_context((src, dst)):
+ pass
+ self.assertEqual(calls, rmanager.__calls__)
+
+
+class GuardedEntity(guarded.Entity):
+
+ def __init__(self, rm_list):
+ self.rm_list = rm_list
+
+ def get_rm_locks(self):
+ return self.rm_list
+
+
+@expandPermutations
+class ResourceManagerLockTest(VdsmTestCase):
+
+ def test_acquire_release(self):
+ res_mgr = FakeResourceManager()
+
+ res = guarded.ResourceManagerLock('ns_A', 'name_A', rm.LockType.shared)
+ with MonkeyPatchScope([(guarded, 'rmanager', res_mgr)]):
+ expected = []
+ res.acquire()
+ expected.append(('acquireResource',
+ (res.ns, res.name, res.mode), {}))
+ self.assertEqual(expected, res_mgr.__calls__)
+ res.release()
+ expected.append(('releaseResource', (res.ns, res.name), {}))
+ self.assertEqual(expected, res_mgr.__calls__)
+
+ @permutations((
+ (('A', 'a', rm.LockType.shared), ('B', 'a', rm.LockType.shared)),
+ (('A', 'a', rm.LockType.shared), ('A', 'b', rm.LockType.shared)),
+ ))
+ def test_less_than(self, a, b):
+ b = guarded.ResourceManagerLock(*b)
+ a = guarded.ResourceManagerLock(*a)
+ self.assertLess(a, b)
+
+ def test_equal_with_different_mode(self):
+ a = guarded.ResourceManagerLock('A', 'a', rm.LockType.shared)
+ b = guarded.ResourceManagerLock('A', 'a', rm.LockType.exclusive)
+ self.assertEqual(a, b)
diff --git a/vdsm.spec.in b/vdsm.spec.in
index b5d4f67..9b95e98 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1240,6 +1240,7 @@
%{python_sitelib}/%{vdsm_name}/storage/exception.py*
%{python_sitelib}/%{vdsm_name}/storage/fileUtils.py*
%{python_sitelib}/%{vdsm_name}/storage/fuser.py*
+%{python_sitelib}/%{vdsm_name}/storage/guarded.py*
%{python_sitelib}/%{vdsm_name}/storage/hba.py*
%{python_sitelib}/%{vdsm_name}/storage/misc.py*
%{python_sitelib}/%{vdsm_name}/storage/mount.py*
--
To view, visit https://gerrit.ovirt.org/61435
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b0a204818d44b6205515277f4c2834cb2b7a057
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
7 years, 9 months
Change in vdsm[master]: tests: Introduce FakeSanlock
by alitke@redhat.com
Adam Litke has uploaded a new change for review.
Change subject: tests: Introduce FakeSanlock
......................................................................
tests: Introduce FakeSanlock
For the few tests that depend on sanlock, introduce FakeSanlock which
does nothing except record acquire and release calls so that we can
check that locks were acquired as expected.
Change-Id: I5701b85b02059cf6e16795a54fadfe3213e346ca
Signed-off-by: Adam Litke <alitke(a)redhat.com>
---
M tests/storagefakelib.py
1 file changed, 18 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/65/62265/1
diff --git a/tests/storagefakelib.py b/tests/storagefakelib.py
index 7ea0cee..d52e19e 100644
--- a/tests/storagefakelib.py
+++ b/tests/storagefakelib.py
@@ -307,3 +307,21 @@
def manuallyRemoveDomain(self, sdUUID):
self.domains.pop(sdUUID, None)
+
+
+class FakeSanlock(object):
+ SLKFD = 15
+
+ def register(self):
+ return self.SLKFD
+
+ def init_resource(self, sdUUID, volUUID, lockDisk):
+ pass
+
+ @recorded
+ def acquire(self, lockspace, resource, lease, slkfd):
+ pass
+
+ @recorded
+ def release(self, lockspace, resource, lease, slkfd):
+ pass
--
To view, visit https://gerrit.ovirt.org/62265
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5701b85b02059cf6e16795a54fadfe3213e346ca
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
7 years, 9 months