Adam Litke has uploaded a new change for review.
Change subject: Live Merge: Proactively extend merge target volume
......................................................................
Live Merge: Proactively extend merge target volume
Since libvirt is not yet providing the write watermark information for
all volumes, we need to employ a workaround to support live merge on
block-based storage. When starting a merge, we can request the base
volume (merge target) to be extended to the size of the top volume
(merge source) plus one chunk to accomodate active writing. This may
cause some over-extension of the merge target volume but the upside is
that the merge should complete without active monitoring. Future
versions of oVirt will take advantage of an upcoming libvirt enhancement
to report write watermarks for all volumes which will allow on-demand
volume extension.
Change-Id: I3d483f9dfe6e19a9f9fec84950bf2ea0c7ed7946
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1155583
Signed-off-by: Adam Litke <alitke(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 29 insertions(+), 73 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/14/35614/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 1ae33b1..ab71892 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -870,8 +870,9 @@
to be extended. For the leaf volume curSize == self.apparentsize.
For internal volumes it is discovered by calling irs.getVolumeSize().
"""
- return (self.volExtensionChunk +
- ((curSize + constants.MEGAB - 1) / constants.MEGAB))
+ nextSize = (self.volExtensionChunk +
+ ((curSize + constants.MEGAB - 1) / constants.MEGAB))
+ return min(nextSize, self.truesize)
@property
def networkDev(self):
@@ -1832,69 +1833,12 @@
with self._confLock:
self.conf['timeOffset'] = newTimeOffset
- def _getMergeWriteWatermarks(self):
- drives = [drive for drive in self.getDiskDevices()
- if isVdsmImage(drive) and drive.blockDev]
- allChains = self._driveGetActualVolumeChain(drives).values()
- return dict((entry.uuid, entry.allocation)
- for chain in allChains
- for entry in chain)
-
- def _getLiveMergeExtendCandidates(self):
- ret = {}
-
- # The common case is that there are no active jobs.
- if not self.conf['_blockJobs'].values():
- return ret
-
- try:
- watermarks = self._getMergeWriteWatermarks()
- except LookupError:
- self.log.warning("Failed to look up watermark information")
- return ret
-
- for job in self.conf['_blockJobs'].values():
- try:
- drive = self._findDriveByUUIDs(job['disk'])
- except LookupError:
- # After an active layer merge completes the vdsm metadata will
- # be out of sync for a brief period. If we cannot find the old
- # disk then it's safe to skip it.
- continue
- if not drive.blockDev or drive.format != 'cow':
- continue
-
- if job['strategy'] == 'commit':
- volumeID = job['baseVolume']
- else:
- self.log.debug("Unrecognized merge strategy '%s'",
- job['strategy'])
- continue
- volSize = self.cif.irs.getVolumeSize(drive.domainID, drive.poolID,
- drive.imageID, volumeID)
- if volSize['status']['code'] != 0:
- self.log.error("Unable to get the size of volume %s (domain: "
- "%s image: %s)", volumeID, drive.domainID,
- drive.imageID)
- continue
-
- if volumeID in watermarks:
- self.log.debug("Adding live merge extension candidate: "
- "volume=%s", volumeID)
- ret[drive.imageID] = {
- 'alloc': watermarks[volumeID],
- 'physical': int(volSize['truesize']),
- 'capacity': drive.apparentsize,
- 'volumeID': volumeID}
- else:
- self.log.warning("No watermark info available for %s",
- volumeID)
- return ret
-
def _getExtendCandidates(self):
ret = []
- mergeCandidates = self._getLiveMergeExtendCandidates()
+ # FIXME: mergeCandidates should be a dictionary of candidate volumes
+ # once libvirt starts reporting watermark information for all volumes.
+ mergeCandidates = {}
for drive in self._devices[DISK_DEVICES]:
if not drive.blockDev or drive.format != 'cow':
continue
@@ -5172,7 +5116,6 @@
if res['info']['voltype'] == 'SHARED':
self.log.error("merge: Refusing to merge into a shared volume")
return errCode['mergeErr']
- baseSize = int(res['info']['apparentsize'])
# Indicate that we expect libvirt to maintain the relative paths of
# backing files. This is necessary to ensure that a volume chain is
@@ -5186,6 +5129,19 @@
# transitioned to the second phase. We must then tell libvirt to
# pivot to the new active layer (baseVolUUID).
flags |= libvirt.VIR_DOMAIN_BLOCK_COMMIT_ACTIVE
+
+ # If top is the active layer, it's allocated size is stored in
+ # drive.apparantsize.
+ topSize = drive.apparentsize
+ else:
+ # If top is an internal volume, we must call getVolumeInfo
+ res = self.cif.irs.getVolumeInfo(drive.domainID, drive.poolID,
+ drive.imageID, topVolUUID)
+ if res['status']['code'] != 0:
+ self.log.error("Unable to get volume info for '%s'",
+ topVolUUID)
+ return errCode['mergeErr']
+ topSize = int(res['info']['apparentsize'])
# Take the jobs lock here to protect the new job we are tracking from
# being cleaned up by queryBlockJobs() since it won't exist right away
@@ -5210,9 +5166,13 @@
# blockCommit will cause data to be written into the base volume.
# Perform an initial extension to ensure there is enough space to
- # start copying. The normal monitoring code will take care of any
- # future internal volume extensions that may be necessary
- self.extendDriveVolume(drive, baseVolUUID, baseSize)
+ # copy all the required data. Normally we'd use monitoring to extend
+ # the volume on-demand but internal watermark information is not being
+ # reported by libvirt so we must do the full extension up front. In
+ # the worst case, we'll need to extend 'base' to the same size as 'top'
+ # plus a bit more to accomodate additional writes to 'top' during the
+ # live merge operation.
+ self.extendDriveVolume(drive, baseVolUUID, topSize)
# Trigger the collection of stats before returning so that callers
# of getVmStats after this returns will see the new job
@@ -5240,14 +5200,10 @@
break
sourceAttr = ('file', 'dev')[drive.blockDev]
path = sourceXML.getAttribute(sourceAttr)
+
+ # TODO: Allocation information is not available in the XML. Switch
+ # to the new interface once it becomes available in libvirt.
alloc = None
- if drive.blockDev:
- allocXML = find_element_by_name(diskXML, 'allocation')
- if not allocXML:
- self.log.debug("<allocation/> missing from backing "
- "chain for block device %s", drive.name)
- break
- alloc = int(allocXML.firstChild.nodeValue)
bsXML = find_element_by_name(diskXML, 'backingStore')
if not bsXML:
self.log.warning("<backingStore/> missing from backing "
--
To view, visit http://gerrit.ovirt.org/35614
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3d483f9dfe6e19a9f9fec84950bf2ea0c7ed7946
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Hello Federico Simoncelli, Allon Mureinik, Francesco Romani,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/35768
to review the following change.
Change subject: Do not extend volume beyond truesize
......................................................................
Do not extend volume beyond truesize
When calling getNextVolumeSize on a volume that is close to fully
allocated, the function can return a value larger than the volume's real
capacity. Add an explicit ceiling to prevent this.
Change-Id: Ib616224d88c886700a7f121eaf6da84ba8276e55
Signed-off-by: Adam Litke <alitke(a)redhat.com>
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1155583
Reviewed-on: http://gerrit.ovirt.org/35757
Reviewed-by: Allon Mureinik <amureini(a)redhat.com>
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/68/35768/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 15d4620..335ade5 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1356,8 +1356,9 @@
to be extended. For the leaf volume curSize == self.apparentsize.
For internal volumes it is discovered by calling irs.getVolumeSize().
"""
- return (self.volExtensionChunk +
- ((curSize + constants.MEGAB - 1) / constants.MEGAB))
+ nextSize = (self.volExtensionChunk +
+ ((curSize + constants.MEGAB - 1) / constants.MEGAB))
+ return min(nextSize, self.truesize)
@property
def networkDev(self):
--
To view, visit http://gerrit.ovirt.org/35768
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib616224d88c886700a7f121eaf6da84ba8276e55
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Adam Litke has uploaded a new change for review.
Change subject: Do not extend volume beyond truesize
......................................................................
Do not extend volume beyond truesize
When calling getNextVolumeSize on a volume that is close to fully
allocated, the function can return a value larger than the volume's real
capacity. Add an explicit ceiling to prevent this.
Change-Id: Ib616224d88c886700a7f121eaf6da84ba8276e55
Signed-off-by: Adam Litke <alitke(a)redhat.com>
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1155583
---
M vdsm/virt/vm.py
1 file changed, 3 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/57/35757/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index adad036..a986327 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -868,8 +868,9 @@
to be extended. For the leaf volume curSize == self.apparentsize.
For internal volumes it is discovered by calling irs.getVolumeSize().
"""
- return (self.volExtensionChunk +
- ((curSize + constants.MEGAB - 1) / constants.MEGAB))
+ nextSize = (self.volExtensionChunk +
+ ((curSize + constants.MEGAB - 1) / constants.MEGAB))
+ return min(nextSize, self.truesize)
@property
def networkDev(self):
--
To view, visit http://gerrit.ovirt.org/35757
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib616224d88c886700a7f121eaf6da84ba8276e55
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Hello Federico Simoncelli, Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/35769
to review the following change.
Change subject: storage: Search only the current image for children
......................................................................
storage: Search only the current image for children
The getChildren method of FileVolume is currently searching all images
in the storage domain for children. A glob of all metadata files in the
SD is cached and then passed to grep to look for the child volumes. The
problem is that deleteVolumes takes an exclusive lock on the image
namespace only. If deletes are active on other images at the same time
we'll get errors with missing files.
The code attempts to report the children of a template base volume.
However, this is not needed now and is not expected to be required in
the future. The getVolsOfImage API should be used for templates.
Therefore, we can fix this race by limiting the search for children to a
single image.
Change-Id: I2ef9012cee3a8cb891926510c10ecc47b7cddaa1
Signed-off-by: Adam Litke <alitke(a)redhat.com>
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1069610
Reviewed-on: http://gerrit.ovirt.org/35096
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/storage/fileVolume.py
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/35769/1
diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index a8e9835..20573bd 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -361,10 +361,10 @@
def getChildren(self):
""" Return children volume UUIDs.
- Children can be found in any image of the volume SD.
+ This API is not suitable for use with a template's base volume.
"""
domPath = self.imagePath.split('images')[0]
- metaPattern = os.path.join(domPath, 'images', '*', '*.meta')
+ metaPattern = os.path.join(domPath, 'images', self.imgUUID, '*.meta')
metaPaths = oop.getProcessPool(self.sdUUID).glob.glob(metaPattern)
pattern = "%s.*%s" % (volume.PUUID, self.volUUID)
matches = grepCmd(pattern, metaPaths)
--
To view, visit http://gerrit.ovirt.org/35769
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2ef9012cee3a8cb891926510c10ecc47b7cddaa1
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Francesco Romani has uploaded a new change for review.
Change subject: tests: fix HostStatsThread tests
......................................................................
tests: fix HostStatsThread tests
In commit 4567f368bc3b initial tests for HostStatsThread
were added. Unfortunately, the patch was not rebased
before to be verified, and so it broke master.
The mistake slipped into because the patch applies cleanly
against master.
This patch fixes that in the following way
- mark test as broken: the existing behaviour is *not*
to continue in presence of errors - see
http://gerrit.ovirt.org/#/c/29401/ abandoned
for that reason.
- wrap a real HostSample to be more correct
- add a timeout to avoid hang indefinitely.
Change-Id: Iefeacf5f105384040b4a81f9835faa1fa5033c07
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M tests/samplingTests.py
1 file changed, 5 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/35764/1
diff --git a/tests/samplingTests.py b/tests/samplingTests.py
index 3c4273c..503d934 100644
--- a/tests/samplingTests.py
+++ b/tests/samplingTests.py
@@ -28,6 +28,7 @@
from vdsm import ipwrapper
import virt.sampling as sampling
+from testValidation import brokentest
from testlib import permutations, expandPermutations
from testlib import VdsmTestCase as TestCaseBase
from monkeypatch import MonkeyPatchScope
@@ -216,6 +217,7 @@
self._sampleCount = 0
self._samplingDone = threading.Event()
+ @brokentest
def testContinueWithErrors(self):
"""
bz1113948: do not give up on errors != TimeoutError
@@ -227,12 +229,13 @@
if self._sampleCount == self.STOP_SAMPLE:
self._hs.stop()
self._samplingDone.set()
- return None # sampling.HostSample(pid)
+ return sampling.HostSample(1)
with MonkeyPatchScope([(sampling, 'HostSample', WrapHostSample),
(sampling.HostStatsThread,
'SAMPLE_INTERVAL_SEC', 0.1)]):
self._hs = sampling.HostStatsThread(self.log)
self._hs.start()
- self._samplingDone.wait()
+ self._samplingDone.wait(3.0)
+ self.assertTrue(self._samplingDone.is_set())
self.assertTrue(self._sampleCount >= self.STOP_SAMPLE)
--
To view, visit http://gerrit.ovirt.org/35764
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iefeacf5f105384040b4a81f9835faa1fa5033c07
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Ido Barkan has uploaded a new change for review.
Change subject: rename networkAttrs to attrs
......................................................................
rename networkAttrs to attrs
Change-Id: I978aad73b0981785b3e7e05d94b8f7d0250fd82f
Signed-off-by: ibarkan <ibarkan(a)redhat.com>
---
M vdsm/network/api.py
1 file changed, 7 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/35689/1
diff --git a/vdsm/network/api.py b/vdsm/network/api.py
index b8b4bba..434ce3d 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -669,12 +669,12 @@
logger.debug("Applying...")
with ConfiguratorClass(options.get('_inRollback', False)) as configurator:
# Remove edited networks and networks with 'remove' attribute
- for network, networkAttrs in networks.items():
+ for network, attrs in networks.items():
if network in _netinfo.networks:
logger.debug("Removing network %r", network)
delNetwork(network, configurator=configurator, force=force,
implicitBonding=False, _netinfo=_netinfo)
- if 'remove' in networkAttrs:
+ if 'remove' in attrs:
del networks[network]
del libvirt_nets[network]
_netinfo.updateDevices()
@@ -686,11 +686,11 @@
logger.debug('Removing broken network %r', network)
_delBrokenNetwork(network, libvirt_nets[network],
configurator=configurator)
- if 'remove' in networkAttrs:
+ if 'remove' in attrs:
del networks[network]
del libvirt_nets[network]
_netinfo.updateDevices()
- elif 'remove' in networkAttrs:
+ elif 'remove' in attrs:
raise ConfigNetworkError(ne.ERR_BAD_BRIDGE, "Cannot delete "
"network %r: It doesn't exist in the "
"system" % network)
@@ -701,8 +701,8 @@
# We need to use the newest host info
_netinfo.updateDevices()
- for network, networkAttrs in networks.iteritems():
- d = dict(networkAttrs)
+ for network, attrs in networks.iteritems():
+ d = dict(attrs)
if 'bonding' in d:
d.update(_buildBondOptions(d['bonding'], bondings, _netinfo))
else:
@@ -717,7 +717,7 @@
if cne.errCode == ne.ERR_FAILED_IFUP:
logger.debug("Adding network %r failed. Running "
"orphan-devices cleanup", network)
- _emergencyNetworkCleanup(network, networkAttrs,
+ _emergencyNetworkCleanup(network, attrs,
configurator)
raise
--
To view, visit http://gerrit.ovirt.org/35689
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I978aad73b0981785b3e7e05d94b8f7d0250fd82f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan(a)redhat.com>
Ido Barkan has uploaded a new change for review.
Change subject: rename networksAdded to connectivity_check_networks
......................................................................
rename networksAdded to connectivity_check_networks
Change-Id: I452e051c1a82734097550c13c766967283386c79
Signed-off-by: ibarkan <ibarkan(a)redhat.com>
---
M vdsm/network/api.py
1 file changed, 3 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/35688/1
diff --git a/vdsm/network/api.py b/vdsm/network/api.py
index ed23ab9..b8b4bba 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -664,7 +664,7 @@
libvirt_nets = netinfo.networks()
_netinfo = netinfo.NetInfo(_netinfo=netinfo.get(
netinfo._libvirtNets2vdsm(libvirt_nets)))
- networksAdded = set()
+ connectivity_check_networks = set()
logger.debug("Applying...")
with ConfiguratorClass(options.get('_inRollback', False)) as configurator:
@@ -695,7 +695,7 @@
"network %r: It doesn't exist in the "
"system" % network)
else:
- networksAdded.add(network)
+ connectivity_check_networks.add(network)
_handleBondings(bondings, configurator)
@@ -728,7 +728,7 @@
if not clientSeen(int(options.get('connectivityTimeout',
CONNECTIVITY_TIMEOUT_DEFAULT))):
logger.info('Connectivity check failed, rolling back')
- for network in networksAdded:
+ for network in connectivity_check_networks:
# If the new added network was created on top of
# existing bond, we need to keep the bond on rollback
# flow, else we will break the new created bond.
--
To view, visit http://gerrit.ovirt.org/35688
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I452e051c1a82734097550c13c766967283386c79
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan(a)redhat.com>