January 2016 - vdsm-patches - Fedora mailing-lists

Change in vdsm[master]: WIP: add simple tracemalloc inspection tool
by fromani＠redhat.com 31 Aug '16

31 Aug '16

Francesco Romani has uploaded a new change for review. Change subject: WIP: add simple tracemalloc inspection tool ...................................................................... WIP: add simple tracemalloc inspection tool Change-Id: Ideb2652f345edb6f4a4d66c5299b601e53e85d33 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- A contrib/memory-stats A contrib/tracemalloc.py 2 files changed, 592 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/48115/1 diff --git a/contrib/memory-stats b/contrib/memory-stats new file mode 100755 index 0000000..5625ad1 --- /dev/null +++ b/contrib/memory-stats @@ -0,0 +1,102 @@ +#!/usr/bin/env python +# +# Copyright 2015 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 +# + +import argparse +import os +import sys +import time + +import tracemalloc + + +def eprint(s): + sys.stderr.write('%s\n' % s) + + +def _load(filename): + if filename: + eprint('Loading snapshot: "%s"' % filename) + before = time.time() + snap = tracemalloc.Snapshot.load(filename) + eprint('Loaded snapshot: "%s" in %.3f seconds' % ( + filename, time.time() - before)) + return snap + return None + + +def display_top(snapshot1, snapshot2=None, group_by='lineno', limit=10): + if snapshot2 is None: + top_stats = snapshot1.statistics(group_by) + else: + top_stats = snapshot2.compare_to(snapshot1, group_by) + print_stats(top_stats, limit) + + +def _print_sep(width=52): + print('-' * width) + + +def print_stats(top_stats, limit=10): + total = float(sum(stat.size for stat in top_stats)) + print("* Total allocated size: %.2f KiB" % (total / 1024.)) + print("* Top %s lines:" % limit) + _print_sep() + for index, stat in enumerate(top_stats[:limit], 1): + frame = stat.traceback[0] + # replace "/path/to/module/file.py" with "module/file.py" + filename = os.sep.join(frame.filename.split(os.sep)[-2:]) + where = "%s:%s" % (filename, frame.lineno) + print("#%3s: %-32s: %.2f KiB (%.2f%%)" + % (index, where, + stat.size / 1024., + (100. * stat.size) / total)) + + other = top_stats[limit:] + if other: + size = sum(stat.size for stat in other) + _print_sep() + where = "%i entries" % len(other) + print("rest: %-32s: %.2f KiB (%.2f%%)" + % (where, size / 1024., (100. * size) / total)) + + +ap = argparse.ArgumentParser(description='analyze tracemalloc snapshot') +ap.add_argument('snapshot', nargs='?', help='memory snapshot to analyze') +ap.add_argument('-c', '--compare-to', metavar='OS', dest='to_compare', + help='memory snapshot to compare') +ap.add_argument('-g', '--group-by', metavar='ATTR', dest='groupby', + default='lineno', + help='group samples by either: lineno, filename, traceback') +ap.add_argument('-l', '--limit', metavar='L', dest='limit', default=10, + type=int, + help='limit output to L top entries') + + +args = ap.parse_args() + +if not args.snapshot: + ap.print_help() + sys.exit(1) + +display_top(_load(args.snapshot), + _load(args.to_compare), + args.groupby, + args.limit) diff --git a/contrib/tracemalloc.py b/contrib/tracemalloc.py new file mode 100644 index 0000000..47b633e --- /dev/null +++ b/contrib/tracemalloc.py @@ -0,0 +1,490 @@ +# Copy of tracemalloc.py from pytracemalloc without _tracemalloc, +# to be able to read snapshot without having to install the _tracemaloc +# module. +from collections import Sequence, Iterable +import fnmatch +import linecache +import os.path +import pickle + + +try: + from functools import total_ordering +except ImportError: + # Python 2.6 + def total_ordering(cls): + # Function backported from Python 2.7 + convert = { + '__lt__': [('__gt__', lambda self, other: _not_op_and_not_eq(self.__lt__, self, other)), + ('__le__', lambda self, other: _op_or_eq(self.__lt__, self, other)), + ('__ge__', lambda self, other: _not_op(self.__lt__, other))], + '__le__': [('__ge__', lambda self, other: _not_op_or_eq(self.__le__, self, other)), + ('__lt__', lambda self, other: _op_and_not_eq(self.__le__, self, other)), + ('__gt__', lambda self, other: _not_op(self.__le__, other))], + '__gt__': [('__lt__', lambda self, other: _not_op_and_not_eq(self.__gt__, self, other)), + ('__ge__', lambda self, other: _op_or_eq(self.__gt__, self, other)), + ('__le__', lambda self, other: _not_op(self.__gt__, other))], + '__ge__': [('__le__', lambda self, other: _not_op_or_eq(self.__ge__, self, other)), + ('__gt__', lambda self, other: _op_and_not_eq(self.__ge__, self, other)), + ('__lt__', lambda self, other: _not_op(self.__ge__, other))] + } + roots = [op for op in convert if getattr(cls, op, None) is not getattr(object, op, None)] + if not roots: + raise ValueError('must define at least one ordering operation: < > <= >=') + root = max(roots) + for opname, opfunc in convert[root]: + if opname not in roots: + opfunc.__name__ = opname + opfunc.__doc__ = getattr(int, opname).__doc__ + setattr(cls, opname, opfunc) + return cls + + +def _format_size(size, sign): + for unit in ('B', 'KiB', 'MiB', 'GiB', 'TiB'): + if abs(size) < 100 and unit != 'B': + # 3 digits (xx.x UNIT) + if sign: + return "%+.1f %s" % (size, unit) + else: + return "%.1f %s" % (size, unit) + if abs(size) < 10 * 1024 or unit == 'TiB': + # 4 or 5 digits (xxxx UNIT) + if sign: + return "%+.0f %s" % (size, unit) + else: + return "%.0f %s" % (size, unit) + size /= 1024 + + +class Statistic(object): + """ + Statistic difference on memory allocations between two Snapshot instance. + """ + + __slots__ = ('traceback', 'size', 'count') + + def __init__(self, traceback, size, count): + self.traceback = traceback + self.size = size + self.count = count + + def __hash__(self): + return hash((self.traceback, self.size, self.count)) + + def __eq__(self, other): + return (self.traceback == other.traceback + and self.size == other.size + and self.count == other.count) + + def __str__(self): + text = ("%s: size=%s, count=%i" + % (self.traceback, + _format_size(self.size, False), + self.count)) + if self.count: + average = self.size / self.count + text += ", average=%s" % _format_size(average, False) + return text + + def __repr__(self): + return ('<Statistic traceback=%r size=%i count=%i>' + % (self.traceback, self.size, self.count)) + + def _sort_key(self): + return (self.size, self.count, self.traceback) + + +class StatisticDiff(object): + """ + Statistic difference on memory allocations between an old and a new + Snapshot instance. + """ + __slots__ = ('traceback', 'size', 'size_diff', 'count', 'count_diff') + + def __init__(self, traceback, size, size_diff, count, count_diff): + self.traceback = traceback + self.size = size + self.size_diff = size_diff + self.count = count + self.count_diff = count_diff + + def __hash__(self): + return hash((self.traceback, self.size, self.size_diff, + self.count, self.count_diff)) + + def __eq__(self, other): + return (self.traceback == other.traceback + and self.size == other.size + and self.size_diff == other.size_diff + and self.count == other.count + and self.count_diff == other.count_diff) + + def __str__(self): + text = ("%s: size=%s (%s), count=%i (%+i)" + % (self.traceback, + _format_size(self.size, False), + _format_size(self.size_diff, True), + self.count, + self.count_diff)) + if self.count: + average = self.size / self.count + text += ", average=%s" % _format_size(average, False) + return text + + def __repr__(self): + return ('<StatisticDiff traceback=%r size=%i (%+i) count=%i (%+i)>' + % (self.traceback, self.size, self.size_diff, + self.count, self.count_diff)) + + def _sort_key(self): + return (abs(self.size_diff), self.size, + abs(self.count_diff), self.count, + self.traceback) + + +def _compare_grouped_stats(old_group, new_group): + statistics = [] + for traceback, stat in new_group.items(): + previous = old_group.pop(traceback, None) + if previous is not None: + stat = StatisticDiff(traceback, + stat.size, stat.size - previous.size, + stat.count, stat.count - previous.count) + else: + stat = StatisticDiff(traceback, + stat.size, stat.size, + stat.count, stat.count) + statistics.append(stat) + + for traceback, stat in old_group.items(): + stat = StatisticDiff(traceback, 0, -stat.size, 0, -stat.count) + statistics.append(stat) + return statistics + + +@total_ordering +class Frame(object): + """ + Frame of a traceback. + """ + __slots__ = ("_frame",) + + def __init__(self, frame): + # frame is a tuple: (filename: str, lineno: int) + self._frame = frame + + @property + def filename(self): + return self._frame[0] + + @property + def lineno(self): + return self._frame[1] + + def __eq__(self, other): + return (self._frame == other._frame) + + def __lt__(self, other): + return (self._frame < other._frame) + + def __hash__(self): + return hash(self._frame) + + def __str__(self): + return "%s:%s" % (self.filename, self.lineno) + + def __repr__(self): + return "<Frame filename=%r lineno=%r>" % (self.filename, self.lineno) + + +@total_ordering +class Traceback(Sequence): + """ + Sequence of Frame instances sorted from the most recent frame + to the oldest frame. + """ + __slots__ = ("_frames",) + + def __init__(self, frames): + Sequence.__init__(self) + # frames is a tuple of frame tuples: see Frame constructor for the + # format of a frame tuple + self._frames = frames + + def __len__(self): + return len(self._frames) + + def __getitem__(self, index): + if isinstance(index, slice): + return tuple(Frame(trace) for trace in self._frames[index]) + else: + return Frame(self._frames[index]) + + def __contains__(self, frame): + return frame._frame in self._frames + + def __hash__(self): + return hash(self._frames) + + def __eq__(self, other): + return (self._frames == other._frames) + + def __lt__(self, other): + return (self._frames < other._frames) + + def __str__(self): + return str(self[0]) + + def __repr__(self): + return "<Traceback %r>" % (tuple(self),) + + def format(self, limit=None): + lines = [] + if limit is not None and limit < 0: + return lines + for frame in self[:limit]: + lines.append(' File "%s", line %s' + % (frame.filename, frame.lineno)) + line = linecache.getline(frame.filename, frame.lineno).strip() + if line: + lines.append(' %s' % line) + return lines + + +class Trace(object): + """ + Trace of a memory block. + """ + __slots__ = ("_trace",) + + def __init__(self, trace): + # trace is a tuple: (size, traceback), see Traceback constructor + # for the format of the traceback tuple + self._trace = trace + + @property + def size(self): + return self._trace[0] + + @property + def traceback(self): + return Traceback(self._trace[1]) + + def __eq__(self, other): + return (self._trace == other._trace) + + def __hash__(self): + return hash(self._trace) + + def __str__(self): + return "%s: %s" % (self.traceback, _format_size(self.size, False)) + + def __repr__(self): + return ("<Trace size=%s, traceback=%r>" + % (_format_size(self.size, False), self.traceback)) + + +class _Traces(Sequence): + def __init__(self, traces): + Sequence.__init__(self) + # traces is a tuple of trace tuples: see Trace constructor + self._traces = traces + + def __len__(self): + return len(self._traces) + + def __getitem__(self, index): + if isinstance(index, slice): + return tuple(Trace(trace) for trace in self._traces[index]) + else: + return Trace(self._traces[index]) + + def __contains__(self, trace): + return trace._trace in self._traces + + def __eq__(self, other): + return (self._traces == other._traces) + + def __repr__(self): + return "<Traces len=%s>" % len(self) + + +def _normalize_filename(filename): + filename = os.path.normcase(filename) + if filename.endswith(('.pyc', '.pyo')): + filename = filename[:-1] + return filename + + +class Filter(object): + def __init__(self, inclusive, filename_pattern, + lineno=None, all_frames=False): + self.inclusive = inclusive + self._filename_pattern = _normalize_filename(filename_pattern) + self.lineno = lineno + self.all_frames = all_frames + + @property + def filename_pattern(self): + return self._filename_pattern + + def __match_frame(self, filename, lineno): + filename = _normalize_filename(filename) + if not fnmatch.fnmatch(filename, self._filename_pattern): + return False + if self.lineno is None: + return True + else: + return (lineno == self.lineno) + + def _match_frame(self, filename, lineno): + return self.__match_frame(filename, lineno) ^ (not self.inclusive) + + def _match_traceback(self, traceback): + if self.all_frames: + if any(self.__match_frame(filename, lineno) + for filename, lineno in traceback): + return self.inclusive + else: + return (not self.inclusive) + else: + filename, lineno = traceback[0] + return self._match_frame(filename, lineno) + + +class Snapshot(object): + """ + Snapshot of traces of memory blocks allocated by Python. + """ + + def __init__(self, traces, traceback_limit): + # traces is a tuple of trace tuples: see _Traces constructor for + # the exact format + self.traces = _Traces(traces) + self.traceback_limit = traceback_limit + + def dump(self, filename): + """ + Write the snapshot into a file. + """ + with open(filename, "wb") as fp: + pickle.dump(self, fp, pickle.HIGHEST_PROTOCOL) + + @staticmethod + def load(filename): + """ + Load a snapshot from a file. + """ + with open(filename, "rb") as fp: + return pickle.load(fp) + + def _filter_trace(self, include_filters, exclude_filters, trace): + traceback = trace[1] + if include_filters: + if not any(trace_filter._match_traceback(traceback) + for trace_filter in include_filters): + return False + if exclude_filters: + if any(not trace_filter._match_traceback(traceback) + for trace_filter in exclude_filters): + return False + return True + + def filter_traces(self, filters): + """ + Create a new Snapshot instance with a filtered traces sequence, filters + is a list of Filter instances. If filters is an empty list, return a + new Snapshot instance with a copy of the traces. + """ + if not isinstance(filters, Iterable): + raise TypeError("filters must be a list of filters, not %s" + % type(filters).__name__) + if filters: + include_filters = [] + exclude_filters = [] + for trace_filter in filters: + if trace_filter.inclusive: + include_filters.append(trace_filter) + else: + exclude_filters.append(trace_filter) + new_traces = [trace for trace in self.traces._traces + if self._filter_trace(include_filters, + exclude_filters, + trace)] + else: + new_traces = self.traces._traces[:] + return Snapshot(new_traces, self.traceback_limit) + + def _group_by(self, key_type, cumulative): + if key_type not in ('traceback', 'filename', 'lineno'): + raise ValueError("unknown key_type: %r" % (key_type,)) + if cumulative and key_type not in ('lineno', 'filename'): + raise ValueError("cumulative mode cannot by used " + "with key type %r" % key_type) + + stats = {} + tracebacks = {} + if not cumulative: + for trace in self.traces._traces: + size, trace_traceback = trace + try: + traceback = tracebacks[trace_traceback] + except KeyError: + if key_type == 'traceback': + frames = trace_traceback + elif key_type == 'lineno': + frames = trace_traceback[:1] + else: # key_type == 'filename': + frames = ((trace_traceback[0][0], 0),) + traceback = Traceback(frames) + tracebacks[trace_traceback] = traceback + try: + stat = stats[traceback] + stat.size += size + stat.count += 1 + except KeyError: + stats[traceback] = Statistic(traceback, size, 1) + else: + # cumulative statistics + for trace in self.traces._traces: + size, trace_traceback = trace + for frame in trace_traceback: + try: + traceback = tracebacks[frame] + except KeyError: + if key_type == 'lineno': + frames = (frame,) + else: # key_type == 'filename': + frames = ((frame[0], 0),) + traceback = Traceback(frames) + tracebacks[frame] = traceback + try: + stat = stats[traceback] + stat.size += size + stat.count += 1 + except KeyError: + stats[traceback] = Statistic(traceback, size, 1) + return stats + + def statistics(self, key_type, cumulative=False): + """ + Group statistics by key_type. Return a sorted list of Statistic + instances. + """ + grouped = self._group_by(key_type, cumulative) + statistics = list(grouped.values()) + statistics.sort(reverse=True, key=Statistic._sort_key) + return statistics + + def compare_to(self, old_snapshot, key_type, cumulative=False): + """ + Compute the differences with an old snapshot old_snapshot. Get + statistics as a sorted list of StatisticDiff instances, grouped by + group_by. + """ + new_group = self._group_by(key_type, cumulative) + old_group = old_snapshot._group_by(key_type, cumulative) + statistics = _compare_grouped_stats(old_group, new_group) + statistics.sort(reverse=True, key=StatisticDiff._sort_key) + return statistics -- To view, visit https://gerrit.ovirt.org/48115 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ideb2652f345edb6f4a4d66c5299b601e53e85d33 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

2 4

Change in vdsm[master]: utils: Add changehash function for change detection
by Nir Soffer 27 Aug '16

27 Aug '16

Nir Soffer has uploaded a new change for review. Change subject: utils: Add changehash function for change detection ...................................................................... utils: Add changehash function for change detection We use Python built-in hash to detect changes in vm state without sending the state in each response. This function is not suitable for this usage. Now we use generic utils.changehash(), implemented using md5 hexdigest. Change-Id: I2242a594383e2d2fe64e3a581f18b8ac662648b0 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M lib/vdsm/utils.py M vdsm/virt/vm.py 2 files changed, 13 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/33045/1 diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index 23c63e8..1b4a9d5 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -37,6 +37,7 @@ import glob import io import itertools +import hashlib import logging import re import sys @@ -1133,3 +1134,13 @@ flags = fcntl.fcntl(fd, fcntl.F_GETFL) flags |= os.O_NONBLOCK fcntl.fcntl(fd, fcntl.F_SETFL, flags) + + +def changehash(s): + """ + Returns a hash of string s, suitable for change detection. + + Tipically changehash(s) is sent to client frequently. When a client detect + that changehash(s) changed, it ask for s itself, which may be much bigger. + """ + return hashlib.md5(s).hexdigest() diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 941f283..b1567f9 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -1500,7 +1500,7 @@ self.guestAgent = guestagent.GuestAgent( self._guestSocketFile, self.cif.channelListener, self.log) self._lastXMLDesc = '<domain><uuid>%s</uuid></domain>' % self.id - self._devXmlHash = '0' + self._devXmlHash = utils.changehash('') self._released = False self._releaseLock = threading.Lock() self.saveState() @@ -4495,7 +4495,7 @@ self._lastXMLDesc = self._dom.XMLDesc(0) devxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \ getElementsByTagName('devices')[0] - self._devXmlHash = str(hash(devxml.toxml())) + self._devXmlHash = utils.changehash(devxml.toxml()) return self._lastXMLDesc -- To view, visit http://gerrit.ovirt.org/33045 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2242a594383e2d2fe64e3a581f18b8ac662648b0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

9 26

Change in vdsm[master]: Adding toolTests.py to python3 check modules
by ybronhei＠redhat.com 23 Aug '16

23 Aug '16

Yaniv Bronhaim has uploaded a new change for review. Change subject: Adding toolTests.py to python3 check modules ...................................................................... Adding toolTests.py to python3 check modules Change-Id: I52f11a1a10cae46773d05d0c09da80ad8eb3b772 Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com> --- M lib/vdsm/tool/configfile.py M tests/Makefile.am 2 files changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/52/48052/1 diff --git a/lib/vdsm/tool/configfile.py b/lib/vdsm/tool/configfile.py index 26ec114..0c58205 100644 --- a/lib/vdsm/tool/configfile.py +++ b/lib/vdsm/tool/configfile.py @@ -18,12 +18,12 @@ # from __future__ import absolute_import -import ConfigParser import functools import os import tempfile import re import selinux +from six.moves import configparser as ConfigParser import io from .. import utils @@ -263,5 +263,5 @@ def read(self, path): with open(path, 'r') as f: return self.wrapped.readfp( - io.StringIO(u'[root]\n' + f.read().decode()) + io.StringIO(u'[root]\n' + f.read()) ) diff --git a/tests/Makefile.am b/tests/Makefile.am index 205289e..3b1f8bf 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -30,6 +30,7 @@ apiData.py \ cmdutilsTests.py \ concurrentTests.py \ + toolTests.py \ $(NULL) device_modules = \ -- To view, visit https://gerrit.ovirt.org/48052 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I52f11a1a10cae46773d05d0c09da80ad8eb3b772 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>

5 15

Change in vdsm[master]: clusterLock: Acquire, release, and inquire volume leases
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: clusterLock: Acquire, release, and inquire volume leases ...................................................................... clusterLock: Acquire, release, and inquire volume leases Although we have been creating volume leases on storage for awhile now, there have been no APIs to manage them. Add this support to the clusterLock implementations. Change-Id: Icca901ccd27358767c023cd55b7a3823531d2a5a Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/clusterlock.py M vdsm/storage/sd.py 2 files changed, 80 insertions(+), 23 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/38622/1 diff --git a/vdsm/storage/clusterlock.py b/vdsm/storage/clusterlock.py index 51d2906..4077068 100644 --- a/vdsm/storage/clusterlock.py +++ b/vdsm/storage/clusterlock.py @@ -73,6 +73,10 @@ """Raised when the clusterlock class is not supporting inquire""" +class ResourceLeasesNotSupportedError(Exception): + """Raised when the clusterlock class does not support resource leases""" + + class SafeLease(object): log = logging.getLogger("Storage.SafeLease") @@ -145,7 +149,13 @@ raise se.AcquireLockFailure(self._sdUUID, rc, out, err) self.log.debug("Clustered lock acquired successfully") + def acquireResource(self, resource, lockDisk, shared=False): + raise ResourceLeasesNotSupportedError() + def inquire(self): + raise InquireNotSupportedError() + + def inquireResource(self, resource, lockDisk): raise InquireNotSupportedError() def getLockUtilFullPath(self): @@ -165,6 +175,8 @@ self.log.debug("Cluster lock released successfully") + def releaseResource(self, resource, lockDisk): + raise ResourceLeasesNotSupportedError() initSANLockLog = logging.getLogger("Storage.initSANLock") @@ -286,13 +298,9 @@ # HOST_FAIL. return HOST_STATUS_FREE - # The hostId parameter is maintained here only for compatibility with - # ClusterLock. We could consider to remove it in the future but keeping it - # for logging purpose is desirable. - def acquire(self, hostId): + def _acquire(self, resource, lockDisk, shared=False): with nested(self._lock, SANLock._sanlock_lock): - self.log.info("Acquiring cluster lock for domain %s (id: %s)", - self._sdUUID, hostId) + self.log.info("Acquiring resource %s, shared=%s", resource, shared) while True: if SANLock._sanlock_fd is None: @@ -304,27 +312,42 @@ "Cannot register to sanlock", str(e)) try: - sanlock.acquire(self._sdUUID, SDM_LEASE_NAME, - self.getLockDisk(), - slkfd=SANLock._sanlock_fd) + sanlock.acquire(self._sdUUID, resource, lockDisk, + slkfd=SANLock._sanlock_fd, shared=shared) except sanlock.SanlockException as e: if e.errno != os.errno.EPIPE: raise se.AcquireLockFailure( self._sdUUID, e.errno, - "Cannot acquire cluster lock", str(e)) + "Cannot acquire sanlock resource", str(e)) SANLock._sanlock_fd = None continue break - self.log.debug("Cluster lock for domain %s successfully acquired " - "(id: %s)", self._sdUUID, hostId) + self.log.debug("Resource %s successfully acquired", resource) + + # The hostId parameter is maintained here only for compatibility with + # ClusterLock. We could consider to remove it in the future but keeping it + # for logging purpose is desirable. + def acquire(self, hostId): + self.log.info("Acquiring cluster lock for domain %s (id: %s)", + self._sdUUID, hostId) + self._acquire(SDM_LEASE_NAME, self.getLockDisk()) + self.log.debug("Cluster lock for domain %s successfully acquired " + "(id: %s)", self._sdUUID, hostId) + + def acquireResource(self, resource, lockDisk, shared=False): + self._acquire(resource, lockDisk, shared) + res, owners = self._inquire(resource, lockDisk) + self.log.debug("ALITKE: acquire: res:%s owners:%s", res, owners) + + def _inquire(self, resource, lockDisk): + res = sanlock.read_resource(*lockDisk[0]) + owners = sanlock.read_resource_owners(self._sdUUID, resource, lockDisk) + return res, owners def inquire(self): - resource = sanlock.read_resource(self._leasesPath, SDM_LEASE_OFFSET) - owners = sanlock.read_resource_owners(self._sdUUID, SDM_LEASE_NAME, - self.getLockDisk()) - + resource, owners = self._inquire(SDM_LEASE_NAME, self.getLockDisk()) if len(owners) == 1: return resource.get("version"), owners[0].get("host_id") elif len(owners) > 1: @@ -334,19 +357,32 @@ return None, None - def release(self): + def inquireResource(self, resource, lockDisk): + resource, owners = self._inquire(SDM_LEASE_NAME, self.getLockDisk()) + return (resource.get("version"), + [owner.get("host_id") for owner in owners]) + + def _release(self, resource, lockDisk): with self._lock: - self.log.info("Releasing cluster lock for domain %s", self._sdUUID) + self.log.info("Releasing resource %s", resource) try: - sanlock.release(self._sdUUID, SDM_LEASE_NAME, - self.getLockDisk(), slkfd=SANLock._sanlock_fd) + sanlock.release(self._sdUUID, resource, lockDisk, + slkfd=SANLock._sanlock_fd) except sanlock.SanlockException as e: - raise se.ReleaseLockFailure(self._sdUUID, e) + raise se.ReleaseLockFailure(resource, e) self._sanlockfd = None - self.log.debug("Cluster lock for domain %s successfully released", - self._sdUUID) + self.log.debug("Resource %s successfully released", resource) + + def release(self): + self.log.info("Releasing cluster lock for domain %s", self._sdUUID) + self._release(SDM_LEASE_NAME, self.getLockDisk()) + self.log.debug("Cluster lock for domain %s successfully released", + self._sdUUID) + + def releaseResource(self, resource, lockDisk): + self._release(resource, lockDisk) class LocalLock(object): @@ -462,10 +498,16 @@ self.log.debug("Local lock for domain %s successfully acquired " "(id: %s)", self._sdUUID, hostId) + def acquireResource(self, resource, lockDisk, shared=False): + raise ResourceLeasesNotSupportedError() + def inquire(self): with self._globalLockMapSync: hostId, lockFile = self._getLease() return self.LVER, hostId if lockFile else None, None + + def inquireResource(self, resource, lockDisk): + raise InquireNotSupportedError() def release(self): with self._globalLockMapSync: @@ -483,3 +525,6 @@ self.log.debug("Local lock for domain %s successfully released", self._sdUUID) + + def releaseResource(self, resource, lockDisk): + raise ResourceLeasesNotSupportedError() \ No newline at end of file diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py index a771abf..9e6f281 100644 --- a/vdsm/storage/sd.py +++ b/vdsm/storage/sd.py @@ -514,6 +514,18 @@ def inquireClusterLock(self): return self._clusterLock.inquire() + def acquireVolumeLease(self, imgUUID, volUUID, shared=False): + lockDisk = self.getVolumeLease(imgUUID, volUUID) + self._clusterLock.acquireResource(volUUID, [lockDisk], shared) + + def releaseVolumeLease(self, imgUUID, volUUID): + lockDisk = self.getVolumeLease(imgUUID, volUUID) + self._clusterLock.releaseResource(volUUID, [lockDisk]) + + def inquireVolumeLease(self, imgUUID, volUUID): + lockDisk = self.getVolumeLease(imgUUID, volUUID) + return self._clusterLock.inquireResource(volUUID, [lockDisk]) + def attach(self, spUUID): self.invalidateMetadata() pools = self.getPools() -- To view, visit https://gerrit.ovirt.org/38622 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icca901ccd27358767c023cd55b7a3823531d2a5a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

5 47

Change in vdsm[master]: storage: Factor out getAllVolumes
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: storage: Factor out getAllVolumes ...................................................................... storage: Factor out getAllVolumes The SDM garbage collector needs to a mapping of volumes by imageID. Factor out this logic from getAllVolumes so it can be reused later. Change-Id: If8c55edf6416dddc4a4f30b91422b3e968df2b99 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/fileSD.py 1 file changed, 17 insertions(+), 12 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/77/40377/1 diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py index 1220939..042f836 100644 --- a/vdsm/storage/fileSD.py +++ b/vdsm/storage/fileSD.py @@ -18,7 +18,6 @@ # Refer to the README and COPYING files for full details of the license # -import collections import os import errno import logging @@ -405,6 +404,21 @@ """ pass + def getImageToVolumesMap(self): + volMetaPattern = os.path.join(self.mountpoint, self.sdUUID, + sd.DOMAIN_IMAGES, "*", "*.meta") + volMetaPaths = self.oop.glob.glob(volMetaPattern) + images = {} + for metaPath in volMetaPaths: + head, tail = os.path.split(metaPath) + volUUID, volExt = os.path.splitext(tail) + imgUUID = os.path.basename(head) + try: + images[imgUUID].append(volUUID) + except KeyError: + images[imgUUID] = [volUUID] + return images + def getAllVolumes(self): """ Return dict {volUUID: ((imgUUIDs,), parentUUID)} of the domain. @@ -422,17 +436,8 @@ Template volumes have no parent, and thus we report BLANK_UUID as their parentUUID. """ - volMetaPattern = os.path.join(self.mountpoint, self.sdUUID, - sd.DOMAIN_IMAGES, "*", "*.meta") - volMetaPaths = self.oop.glob.glob(volMetaPattern) - - # First create mapping from images to volumes - images = collections.defaultdict(list) - for metaPath in volMetaPaths: - head, tail = os.path.split(metaPath) - volUUID, volExt = os.path.splitext(tail) - imgUUID = os.path.basename(head) - images[imgUUID].append(volUUID) + # First get the mapping from images to volumes + images = self.getImageToVolumesMap() # Using images to volumes mapping, we can create volumes to images # mapping, detecting template volumes and template images, based on -- To view, visit https://gerrit.ovirt.org/40377 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: If8c55edf6416dddc4a4f30b91422b3e968df2b99 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

4 18

Change in vdsm[master]: clusterlock: Add reference counting
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: clusterlock: Add reference counting ...................................................................... clusterlock: Add reference counting The clusterLock can now be acquired by multiple threads of execution since it is used by SDM verbs now. We need reference counting to ensure that one thread does not release the clusterLock while another thread still needs it. Change-Id: I846116ae16e88a51bdce20f97ddf22859dea3086 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/clusterlock.py 1 file changed, 55 insertions(+), 45 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/78/40378/1 diff --git a/vdsm/storage/clusterlock.py b/vdsm/storage/clusterlock.py index 86d47ad..96bba2b 100644 --- a/vdsm/storage/clusterlock.py +++ b/vdsm/storage/clusterlock.py @@ -211,6 +211,7 @@ def __init__(self, sdUUID, idsPath, leasesPath, *args): self._lock = threading.Lock() + self._clusterLockUsers = 0 self._sdUUID = sdUUID self._idsPath = idsPath self._leasesPath = leasesPath @@ -302,16 +303,22 @@ # ClusterLock. We could consider to remove it in the future but keeping it # for logging purpose is desirable. def acquireClusterLock(self, hostId): - self.log.info("Acquiring cluster lock for domain %s (id: %s)", - self._sdUUID, hostId) - self._acquire(SDM_LEASE_NAME, self.getLockDisk()) - self.log.debug("Cluster lock for domain %s successfully acquired " - "(id: %s)", self._sdUUID, hostId) + with nested(self._lock, SANLock._sanlock_lock): + self.log.info("Acquiring cluster lock for domain %s (id: %s)", + self._sdUUID, hostId) + if self._clusterLockUsers == 0: + self._acquire(SDM_LEASE_NAME, self.getLockDisk()) + self._clusterLockUsers = self._clusterLockUsers + 1 + self.log.debug("Cluster lock for domain %s successfully acquired " + "(id: %s, users: %i)", self._sdUUID, hostId, + self._clusterLockUsers) def acquireResource(self, resource, lockDisk, shared=False): - self.log.info("Acquiring resource lock for %s", resource) - self._acquire(resource, lockDisk, shared) - self.log.debug("Resource lock for %s successfully acquired", resource) + with nested(self._lock, SANLock._sanlock_lock): + self.log.info("Acquiring resource lock for %s", resource) + self._acquire(resource, lockDisk, shared) + self.log.debug("Resource lock for %s successfully acquired", + resource) def inquireClusterLock(self): resource, owners = self._inquire(SDM_LEASE_NAME, self.getLockDisk()) @@ -330,43 +337,47 @@ [owner.get("host_id") for owner in owners]) def releaseClusterLock(self): - self.log.info("Releasing cluster lock for domain %s", self._sdUUID) - self._release(SDM_LEASE_NAME, self.getLockDisk()) - self.log.debug("Cluster lock for domain %s successfully released", - self._sdUUID) + with self._lock: + self.log.info("Releasing cluster lock for domain %s", self._sdUUID) + if self._clusterLockUsers == 1: + self._release(SDM_LEASE_NAME, self.getLockDisk()) + self._clusterLockUsers = self._clusterLockUsers - 1 + self.log.debug("Cluster lock for domain %s successfully released " + "(users: %i)", self._sdUUID, self._clusterLockUsers) def releaseResource(self, resource, lockDisk): - self.log.info("Releasing resource lock for %s", resource) - self._release(resource, lockDisk) - self.log.debug("Resource lock for %s successfully released", resource) + with self._lock: + self.log.info("Releasing resource lock for %s", resource) + self._release(resource, lockDisk) + self.log.debug("Resource lock for %s successfully released", + resource) def _acquire(self, resource, lockDisk, shared=False): - with nested(self._lock, SANLock._sanlock_lock): - self.log.info("Acquiring resource %s, shared=%s", resource, shared) + self.log.info("Acquiring resource %s, shared=%s", resource, shared) - while True: - if SANLock._sanlock_fd is None: - try: - SANLock._sanlock_fd = sanlock.register() - except sanlock.SanlockException as e: - raise se.AcquireLockFailure( - self._sdUUID, e.errno, - "Cannot register to sanlock", str(e)) - + while True: + if SANLock._sanlock_fd is None: try: - sanlock.acquire(self._sdUUID, resource, lockDisk, - slkfd=SANLock._sanlock_fd, shared=shared) + SANLock._sanlock_fd = sanlock.register() except sanlock.SanlockException as e: - if e.errno != os.errno.EPIPE: - raise se.AcquireLockFailure( - self._sdUUID, e.errno, - "Cannot acquire sanlock resource", str(e)) - SANLock._sanlock_fd = None - continue + raise se.AcquireLockFailure( + self._sdUUID, e.errno, "Cannot register to sanlock", + str(e)) - break + try: + sanlock.acquire(self._sdUUID, resource, lockDisk, + slkfd=SANLock._sanlock_fd, shared=shared) + except sanlock.SanlockException as e: + if e.errno != os.errno.EPIPE: + raise se.AcquireLockFailure( + self._sdUUID, e.errno, + "Cannot acquire sanlock resource", str(e)) + SANLock._sanlock_fd = None + continue - self.log.debug("Resource %s successfully acquired", resource) + break + + self.log.debug("Resource %s successfully acquired", resource) def _inquire(self, resource, lockDisk): res = sanlock.read_resource(*lockDisk[0]) @@ -374,17 +385,16 @@ return res, owners def _release(self, resource, lockDisk): - with self._lock: - self.log.info("Releasing resource %s", resource) + self.log.info("Releasing resource %s", resource) - try: - sanlock.release(self._sdUUID, resource, lockDisk, - slkfd=SANLock._sanlock_fd) - except sanlock.SanlockException as e: - raise se.ReleaseLockFailure(resource, e) + try: + sanlock.release(self._sdUUID, resource, lockDisk, + slkfd=SANLock._sanlock_fd) + except sanlock.SanlockException as e: + raise se.ReleaseLockFailure(resource, e) - self._sanlockfd = None - self.log.debug("Resource %s successfully released", resource) + self._sanlockfd = None + self.log.debug("Resource %s successfully released", resource) class LocalLock(object): -- To view, visit https://gerrit.ovirt.org/40378 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I846116ae16e88a51bdce20f97ddf22859dea3086 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 16

Change in vdsm[master]: XXX: Insert sleep when creating block SD
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: XXX: Insert sleep when creating block SD ...................................................................... XXX: Insert sleep when creating block SD Change-Id: I10ed63d747f1353da824af8ec56ac8a6f66d666b Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/blockSD.py 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/43553/1 diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 0b8bdc7..0d5e403 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -896,6 +896,10 @@ # Create VMS file system _createVMSfs(os.path.join("/dev", vgName, MASTERLV)) + # XXX: When running an iSCSI server and initiator on the same host we + # need to wait a bit for IO to settle before deactivating LVs + import time + time.sleep(10) lvm.deactivateLVs(vgName, MASTERLV) path = lvm.lvPath(vgName, sd.METADATA) -- To view, visit https://gerrit.ovirt.org/43553 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I10ed63d747f1353da824af8ec56ac8a6f66d666b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

4 12

Change in vdsm[master]: sdm: Add decorators for sdm-only functions and their callers
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: sdm: Add decorators for sdm-only functions and their callers ...................................................................... sdm: Add decorators for sdm-only functions and their callers Change-Id: Iaf736144e5640519851dc9175b5f17539d0ce23e Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M tests/Makefile.am A tests/sdm_tests.py M vdsm.spec.in M vdsm/storage/Makefile.am A vdsm/storage/sdm.py 5 files changed, 102 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/43556/1 diff --git a/tests/Makefile.am b/tests/Makefile.am index f116fc4..5907a0d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -92,6 +92,7 @@ scheduleTests.py \ schemaTests.py \ schemaValidationTest.py \ + sdm_tests.py \ securableTests.py \ sourceroutingTests.py \ sslhelper.py \ diff --git a/tests/sdm_tests.py b/tests/sdm_tests.py new file mode 100644 index 0000000..95ae126 --- /dev/null +++ b/tests/sdm_tests.py @@ -0,0 +1,43 @@ +# +# Copyright 2015 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 testlib import VdsmTestCase + +from storage import sdm +from storage.threadLocal import vars + + +class RequireSDMTests(VdsmTestCase): + def setUp(self): + try: + delattr(vars, '__sdm__') + except AttributeError: + pass + + @sdm.sdm_verb + def test_allowed(self): + self.assertTrue(self.sdm_fn()) + + def test_denied(self): + self.assertRaises(sdm.SDMFunctionNotCallable, self.sdm_fn) + + @sdm.require_sdm + def sdm_fn(self): + return True \ No newline at end of file diff --git a/vdsm.spec.in b/vdsm.spec.in index 57e673e..24e0ab7 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -936,6 +936,7 @@ %{_datadir}/%{vdsm_name}/storage/clusterlock.py* %{_datadir}/%{vdsm_name}/storage/sdc.py* %{_datadir}/%{vdsm_name}/storage/sd.py* +%{_datadir}/%{vdsm_name}/storage/sdm.py* %{_datadir}/%{vdsm_name}/storage/securable.py* %{_datadir}/%{vdsm_name}/storage/sp.py* %{_datadir}/%{vdsm_name}/storage/spbackends.py* diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am index e460518..51768ce 100644 --- a/vdsm/storage/Makefile.am +++ b/vdsm/storage/Makefile.am @@ -59,6 +59,7 @@ resourceManager.py \ sdc.py \ sd.py \ + sdm.py \ securable.py \ sp.py \ spbackends.py \ diff --git a/vdsm/storage/sdm.py b/vdsm/storage/sdm.py new file mode 100644 index 0000000..97e599f --- /dev/null +++ b/vdsm/storage/sdm.py @@ -0,0 +1,56 @@ +# +# Copyright 2015 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 functools import wraps +from storage.threadLocal import vars + + +class SDMFunctionNotCallable(Exception): + pass + + +def require_sdm(f): + """ + SDM method decorator. + + This decorator is used to mark methods that can only be called when the + storage is operating in SDM mode (ie. without SPM). + """ + @wraps(f) + def wrapper(*args, **kwds): + if getattr(vars, '__sdm__', False): + return f(*args, **kwds) + else: + raise SDMFunctionNotCallable(f.__name__) + return wrapper + + +def sdm_verb(f): + """ + SDM verb decorator + + This decorator indicates that a function is designed to work without SPM + and is approved to access SDM-only functions. + """ + @wraps(f) + def wrapper(*args, **kwds): + vars.__sdm__ = True + return f(*args, **kwds) + return wrapper -- To view, visit https://gerrit.ovirt.org/43556 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iaf736144e5640519851dc9175b5f17539d0ce23e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

4 11

Change in vdsm[master]: sdm: Add find_domain_manifest helpers
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: sdm: Add find_domain_manifest helpers ...................................................................... sdm: Add find_domain_manifest helpers Change-Id: Ia5d139bde2349c8231141fe842d9ada96fa279fe Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M tests/Makefile.am A tests/domainmanifestfactory_tests.py M tests/manifest_tests.py M tests/storagefakelib.py M tests/storagetestutils.py M vdsm.spec.in M vdsm/storage/Makefile.am M vdsm/storage/blockSD.py A vdsm/storage/domainManifestFactory.py M vdsm/storage/glusterSD.py M vdsm/storage/localFsSD.py M vdsm/storage/nfsSD.py 12 files changed, 214 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/55/43555/1 diff --git a/tests/Makefile.am b/tests/Makefile.am index 9cc3692..f116fc4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -43,6 +43,7 @@ cpuProfileTests.py \ deviceTests.py \ domainDescriptorTests.py \ + domainmanifestfactory_tests.py \ encodingTests.py \ executorTests.py \ fileSDTests.py \ diff --git a/tests/domainmanifestfactory_tests.py b/tests/domainmanifestfactory_tests.py new file mode 100644 index 0000000..4c3f63e --- /dev/null +++ b/tests/domainmanifestfactory_tests.py @@ -0,0 +1,124 @@ +# Copyright 2015 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 +# + +import os +import uuid + +from testlib import VdsmTestCase, namedTemporaryDir +from monkeypatch import MonkeyPatchScope +from storagefakelib import FakeLVM +from storagetestutils import make_blocksd_manifest, make_vg, \ + make_filesd_manifest + +from storage import sd, blockSD, fileSD, localFsSD, mount +from storage import storage_exception as se +from storage import domainManifestFactory + + +class DomainManifestFactoryTests(VdsmTestCase): + def get_patches(self, tmpdir, lvm): + return [(blockSD, 'lvm', lvm), + (blockSD, 'selectMetadata', lambda x: {}), + (sd.StorageDomain, 'storage_repository', tmpdir), + (mount, 'isMounted', lambda x: True)] + + def make_mnt(self, tmpdir, link_name): + mntdir = os.path.join(tmpdir, sd.DOMAIN_MNT_POINT) + os.mkdir(mntdir) + os.symlink(tmpdir, os.path.join(mntdir, link_name)) + + def test_localfs_layout(self): + with namedTemporaryDir() as tmpdir: + self.make_mnt(tmpdir, '_localfssd') + lvm = FakeLVM(tmpdir) + with MonkeyPatchScope(self.get_patches(tmpdir, lvm)): + manifest = make_filesd_manifest(tmpdir) + found_manifest = domainManifestFactory.produce( + manifest.sdUUID, manifest._metadata) + self.assertEquals(manifest.sdUUID, found_manifest.sdUUID) + self.assertIsInstance(found_manifest, + fileSD.FileStorageDomainManifest) + + def test_nfs_layout(self): + with namedTemporaryDir() as tmpdir: + self.make_mnt(tmpdir, '192.168.1.1:_localfssd') + lvm = FakeLVM(tmpdir) + with MonkeyPatchScope(self.get_patches(tmpdir, lvm)): + manifest = make_filesd_manifest(tmpdir) + found_manifest = domainManifestFactory.produce( + manifest.sdUUID, manifest._metadata) + self.assertEquals(manifest.sdUUID, found_manifest.sdUUID) + self.assertIsInstance(found_manifest, + fileSD.FileStorageDomainManifest) + + def test_glusterfs_layout(self): + with namedTemporaryDir() as tmpdir: + mntdir = os.path.join(tmpdir, sd.DOMAIN_MNT_POINT) + glusterdir = os.path.join(mntdir, sd.GLUSTERSD_DIR) + os.makedirs(glusterdir) + os.symlink(tmpdir, os.path.join(glusterdir, 'host:_glustersd')) + + lvm = FakeLVM(tmpdir) + with MonkeyPatchScope(self.get_patches(tmpdir, lvm)): + manifest = make_filesd_manifest(tmpdir) + found_manifest = domainManifestFactory.produce( + manifest.sdUUID, manifest._metadata) + self.assertEquals(manifest.sdUUID, found_manifest.sdUUID) + self.assertEquals(manifest.sdUUID, found_manifest.sdUUID) + self.assertIsInstance(found_manifest, + fileSD.FileStorageDomainManifest) + + def test_block_layout(self): + with namedTemporaryDir() as tmpdir: + lvm = FakeLVM(tmpdir) + with MonkeyPatchScope(self.get_patches(tmpdir, lvm)): + manifest = make_blocksd_manifest() + vg_name = make_vg(lvm, manifest) + lvm.vgmd[vg_name]['tags'] = [blockSD.STORAGE_DOMAIN_TAG] + found_manifest = domainManifestFactory.produce( + manifest.sdUUID, manifest._metadata) + self.assertEquals(manifest.sdUUID, found_manifest.sdUUID) + self.assertIsInstance(found_manifest, + blockSD.BlockStorageDomainManifest) + + def test_cached_finder(self): + def _trap(unused): + raise RuntimeError() + + with namedTemporaryDir() as tmpdir: + self.make_mnt(tmpdir, '_localfssd') + lvm = FakeLVM(tmpdir) + patches = self.get_patches(tmpdir, lvm) + with MonkeyPatchScope(patches): + manifest = make_filesd_manifest(tmpdir) + res1 = domainManifestFactory.produce( + manifest.sdUUID, manifest._metadata) + patches.append((localFsSD, 'findDomainManifest', _trap)) + with MonkeyPatchScope(patches): + res2 = domainManifestFactory.produce( + manifest.sdUUID, manifest._metadata) + self.assertEquals(res1.sdUUID, res2.sdUUID) + + def test_not_found(self): + sdUUID = str(uuid.uuid4()) + with namedTemporaryDir() as tmpdir: + lvm = FakeLVM(tmpdir) + with MonkeyPatchScope(self.get_patches(tmpdir, lvm)): + self.assertRaises(se.StorageDomainDoesNotExist, + domainManifestFactory.produce, sdUUID) \ No newline at end of file diff --git a/tests/manifest_tests.py b/tests/manifest_tests.py index a22d7cf..967b663 100644 --- a/tests/manifest_tests.py +++ b/tests/manifest_tests.py @@ -161,7 +161,6 @@ imguuid = str(uuid.uuid4()) imagepath = manifest.getImagePath(imguuid) - os.makedirs(os.path.dirname(imagepath)) with open(imagepath, 'w') as f: f.truncate(0) self.assertEquals(set(), manifest.getAllImages()) diff --git a/tests/storagefakelib.py b/tests/storagefakelib.py index cb40fbb..bf79d91 100644 --- a/tests/storagefakelib.py +++ b/tests/storagefakelib.py @@ -23,6 +23,7 @@ from copy import deepcopy from storage import lvm as real_lvm +from storage import storage_exception as se class FakeLVM(object): @@ -154,7 +155,11 @@ return real_lvm.PV(**md) def getVG(self, vgName): - vg_md = deepcopy(self.vgmd[vgName]) + try: + vg_md = deepcopy(self.vgmd[vgName]) + except KeyError: + raise se.VolumeGroupDoesNotExist(vgName) + vg_attr = real_lvm.VG_ATTR(**vg_md['attr']) vg_md['attr'] = vg_attr return real_lvm.VG(**vg_md) diff --git a/tests/storagetestutils.py b/tests/storagetestutils.py index ed052af..eae803a 100644 --- a/tests/storagetestutils.py +++ b/tests/storagetestutils.py @@ -79,7 +79,7 @@ if metadata is None: metadata = {sd.DMDK_VERSION: 3} manifest = fileSD.FileStorageDomainManifest(domain_path, metadata) - os.makedirs(os.path.join(manifest.domaindir, sduuid, sd.DOMAIN_IMAGES)) + os.makedirs(os.path.join(manifest.domaindir, sd.DOMAIN_IMAGES)) return manifest diff --git a/vdsm.spec.in b/vdsm.spec.in index 4ba9812..57e673e 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -906,6 +906,7 @@ %{_datadir}/%{vdsm_name}/storage/curlImgWrap.py* %{_datadir}/%{vdsm_name}/storage/devicemapper.py* %{_datadir}/%{vdsm_name}/storage/dispatcher.py* +%{_datadir}/%{vdsm_name}/storage/domainManifestFactory.py %{_datadir}/%{vdsm_name}/storage/monitor.py* %{_datadir}/%{vdsm_name}/storage/fileSD.py* %{_datadir}/%{vdsm_name}/storage/fileUtils.py* diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am index a4ec93a..e460518 100644 --- a/vdsm/storage/Makefile.am +++ b/vdsm/storage/Makefile.am @@ -31,6 +31,7 @@ curlImgWrap.py \ devicemapper.py \ dispatcher.py \ + domainManifestFactory.py \ fileSD.py \ fileUtils.py \ fileVolume.py \ diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 121cc6b..6de2737 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -1426,5 +1426,10 @@ return BlockStorageDomain(BlockStorageDomain.findDomainPath(sdUUID)) +def findDomainManifest(sdUUID, metadata=None): + return BlockStorageDomainManifest( + BlockStorageDomain.findDomainPath(sdUUID), metadata) + + def getStorageDomainsList(): return [vg.name for vg in lvm.getAllVGs() if _isSD(vg)] diff --git a/vdsm/storage/domainManifestFactory.py b/vdsm/storage/domainManifestFactory.py new file mode 100644 index 0000000..fced9de --- /dev/null +++ b/vdsm/storage/domainManifestFactory.py @@ -0,0 +1,60 @@ +# +# Copyright 2015 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 +# + +import logging + +import blockSD, glusterSD, localFsSD, nfsSD +import storage_exception as se + + +log = logging.getLogger('Storage.domainManifestFactory') + +domain_manifest_producers = {} + + +def produce(sdUUID, metadata=None): + log.error("looking for domain manifest %s", sdUUID) + + # Check our cache of finder functions first. Since a domain manifest + # cannot change its type we don't have to worry about invalidation. + try: + producer = domain_manifest_producers[sdUUID] + except KeyError: + pass + else: + return producer(sdUUID, metadata) + + # The order is somewhat important, it's ordered + # by how quickly get can find the domain. For instance + # if an nfs mount is unavailable we will get stuck + # until it times out, this should affect fetching + # of block\local domains. If for any case in the future + # this changes, please update the order. + for mod in (blockSD, glusterSD, localFsSD, nfsSD): + try: + manifest = mod.findDomainManifest(sdUUID, metadata) + domain_manifest_producers[sdUUID] = mod.findDomainManifest + return manifest + except se.StorageDomainDoesNotExist: + pass + except Exception: + log.exception("Error while looking for domain `%s`", sdUUID) + + raise se.StorageDomainDoesNotExist(sdUUID) diff --git a/vdsm/storage/glusterSD.py b/vdsm/storage/glusterSD.py index 2931083..a62e242 100644 --- a/vdsm/storage/glusterSD.py +++ b/vdsm/storage/glusterSD.py @@ -30,3 +30,8 @@ def findDomain(sdUUID): return GlusterStorageDomain(GlusterStorageDomain.findDomainPath(sdUUID)) + + +def findDomainManifest(sdUUID, metadata=None): + return fileSD.FileStorageDomainManifest( + GlusterStorageDomain.findDomainPath(sdUUID), metadata) diff --git a/vdsm/storage/localFsSD.py b/vdsm/storage/localFsSD.py index 7e11330..b4e0ce0 100644 --- a/vdsm/storage/localFsSD.py +++ b/vdsm/storage/localFsSD.py @@ -119,3 +119,8 @@ def findDomain(sdUUID): return LocalFsStorageDomain(LocalFsStorageDomain.findDomainPath(sdUUID)) + + +def findDomainManifest(sdUUID, metadata=None): + return fileSD.FileStorageDomainManifest( + LocalFsStorageDomain.findDomainPath(sdUUID), metadata) \ No newline at end of file diff --git a/vdsm/storage/nfsSD.py b/vdsm/storage/nfsSD.py index 1f94439..739efc8 100644 --- a/vdsm/storage/nfsSD.py +++ b/vdsm/storage/nfsSD.py @@ -120,3 +120,8 @@ def findDomain(sdUUID): return NfsStorageDomain(NfsStorageDomain.findDomainPath(sdUUID)) + + +def findDomainManifest(sdUUID, metadata=None): + return fileSD.FileStorageDomainManifest( + NfsStorageDomain.findDomainPath(sdUUID), metadata) -- To view, visit https://gerrit.ovirt.org/43555 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia5d139bde2349c8231141fe842d9ada96fa279fe Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

4 12

Change in vdsm[master]: storage: Factor apparentsize adjustment into a reusable func...
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: storage: Factor apparentsize adjustment into a reusable function ...................................................................... storage: Factor apparentsize adjustment into a reusable function When creating a new volume sometimes the actual size is larger than the requested size due to underlying storage limitations (such as physical extent granularity in LVM). In these cases, the new volume's size must be adjusted to match the actual size. The SDM volume creation code would also like to use this function so extract it into the VolumeMetadata class. Change-Id: I7395850e127b941fc50ae8d5659868a2499b5076 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/volume.py 1 file changed, 28 insertions(+), 18 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/44/44044/1 diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py index f7e5bc0..723b3d9 100644 --- a/vdsm/storage/volume.py +++ b/vdsm/storage/volume.py @@ -495,6 +495,31 @@ puuid) return None + @classmethod + def adjust_new_volume_size(cls, dom_manifest, img_id, vol_id, req_size, + vol_format): + # When the volume format is raw what the guest sees is the apparent + # size of the file/device therefore if the requested size doesn't + # match the apparent size (eg: physical extent granularity in LVM) + # we need to update the size value so that the metadata reflects + # the correct state. + if vol_format == RAW_FORMAT: + apparentSize = int(dom_manifest.getVSize(img_id, vol_id) / + BLOCK_SIZE) + if apparentSize < req_size: + cls.log.error("The volume %s apparent size %s is smaller " + "than the requested size %s", + vol_id, apparentSize, req_size) + raise se.VolumeCreationError() + if apparentSize > req_size: + cls.log.info("The requested size for volume %s doesn't " + "match the granularity on domain %s, " + "updating the volume size from %s to %s", + vol_id, dom_manifest.sdUUID, req_size, + apparentSize) + return apparentSize + return req_size + class Volume(object): log = logging.getLogger('Storage.Volume') @@ -850,24 +875,9 @@ cls.log.error("Failed to create volume %s: %s", volPath, e) vars.task.popRecovery() raise - # When the volume format is raw what the guest sees is the apparent - # size of the file/device therefore if the requested size doesn't - # match the apparent size (eg: physical extent granularity in LVM) - # we need to update the size value so that the metadata reflects - # the correct state. - if volFormat == RAW_FORMAT: - apparentSize = int(dom.getVSize(imgUUID, volUUID) / BLOCK_SIZE) - if apparentSize < size: - cls.log.error("The volume %s apparent size %s is smaller " - "than the requested size %s", - volUUID, apparentSize, size) - raise se.VolumeCreationError() - if apparentSize > size: - cls.log.info("The requested size for volume %s doesn't " - "match the granularity on domain %s, " - "updating the volume size from %s to %s", - volUUID, sdUUID, size, apparentSize) - size = apparentSize + + size = VolumeMetadata.adjust_new_volume_size(dom, imgUUID, volUUID, + size, volFormat) vars.task.pushRecovery( task.Recovery("Create volume metadata rollback", clsModule, -- To view, visit https://gerrit.ovirt.org/44044 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7395850e127b941fc50ae8d5659868a2499b5076 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 7

Change in vdsm[master]: storage: Rename _share to share_to_image
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: storage: Rename _share to share_to_image ...................................................................... storage: Rename _share to share_to_image When a new volume is created whose parent is a template volume, that template must be "shared" into the new volume's image since volume chains only link volumes in the same image. This logic is reusable by the SDM create volume flow but _share should be renamed to share_to_image to remove the _ which designates this as a protected method. Change-Id: I44c7fe43ada38aa97bbd9c08438a0ee686680330 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/blockVolume.py M vdsm/storage/fileVolume.py M vdsm/storage/volume.py 3 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/44046/1 diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py index 47336b9..23e636e 100644 --- a/vdsm/storage/blockVolume.py +++ b/vdsm/storage/blockVolume.py @@ -314,7 +314,7 @@ def refreshVolume(self): lvm.refreshLVs(self.sdUUID, (self.volUUID,)) - def _share(self, dstImgPath): + def share_to_image(self, dstImgPath): """ Share this volume to dstImgPath """ diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py index 15ca05b..c5b8667 100644 --- a/vdsm/storage/fileVolume.py +++ b/vdsm/storage/fileVolume.py @@ -300,7 +300,7 @@ os.path.join(dstImgPath, self.volUUID)) self.oop.utils.forceLink(self._getLeaseVolumePath(), dstLeasePath) - def _share(self, dstImgPath): + def share_to_image(self, dstImgPath): """ Share this volume to dstImgPath, including the metadata and the lease """ diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py index 0d61643..951efe0 100644 --- a/vdsm/storage/volume.py +++ b/vdsm/storage/volume.py @@ -598,7 +598,7 @@ self.md.removeMetadata() def _share(self, dstImgPath): - return self.md._share(dstImgPath) + return self.md.share_to_image(dstImgPath) @classmethod def _getModuleAndClass(cls): -- To view, visit https://gerrit.ovirt.org/44046 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I44c7fe43ada38aa97bbd9c08438a0ee686680330 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 7

Change in vdsm[master]: storage: Rename getParent to getParentId
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: storage: Rename getParent to getParentId ...................................................................... storage: Rename getParent to getParentId The Volume and VolumeMetadata classes have functions to return the parent UUID and parent volume instance. It's a bit unclear that getParent returns the ID so let's make this more explicit by renaming getParent to getParentId. Change-Id: Ie310d731709ebf1ba3589e92a7ae506ccdc8ae22 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/blockVolume.py M vdsm/storage/fileVolume.py M vdsm/storage/image.py M vdsm/storage/sp.py M vdsm/storage/volume.py 5 files changed, 20 insertions(+), 20 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/47/44047/1 diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py index 23e636e..0168d64 100644 --- a/vdsm/storage/blockVolume.py +++ b/vdsm/storage/blockVolume.py @@ -167,7 +167,7 @@ def getParentMeta(self): return self.getMetaParam(volume.PUUID) - def getParent(self): + def getParentId(self): """ Return parent volume UUID """ @@ -490,7 +490,7 @@ try: # We need to blank parent record in our metadata # for parent to become leaf successfully. - puuid = self.getParent() + puuid = self.getParentId() self.setParent(volume.BLANK_UUID) if puuid and puuid != volume.BLANK_UUID: pvol = BlockVolume(self.repoPath, self.sdUUID, self.imgUUID, diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py index c5b8667..2dad91f 100644 --- a/vdsm/storage/fileVolume.py +++ b/vdsm/storage/fileVolume.py @@ -150,7 +150,7 @@ return out - def getParent(self): + def getParentId(self): """ Return parent volume UUID """ @@ -452,7 +452,7 @@ try: # We need to blank parent record in our metadata # for parent to become leaf successfully. - puuid = self.getParent() + puuid = self.getParentId() self.setParent(volume.BLANK_UUID) if puuid and puuid != volume.BLANK_UUID: pvol = FileVolume(self.repoPath, self.sdUUID, diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py index fa0770c..43e08e5 100644 --- a/vdsm/storage/image.py +++ b/vdsm/storage/image.py @@ -232,7 +232,7 @@ chain.insert(0, srcVol) seen.add(srcVol.volUUID) - parentUUID = srcVol.getParent() + parentUUID = srcVol.getParentId() if parentUUID == volume.BLANK_UUID: break @@ -578,7 +578,7 @@ raise se.VolumeNotSparse() srcVolume = self._getSparsifyVolume(tmpSdUUID, tmpImgUUID, - tmpVolume.getParent()) + tmpVolume.getParentId()) tmpVolume.prepare() try: @@ -898,7 +898,7 @@ successor = chain[-1] tmpVol = volclass(self.repoPath, sdDom.sdUUID, imgUUID, successor) dstParent = volclass(self.repoPath, sdDom.sdUUID, imgUUID, - ancestor).getParent() + ancestor).getParentId() # Mark all volumes as illegal while tmpVol and dstParent != tmpVol.volUUID: @@ -926,7 +926,7 @@ successor = chain[-1] srcVol = volclass(self.repoPath, sdUUID, imgUUID, successor) dstParent = volclass(self.repoPath, sdUUID, imgUUID, - ancestor).getParent() + ancestor).getParentId() while srcVol and dstParent != srcVol.volUUID: try: @@ -953,7 +953,7 @@ successor = chain[-1] srcVol = volclass(self.repoPath, sdDom.sdUUID, imgUUID, successor) dstParent = volclass(self.repoPath, sdDom.sdUUID, imgUUID, - ancestor).getParent() + ancestor).getParentId() while srcVol and dstParent != srcVol.volUUID: self.log.info("Remove volume %s from image %s", srcVol.volUUID, @@ -1143,12 +1143,12 @@ """ chain = [] accumulatedChainSize = 0 - endVolName = vols[ancestor].getParent() # TemplateVolName or None + endVolName = vols[ancestor].getParentId() # TemplateVolName or None currVolName = successor while (currVolName != endVolName): chain.insert(0, currVolName) accumulatedChainSize += vols[currVolName].getVolumeSize() - currVolName = vols[currVolName].getParent() + currVolName = vols[currVolName].getParentId() return accumulatedChainSize, chain @@ -1169,7 +1169,7 @@ self.log.debug("unlinking subchain: %s" % subChain) sdDom = sdCache.produce(sdUUID=sdUUID) - dstParent = sdDom.produceVolume(imgUUID, subChain[0]).getParent() + dstParent = sdDom.produceVolume(imgUUID, subChain[0]).getParentId() subChainTailVol = sdDom.produceVolume(imgUUID, subChain[-1]) if subChainTailVol.isLeaf(): self.log.debug("Leaf volume is being removed from the chain. " @@ -1247,10 +1247,10 @@ srcVolParams = srcVol.getVolumeParams() srcVolParams['children'] = [] for vName, vol in vols.iteritems(): - if vol.getParent() == successor: + if vol.getParentId() == successor: srcVolParams['children'].append(vol) dstVol = vols[ancestor] - dstParentUUID = dstVol.getParent() + dstParentUUID = dstVol.getParentId() if dstParentUUID != sd.BLANK_UUID: volParams = vols[dstParentUUID].getVolumeParams() else: diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 8574bac..b92f107 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -1838,7 +1838,7 @@ volUUID=srcVolUUID) if not srcVol.isShared(): - if srcVol.getParent() == volume.BLANK_UUID: + if srcVol.getParentId() == volume.BLANK_UUID: with rmanager.acquireResource(imageResourcesNamespace, srcImgUUID, rm.LockType.exclusive): diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py index 951efe0..691d3b9 100644 --- a/vdsm/storage/volume.py +++ b/vdsm/storage/volume.py @@ -250,7 +250,7 @@ "disktype": meta.get(DISKTYPE, ""), "voltype": meta.get(VOLTYPE, ""), "size": int(meta.get(SIZE, "0")), - "parent": self.getParent(), + "parent": self.getParentId(), "description": meta.get(DESCRIPTION, ""), "pool": meta.get(sd.DMDK_POOLS, ""), "domain": meta.get(DOMAIN, ""), @@ -310,7 +310,7 @@ volParams['size'] = self.getSize() volParams['apparentsize'] = self.getVolumeSize(bs=bs) volParams['truesize'] = self.getVolumeTrueSize(bs=bs) - volParams['parent'] = self.getParent() + volParams['parent'] = self.getParentId() volParams['descr'] = self.getDescription() volParams['legality'] = self.getLegality() return volParams @@ -494,7 +494,7 @@ """ Return parent volume object """ - puuid = self.getParent() + puuid = self.getParentId() if puuid and puuid != BLANK_UUID: return sdCache.produce(self.sdUUID).produceVolume(self.imgUUID, puuid) @@ -571,11 +571,11 @@ """ return self.md.getMetadata(metaId) - def getParent(self): + def getParentId(self): """ Return parent volume UUID """ - return self.md.getParent() + return self.md.getParentId() def getChildren(self): """ Return children volume UUIDs. -- To view, visit https://gerrit.ovirt.org/44047 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie310d731709ebf1ba3589e92a7ae506ccdc8ae22 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 7

Change in vdsm[master]: storage: Rename getParentVolume* to produceParent
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: storage: Rename getParentVolume* to produceParent ...................................................................... storage: Rename getParentVolume* to produceParent The Volume object provides getParentVolume and the VolumeMetadata object provides getParentVolumeMetadata. Both of these functions return an instance of their same type which represents this volume's parent. Since the logic is the same and both object types implement compatible interfaces let's unify around the name produceParent. Once complete, consumers of these objects can use either type without additional modifications. Change-Id: I4553e5bcc8a576abc6fb472148f744c66e5012ef Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/image.py M vdsm/storage/resourceFactories.py M vdsm/storage/volume.py 3 files changed, 16 insertions(+), 16 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/48/44048/1 diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py index 43e08e5..bd79f6e 100644 --- a/vdsm/storage/image.py +++ b/vdsm/storage/image.py @@ -168,7 +168,7 @@ """ chain = self.getChain(sdUUID, imgUUID, volUUID) newsize = 0 - template = chain[0].getParentVolume() + template = chain[0].produceParent() if template: newsize = template.getVolumeSize() for vol in chain: @@ -241,7 +241,7 @@ imgUUID, srcVol.volUUID, parentUUID) raise se.ImageIsNotLegalChain(imgUUID) - srcVol = srcVol.getParentVolume() + srcVol = srcVol.produceParent() self.log.info("sdUUID=%s imgUUID=%s chain=%s ", sdUUID, imgUUID, chain) return chain @@ -254,7 +254,7 @@ # Find all volumes of image (excluding template) chain = self.getChain(sdUUID, imgUUID) # check if the chain is build above a template, or it is a standalone - pvol = chain[0].getParentVolume() + pvol = chain[0].produceParent() if pvol: tmpl = pvol elif chain[0].isShared(): @@ -351,7 +351,7 @@ fakeTemplate = False pimg = volume.BLANK_UUID # standalone chain # check if the chain is build above a template, or it is a standalone - pvol = srcChain[0].getParentVolume() + pvol = srcChain[0].produceParent() if pvol: # find out parent volume parameters volParams = pvol.getVolumeParams() @@ -454,7 +454,7 @@ srcFormat = volume.fmt2str(srcVol.getFormat()) dstFormat = volume.fmt2str(dstVol.getFormat()) - parentVol = dstVol.getParentVolume() + parentVol = dstVol.produceParent() if parentVol is not None: backing = volume.getBackingVolumePath( @@ -731,7 +731,7 @@ raise se.ImageIsNotLegalChain(imgUUID) chain = self.getChain(sdUUID, imgUUID) # check if the chain is build above a template, or it is a standalone - pvol = chain[0].getParentVolume() + pvol = chain[0].produceParent() if pvol: if not pvol.isLegal() or pvol.isFake(): raise se.ImageIsNotLegalChain(imgUUID) @@ -902,7 +902,7 @@ # Mark all volumes as illegal while tmpVol and dstParent != tmpVol.volUUID: - vol = tmpVol.getParentVolume() + vol = tmpVol.produceParent() tmpVol.setLegality(volume.ILLEGAL_VOL) tmpVol = vol @@ -932,7 +932,7 @@ try: self.log.info("Teardown volume %s from image %s", srcVol.volUUID, imgUUID) - vol = srcVol.getParentVolume() + vol = srcVol.produceParent() srcVol.teardown(sdUUID=srcVol.sdUUID, volUUID=srcVol.volUUID, justme=True) srcVol = vol @@ -958,7 +958,7 @@ while srcVol and dstParent != srcVol.volUUID: self.log.info("Remove volume %s from image %s", srcVol.volUUID, imgUUID) - vol = srcVol.getParentVolume() + vol = srcVol.produceParent() srcVol.delete(postZero=postZero, force=True) chain.remove(srcVol.volUUID) srcVol = vol @@ -1321,7 +1321,7 @@ def _activateVolumeForImportExport(self, domain, imgUUID, volUUID=None): chain = self.getChain(domain.sdUUID, imgUUID, volUUID) - template = chain[0].getParentVolume() + template = chain[0].produceParent() if template or len(chain) > 1: self.log.error("Importing and exporting an image with more " diff --git a/vdsm/storage/resourceFactories.py b/vdsm/storage/resourceFactories.py index 98c362b..fb2522d 100644 --- a/vdsm/storage/resourceFactories.py +++ b/vdsm/storage/resourceFactories.py @@ -126,7 +126,7 @@ return [] # check if the chain is build above a template, or it is a standalone - pvol = chain[0].getParentVolume() + pvol = chain[0].produceParent() if pvol: template = pvol.volUUID elif chain[0].isShared(): diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py index 691d3b9..a098eb7 100644 --- a/vdsm/storage/volume.py +++ b/vdsm/storage/volume.py @@ -490,7 +490,7 @@ """ pass # Do not remove this method or the V3 upgrade will fail. - def getParentVolume(self): + def produceParent(self): """ Return parent volume object """ @@ -659,7 +659,7 @@ Rebase volume on top of new backing volume """ if rollback: - pvol = self.getParentVolume() + pvol = self.produceParent() if not pvol: self.log.warn("Can't rebase volume %s, parent missing", self.volUUID) @@ -787,7 +787,7 @@ "volUUID=%s imageDir=%s" % (repoPath, sdUUID, imgUUID, volUUID, imageDir)) vol = sdCache.produce(sdUUID).produceVolume(imgUUID, volUUID) - pvol = vol.getParentVolume() + pvol = vol.produceParent() # Remove volume vol.delete(postZero=False, force=True) if len(cls.getImageVolumes(repoPath, sdUUID, imgUUID)): @@ -1120,7 +1120,7 @@ try: if justme: return True - pvol = self.getParentVolume() + pvol = self.produceParent() if pvol: pvol.prepare(rw=chainrw, justme=False, chainrw=chainrw, setrw=setrw) @@ -1149,7 +1149,7 @@ def getInfo(self): return self.md.getInfo() - def getParentVolume(self): + def produceParent(self): """ Return parent volume object """ -- To view, visit https://gerrit.ovirt.org/44048 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4553e5bcc8a576abc6fb472148f744c66e5012ef Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 7

Change in vdsm[master]: ImageManifest: move getChain
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: ImageManifest: move getChain ...................................................................... ImageManifest: move getChain The implementation of getChain instantiates storagedomain and volume objects. This must be fixed. We must fix all callers of getChain to work with VolumeMetadata objects instead of Volume objects. Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/image.py 1 file changed, 71 insertions(+), 66 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/49/44049/1 diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py index bd79f6e..f852f3c 100644 --- a/vdsm/storage/image.py +++ b/vdsm/storage/image.py @@ -89,6 +89,8 @@ class ImageManifest(object): + log = logging.getLogger('Storage.ImageManifest') + def __init__(self, repoPath): self.repoPath = repoPath @@ -105,6 +107,74 @@ log.info("Create placeholder %s for image's volumes", img_dir) os.mkdir(img_dir) return img_dir + + def getChain(self, sdUUID, imgUUID, volUUID=None): + """ + Return the chain of volumes of image as a sorted list + (not including a shared base (template) if any) + """ + chain = [] + volclass = sdCache.produce(sdUUID).getVolumeClass() + + # Use volUUID when provided + if volUUID: + srcVol = volclass(self.repoPath, sdUUID, imgUUID, volUUID) + + # For template images include only one volume (the template itself) + # NOTE: this relies on the fact that in a template there is only + # one volume + if srcVol.isShared(): + return [srcVol] + + # Find all the volumes when volUUID is not provided + else: + # Find all volumes of image + uuidlist = volclass.getImageVolumes(self.repoPath, sdUUID, imgUUID) + + if not uuidlist: + raise se.ImageDoesNotExistInSD(imgUUID, sdUUID) + + srcVol = volclass(self.repoPath, sdUUID, imgUUID, uuidlist[0]) + + # For template images include only one volume (the template itself) + if len(uuidlist) == 1 and srcVol.isShared(): + return [srcVol] + + # Searching for the leaf + for vol in uuidlist: + srcVol = volclass(self.repoPath, sdUUID, imgUUID, vol) + + if srcVol.isLeaf(): + break + + srcVol = None + + if not srcVol: + self.log.error("There is no leaf in the image %s", imgUUID) + raise se.ImageIsNotLegalChain(imgUUID) + + # We have seen corrupted chains that cause endless loops here. + # https://bugzilla.redhat.com/1125197 + seen = set() + + # Build up the sorted parent -> child chain + while not srcVol.isShared(): + chain.insert(0, srcVol) + seen.add(srcVol.volUUID) + + parentUUID = srcVol.getParentId() + if parentUUID == volume.BLANK_UUID: + break + + if parentUUID in seen: + self.log.error("Image %s volume %s has invalid parent UUID %s", + imgUUID, srcVol.volUUID, parentUUID) + raise se.ImageIsNotLegalChain(imgUUID) + + srcVol = srcVol.produceParent() + + self.log.info("sdUUID=%s imgUUID=%s chain=%s ", sdUUID, imgUUID, chain) + return chain class Image: @@ -179,72 +249,7 @@ return newsize def getChain(self, sdUUID, imgUUID, volUUID=None): - """ - Return the chain of volumes of image as a sorted list - (not including a shared base (template) if any) - """ - chain = [] - volclass = sdCache.produce(sdUUID).getVolumeClass() - - # Use volUUID when provided - if volUUID: - srcVol = volclass(self.repoPath, sdUUID, imgUUID, volUUID) - - # For template images include only one volume (the template itself) - # NOTE: this relies on the fact that in a template there is only - # one volume - if srcVol.isShared(): - return [srcVol] - - # Find all the volumes when volUUID is not provided - else: - # Find all volumes of image - uuidlist = volclass.getImageVolumes(self.repoPath, sdUUID, imgUUID) - - if not uuidlist: - raise se.ImageDoesNotExistInSD(imgUUID, sdUUID) - - srcVol = volclass(self.repoPath, sdUUID, imgUUID, uuidlist[0]) - - # For template images include only one volume (the template itself) - if len(uuidlist) == 1 and srcVol.isShared(): - return [srcVol] - - # Searching for the leaf - for vol in uuidlist: - srcVol = volclass(self.repoPath, sdUUID, imgUUID, vol) - - if srcVol.isLeaf(): - break - - srcVol = None - - if not srcVol: - self.log.error("There is no leaf in the image %s", imgUUID) - raise se.ImageIsNotLegalChain(imgUUID) - - # We have seen corrupted chains that cause endless loops here. - # https://bugzilla.redhat.com/1125197 - seen = set() - - # Build up the sorted parent -> child chain - while not srcVol.isShared(): - chain.insert(0, srcVol) - seen.add(srcVol.volUUID) - - parentUUID = srcVol.getParentId() - if parentUUID == volume.BLANK_UUID: - break - - if parentUUID in seen: - self.log.error("Image %s volume %s has invalid parent UUID %s", - imgUUID, srcVol.volUUID, parentUUID) - raise se.ImageIsNotLegalChain(imgUUID) - - srcVol = srcVol.produceParent() - - self.log.info("sdUUID=%s imgUUID=%s chain=%s ", sdUUID, imgUUID, chain) - return chain + return self.manifest.getChain(sdUUID, imgUUID, volUUID) def getTemplate(self, sdUUID, imgUUID): """ -- To view, visit https://gerrit.ovirt.org/44049 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 7

Change in vdsm[master]: fix: validate volumemetadata objects
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: fix: validate volumemetadata objects ...................................................................... fix: validate volumemetadata objects Change-Id: I8330bb23c8109cee472904bde63caa75d10c0240 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/volume.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/65/44565/1 diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py index a8211a2..d8cd3fb 100644 --- a/vdsm/storage/volume.py +++ b/vdsm/storage/volume.py @@ -142,6 +142,7 @@ raise se.InvalidParameterException("imgUUID", imgUUID) if not volUUID or volUUID == BLANK_UUID: raise se.InvalidParameterException("volUUID", volUUID) + self.validate() @property def imagePath(self): @@ -176,7 +177,6 @@ def __init__(self, md): self.md = md - self.md.validate() @property def sdUUID(self): -- To view, visit https://gerrit.ovirt.org/44565 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8330bb23c8109cee472904bde63caa75d10c0240 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 6

Change in vdsm[master]: StorageDomainManifest: Move imageGarbageCollector
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: StorageDomainManifest: Move imageGarbageCollector ...................................................................... StorageDomainManifest: Move imageGarbageCollector Change-Id: I6d608bed1a7add239aca3ffe96d9e9a118799511 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/fileSD.py M vdsm/storage/sd.py 2 files changed, 23 insertions(+), 20 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/44570/1 diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py index 2508216..2e828c3 100644 --- a/vdsm/storage/fileSD.py +++ b/vdsm/storage/fileSD.py @@ -325,6 +325,20 @@ return leasePath, fileVolume.LEASE_FILEOFFSET return None, None + def imageGarbageCollector(self): + """ + Image Garbage Collector + remove the remnants of the removed images (they could be left sometimes + (on NFS mostly) due to lazy file removal + """ + removedPattern = os.path.join(self.domaindir, sd.DOMAIN_IMAGES, + sd.REMOVED_IMAGE_PREFIX + '*') + removedImages = self.oop.glob.glob(removedPattern) + self.log.debug("Removing remnants of deleted images %s" % + removedImages) + for imageDir in removedImages: + self.oop.fileUtils.cleanupdir(imageDir) + class FileStorageDomain(sd.StorageDomain): manifestClass = FileStorageDomainManifest @@ -639,20 +653,6 @@ mount.getMountFromTarget(self.mountpoint).umount() raise se.FileStorageDomainStaleNFSHandle() raise - - def imageGarbageCollector(self): - """ - Image Garbage Collector - remove the remnants of the removed images (they could be left sometimes - (on NFS mostly) due to lazy file removal - """ - removedPattern = os.path.join(self.domaindir, sd.DOMAIN_IMAGES, - sd.REMOVED_IMAGE_PREFIX + '*') - removedImages = self.oop.glob.glob(removedPattern) - self.log.debug("Removing remnants of deleted images %s" % - removedImages) - for imageDir in removedImages: - self.oop.fileUtils.cleanupdir(imageDir) def templateRelink(self, imgUUID, volUUID): """ diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py index 5635e46..1ddcd45 100644 --- a/vdsm/storage/sd.py +++ b/vdsm/storage/sd.py @@ -470,6 +470,14 @@ if preallocate is not None and preallocate not in volume.VOL_TYPE: raise se.IncorrectType(preallocate) + def imageGarbageCollector(self): + """ + Image Garbage Collector + remove the remnants of the removed images (they could be left sometimes + (on NFS mostly) due to lazy file removal + """ + pass + class StorageDomain(object): log = logging.getLogger("Storage.StorageDomain") @@ -964,9 +972,4 @@ return self._manifest.isData() def imageGarbageCollector(self): - """ - Image Garbage Collector - remove the remnants of the removed images (they could be left sometimes - (on NFS mostly) due to lazy file removal - """ - pass + self._manifest.imageGarbageCollector() -- To view, visit https://gerrit.ovirt.org/44570 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6d608bed1a7add239aca3ffe96d9e9a118799511 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 6

Change in vdsm[master]: storage: Add garbage indicator to BlockSDVol namedtuple
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: storage: Add garbage indicator to BlockSDVol namedtuple ...................................................................... storage: Add garbage indicator to BlockSDVol namedtuple Change-Id: I3e71b3d0ff7c76b815f84d895cd0cbddff251791 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/blockSD.py 1 file changed, 27 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/68/44568/1 diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 8d571a6..77e162b 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -60,7 +60,7 @@ SPECIAL_LVS = (sd.METADATA, sd.LEASES, sd.IDS, sd.INBOX, sd.OUTBOX, MASTERLV) MASTERLV_SIZE = "1024" # In MiB = 2 ** 20 = 1024 ** 2 => 1GiB -BlockSDVol = namedtuple("BlockSDVol", "name, image, parent") +BlockSDVol = namedtuple("BlockSDVol", "name, image, parent, garbage_meta_id") log = logging.getLogger("Storage.BlockSD") @@ -144,6 +144,24 @@ return f.tell() +def _get_garbage_vol_meta_id(sdUUID, volUUID, tags): + """ + If the LV contains TAG_VOL_GARBAGE then treat this volume as garbage. In + order to garbage collect the LV metadata associated with this volume we + need the metadata ID. We cannot call getMetadataId when the LV contains + TAG_VOL_UNINIT so we handle this case specially. + """ + if blockVolume.TAG_VOL_GARBAGE not in tags: + return None + for tag in tags: + if tag.startswith(blockVolume.TAG_PREFIX_MD): + meta_offset = tag[len(blockVolume.TAG_PREFIX_MD):] + return sdUUID, meta_offset + log.error("missing offset tag on volume %s/%s", sdUUID, volUUID) + raise se.VolumeMetadataReadError("missing offset tag on volume %s/%s" % + (sdUUID, volUUID)) + + def _getVolsTree(sdUUID): lvs = lvm.getLV(sdUUID) vols = {} @@ -156,7 +174,8 @@ elif tag.startswith(blockVolume.TAG_PREFIX_PARENT): parent = tag[len(blockVolume.TAG_PREFIX_PARENT):] if parent and image: - vols[lv.name] = BlockSDVol(lv.name, image, parent) + gc_meta_id = _get_garbage_vol_meta_id(sdUUID, lv.name, lv.tags) + vols[lv.name] = BlockSDVol(lv.name, image, parent, gc_meta_id) break else: if lv.name not in SPECIAL_LVS: @@ -175,6 +194,12 @@ Template self image is the 1st term in template volume entry images. """ vols = _getVolsTree(sdUUID) + + # Remove volumes awaiting garbage collection from the result + for vol_id in vols.iterkeys(): + if vols[vol_id].garbage_meta_id is not None: + del vols[vol_id] + res = {} for volName in vols.iterkeys(): res[volName] = {'imgs': [], 'parent': None} -- To view, visit https://gerrit.ovirt.org/44568 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3e71b3d0ff7c76b815f84d895cd0cbddff251791 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 6

Change in vdsm[master]: storage: block: getChildren: Do not count uninit LVs as chil...
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: storage: block: getChildren: Do not count uninit LVs as children ...................................................................... storage: block: getChildren: Do not count uninit LVs as children Any LV with the tag TAG_VOL_UNINIT is not a real volume yet. It is either in the process of being created or deleted. It should not appear in the children list of its parent until the tag is removed. Change-Id: I03e489df8478de26b01ce34f41e87aecc04512ed Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/blockVolume.py 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/44571/1 diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py index ee531f3..51f60ea 100644 --- a/vdsm/storage/blockVolume.py +++ b/vdsm/storage/blockVolume.py @@ -183,7 +183,8 @@ """ lvs = lvm.lvsByTag(self.sdUUID, "%s%s" % (TAG_PREFIX_PARENT, self.volUUID)) - return tuple(lv.name for lv in lvs) + + return tuple(lv.name for lv in lvs if TAG_VOL_UNINIT not in lv.tags) def getImage(self): """ -- To view, visit https://gerrit.ovirt.org/44571 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I03e489df8478de26b01ce34f41e87aecc04512ed Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 6

Change in vdsm[master]: Garbage volumes are not part of an image
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: Garbage volumes are not part of an image ...................................................................... Garbage volumes are not part of an image Change-Id: I8776a4b48cc8d16f420426fb28d4696b6f2bf357 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/blockVolume.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/44830/1 diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py index 9baa3d2..af11725 100644 --- a/vdsm/storage/blockVolume.py +++ b/vdsm/storage/blockVolume.py @@ -334,7 +334,7 @@ (template) """ lvs = lvm.lvsByTag(sdUUID, "%s%s" % (TAG_PREFIX_IMAGE, imgUUID)) - return [lv.name for lv in lvs] + return [lv.name for lv in lvs if TAG_VOL_GARBAGE not in lv.tags] @logskip("ResourceManager") def llPrepare(self, rw=False, setrw=False): -- To view, visit https://gerrit.ovirt.org/44830 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8776a4b48cc8d16f420426fb28d4696b6f2bf357 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 5

Change in vdsm[master]: testing: Add support for failure injection
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: testing: Add support for failure injection ...................................................................... testing: Add support for failure injection Change-Id: I30e562383fb08c4eeeeb91cca33d44a80bbff7f1 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/misc.py M vdsm/storage/storage_exception.py 2 files changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/31/44831/1 diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py index b14fd26..c53f46d 100644 --- a/vdsm/storage/misc.py +++ b/vdsm/storage/misc.py @@ -1007,3 +1007,8 @@ def deprecated(f): """Used to mark exported methods as deprecated""" return f + + +def maybe_fail(chance=0.1): + if chance != 0 and random.random() <= chance: + raise se.InjectedFailure() diff --git a/vdsm/storage/storage_exception.py b/vdsm/storage/storage_exception.py index 3a4d33f..4ca0b56 100644 --- a/vdsm/storage/storage_exception.py +++ b/vdsm/storage/storage_exception.py @@ -1724,3 +1724,8 @@ code = 855 message = ("Could not acquire resource. " "Probably resource factory threw an exception.") + + +class InjectedFailure(GeneralException): + code = 999 + message = "Injected Failure" -- To view, visit https://gerrit.ovirt.org/44831 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I30e562383fb08c4eeeeb91cca33d44a80bbff7f1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

4 6

Change in vdsm[master]: inject failures into create container
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: inject failures into create container ...................................................................... inject failures into create container Change-Id: I1fb5f18be6434ee21ac7550fe14c166b5e82b11e Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/sdm.py 1 file changed, 20 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/44832/1 diff --git a/vdsm/storage/sdm.py b/vdsm/storage/sdm.py index 7a53e04..af4d479 100644 --- a/vdsm/storage/sdm.py +++ b/vdsm/storage/sdm.py @@ -31,6 +31,8 @@ from vdsm import qemuimg +from misc import maybe_fail + log = logging.getLogger('sdm') rmanager = rm.ResourceManager.getInstance() @@ -43,40 +45,58 @@ def create_volume_container(dom_manifest, img_id, size, vol_format, disk_type, vol_id, desc, src_img_id, src_vol_id): + maybe_fail(0.005) hostId = get_domain_host_id(dom_manifest.sdUUID) + maybe_fail(0.02) dom_manifest.acquireDomainLock(hostId) imageResourcesNamespace = sd.getNamespace(dom_manifest.sdUUID, IMAGE_NAMESPACE) try: + maybe_fail(0.02) with rmanager.acquireResource(imageResourcesNamespace, img_id, rm.LockType.exclusive): + maybe_fail(0.02) image_manifest = ImageManifest(dom_manifest.getRepoPath()) + maybe_fail(0.02) img_path = image_manifest.create_image_dir(dom_manifest.sdUUID, img_id) + maybe_fail(0.02) vol_parent_md = None + maybe_fail(0.02) if src_vol_id != volume.BLANK_UUID: + maybe_fail(0.02) # When the src_img_id isn't specified we assume it's the same # as img_id if src_img_id == volume.BLANK_UUID: + maybe_fail(0.02) src_img_id = img_id + maybe_fail(0.02) vol_parent_md = dom_manifest.produceVolume(src_img_id, src_vol_id) + maybe_fail(0.02) size = vol_parent_md.getSize() + maybe_fail(0.02) dom_manifest.create_volume_artifacts(img_id, vol_id, size, vol_format, disk_type, desc, src_vol_id) + maybe_fail(0.02) if src_vol_id != volume.BLANK_UUID: + maybe_fail(0.02) _prepare_volume_for_parenthood(vol_parent_md, src_img_id, src_vol_id, img_id, img_path) + maybe_fail(0.02) # TODO: get actual size from create_volume_artifacts retval size = volume.VolumeMetadata.adjust_new_volume_size( dom_manifest, img_id, vol_id, size, vol_format) + maybe_fail(0.02) vol_path = os.path.join(img_path, vol_id) + maybe_fail(0.02) _initialize_volume_contents(img_id, vol_id, vol_path, vol_format, size, vol_parent_md) + maybe_fail(0.02) dom_manifest.commit_volume_artifacts(img_path, img_id, vol_id) finally: dom_manifest.releaseDomainLock() -- To view, visit https://gerrit.ovirt.org/44832 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1fb5f18be6434ee21ac7550fe14c166b5e82b11e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 5

Change in vdsm[master]: blockVolume: Add classmethod helper to clear a metadata slot
by alitke＠redhat.com 20 Aug '16

20 Aug '16

Adam Litke has uploaded a new change for review. Change subject: blockVolume: Add classmethod helper to clear a metadata slot ...................................................................... blockVolume: Add classmethod helper to clear a metadata slot Change-Id: I338f3fe5f232b1e6e7e8a2474efabcf56333e1cc Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/blockVolume.py 1 file changed, 7 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/33/44833/1 diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py index 79061e5..c6c3938 100644 --- a/vdsm/storage/blockVolume.py +++ b/vdsm/storage/blockVolume.py @@ -296,16 +296,17 @@ # tags self.setMetaParam(volume.IMAGE, imgUUID) - def removeMetadata(self, metaId): - """ - Just wipe meta. - """ + @classmethod + def clearMetadataSlot(cls, metaId): try: - self._putMetadata(metaId, {"NONE": "#" * (sd.METASIZE - 10)}) + cls._putMetadata(metaId, {"NONE": "#" * (sd.METASIZE - 10)}) except Exception as e: - self.log.error(e, exc_info=True) + cls.log.exception(e) raise se.VolumeMetadataWriteError("%s: %s" % (metaId, e)) + def removeMetadata(self, metaId): + self.clearMetadataSlot(metaId) + @classmethod def newVolumeLease(cls, metaId, sdUUID, volUUID): cls.log.debug("Initializing volume lease volUUID=%s sdUUID=%s, " -- To view, visit https://gerrit.ovirt.org/44833 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I338f3fe5f232b1e6e7e8a2474efabcf56333e1cc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 5

Change in vdsm[master]: vm: Cleaner vm stats thread initialization
by Nir Soffer 20 Aug '16

20 Aug '16

Nir Soffer has uploaded a new change for review. Change subject: vm: Cleaner vm stats thread initialization ...................................................................... vm: Cleaner vm stats thread initialization The vm stats thread was initialized too late, leading to unneeded checks and unclear try except blocks when trying to stop the thread. This patch changes AdvancedStatsThread so it keeps a thread instance instead of inheriting from Thread, and initialize it in Vm.__init__, so it is always available when we access it. Change-Id: I2917de42b76ee3dc8b27bdc23b33f3c984a7cc68 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M vdsm/virt/sampling.py M vdsm/virt/vm.py 2 files changed, 11 insertions(+), 18 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/39299/1 diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py index 3206b97..848bb36 100644 --- a/vdsm/virt/sampling.py +++ b/vdsm/virt/sampling.py @@ -401,7 +401,7 @@ return self._samples.last() -class AdvancedStatsThread(threading.Thread): +class AdvancedStatsThread(object): """ A thread that runs the registered AdvancedStatsFunction objects for statistic and monitoring purpose. @@ -412,9 +412,8 @@ """ Initialize an AdvancedStatsThread object """ - threading.Thread.__init__(self) self.daemon = daemon - + self._thread = None self._log = log self._stopEvent = threading.Event() self._contEvent = threading.Event() @@ -426,7 +425,7 @@ """ Register the functions listed as arguments """ - if self.isAlive(): + if self._thread is not None: raise RuntimeError("AdvancedStatsThread is started") for statsFunction in args: @@ -437,7 +436,9 @@ Start the execution of the thread and exit """ self._log.debug("Start statistics collection") - threading.Thread.start(self) + self._thread = threading.Thread(target=self._run) + self._thread.daemon = self.daemon + self._thread.start() def stop(self): """ @@ -464,7 +465,7 @@ def getLastSampleTime(self): return self._statsTime - def run(self): + def _run(self): self._log.debug("Stats thread started") self._contEvent.set() diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 28054bf..1bc04e7 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -779,7 +779,7 @@ self._initTimeRTC = long(self.conf.get('timeOffset', 0)) self._guestEvent = vmstatus.POWERING_UP self._guestEventTime = 0 - self._vmStats = None + self._vmStats = VmStatsThread(self) self._guestCpuRunning = False self._guestCpuLock = threading.Lock() self._startTime = time.time() - \ @@ -1269,7 +1269,7 @@ return toSave = self.status() toSave['startTime'] = self._startTime - if self.lastStatus != vmstatus.DOWN and self._vmStats: + if self.lastStatus != vmstatus.DOWN: guestInfo = self.guestAgent.getGuestInfo() toSave['username'] = guestInfo['username'] toSave['guestIPs'] = guestInfo['guestIPs'] @@ -1758,7 +1758,7 @@ decStats = {} try: - if self._vmStats and self._vmStats.getLastSampleTime() is not None: + if self._vmStats.getLastSampleTime() is not None: decStats = self._vmStats.get() self._setUnresponsiveIfTimeout( stats, @@ -2001,19 +2001,11 @@ return domxml.toxml() def startVmStats(self): - self._vmStats = VmStatsThread(self) self._vmStats.start() self._guestEventTime = self._startTime def stopVmStats(self): - # this is less clean that it could be, but we can get here from - # many flows and with various locks held - # (_releaseLock, _shutdownLock) - # _vmStats may be None already, and we're good with that. - try: - self._vmStats.stop() - except AttributeError: - pass + self._vmStats.stop() @staticmethod def _guestSockCleanup(sock): -- To view, visit https://gerrit.ovirt.org/39299 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2917de42b76ee3dc8b27bdc23b33f3c984a7cc68 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

3 8

Change in vdsm[master]: xmlrpc: retry when flushing socket
by Piotr Kliczewski 16 Aug '16

16 Aug '16

Piotr Kliczewski has uploaded a new change for review. Change subject: xmlrpc: retry when flushing socket ...................................................................... xmlrpc: retry when flushing socket m2crypto requires retry of write and we need to make sure that we do it once flushing data before closing a socket. In order to do so we need to move original implementation to our code base. Bug-Url: https://bugzilla.redhat.com/1261255 Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055 Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com> --- M vdsm/rpc/bindingxmlrpc.py 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/37/47637/2 diff --git a/vdsm/rpc/bindingxmlrpc.py b/vdsm/rpc/bindingxmlrpc.py index c521cd8..9f77457 100644 --- a/vdsm/rpc/bindingxmlrpc.py +++ b/vdsm/rpc/bindingxmlrpc.py @@ -25,6 +25,7 @@ import libvirt import threading import re +import socket import sys from vdsm.password import (ProtectedPassword, @@ -33,6 +34,7 @@ from vdsm import utils from vdsm import xmlrpc from vdsm.define import doneCode, errCode +from vdsm.m2cutils import SSL from vdsm.netinfo import getDeviceByIP import API from vdsm.exception import VdsmException @@ -288,7 +290,23 @@ return r def finish(self): - xmlrpc.IPXMLRPCRequestHandler.finish(self) + if not self.wfile.closed: + while True: + try: + self.wfile.flush() + break + except socket.error: + # An final socket error may have occurred here, + # such as the local error ECONNABORTED. + break + except SSL.SSLError as e: + if e.message == 'bad write retry': + continue + else: + break + + self.wfile.close() + self.rfile.close() threadLocal.client = None threadLocal.server = None threadLocal.flowID = None -- To view, visit https://gerrit.ovirt.org/47637 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I52dd6ad304a82ff6c8d3dff12a38269684abf055 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com> Gerrit-Reviewer: automation(a)ovirt.org

4 9

Change in vdsm[master]: fix remove: iterate while del
by alitke＠redhat.com 13 Aug '16

13 Aug '16

Adam Litke has uploaded a new change for review. Change subject: fix remove: iterate while del ...................................................................... fix remove: iterate while del Change-Id: I594c98cf54ed93d045b76df1c6faa5122277b19e Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/blockSD.py 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/72/44572/1 diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 74bf82d..832fb8b 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -201,8 +201,8 @@ vols = _getVolsTree(sdUUID) # Remove volumes awaiting garbage collection from the result - for vol_id in vols.iterkeys(): - if vols[vol_id].garbage_meta_id is not None: + for vol_id, vol_info in vols.items(): + if vol_info.garbage_meta_id is not None: del vols[vol_id] res = {} -- To view, visit https://gerrit.ovirt.org/44572 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I594c98cf54ed93d045b76df1c6faa5122277b19e Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

3 3

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches January 2016