Change in vdsm[master]: setNumberOfCpus marshalling issue
by Roy Golan
Roy Golan has posted comments on this change.
Change subject: setNumberOfCpus marshalling issue
......................................................................
Patch Set 1: Code-Review-1
you need to fix client/vdsClient.py as well
--
To view, visit http://gerrit.ovirt.org/32414
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie95c23c15bdb9b4932cd36b6f60bf84149c76454
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Roy Golan <rgolan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9 years, 8 months
Change in vdsm[master]: setNumberOfCpus marshalling issue
by Roy Golan
Roy Golan has posted comments on this change.
Change subject: setNumberOfCpus marshalling issue
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/32414
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie95c23c15bdb9b4932cd36b6f60bf84149c76454
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Roy Golan <rgolan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
9 years, 8 months
Change in vdsm[ovirt-3.5]: tool: fix pyflakes errors[1].
by mtayer＠redhat.com
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/32409
to review the following change.
Change subject: tool: fix pyflakes errors[1].
......................................................................
tool: fix pyflakes errors[1].
recently I broke master at change 31293.
[1]
./lib/vdsm/tool/configurators/libvirt.py:29: 'NotRootError' imported but unused
./lib/vdsm/tool/configurators/sanlock.py:24: 'NotRootError' imported but unused
Change-Id: I8e86dd38e3a871d55ffc5cb67d5f4596ec4dc3a6
Signed-off-by: Mooli Tayer <mtayer(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/32256
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/tool/configurators/libvirt.py
M lib/vdsm/tool/configurators/sanlock.py
2 files changed, 0 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/32409/1
diff --git a/lib/vdsm/tool/configurators/libvirt.py b/lib/vdsm/tool/configurators/libvirt.py
index 0f88b32..1b7556b 100644
--- a/lib/vdsm/tool/configurators/libvirt.py
+++ b/lib/vdsm/tool/configurators/libvirt.py
@@ -27,7 +27,6 @@
from vdsm.config import config
from .. import \
- NotRootError, \
service, \
validate_ovirt_certs
from . import \
diff --git a/lib/vdsm/tool/configurators/sanlock.py b/lib/vdsm/tool/configurators/sanlock.py
index 0c847a3..1c2b3dd 100644
--- a/lib/vdsm/tool/configurators/sanlock.py
+++ b/lib/vdsm/tool/configurators/sanlock.py
@@ -21,7 +21,6 @@
import grp
import pwd
-from .. import NotRootError
from .import \
CONFIGURED, \
InvalidConfig, \
--
To view, visit http://gerrit.ovirt.org/32409
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8e86dd38e3a871d55ffc5cb67d5f4596ec4dc3a6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
9 years, 8 months
Change in vdsm[ovirt-3.5]: vdsm-tool: changing root checking in configurator.
by mtayer＠redhat.com
Hello Yaniv Bronhaim, Dima Kuznetsov, Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/32408
to review the following change.
Change subject: vdsm-tool: changing root checking in configurator.
......................................................................
vdsm-tool: changing root checking in configurator.
Checking for root as we currently do in configure of libvirt and
sanlock is not enough. Currently this will fail during the
isConfigured() check since it does not have premissions to the
files it attempts to check:
"OSError: No such file or directory: /etc/libvirt/libvirtd.conf"
It is easy to see that this check is needed for all the exposed
methods of configurator. I'm suggesting to do it in the exposed
verbs in a uniform manner.
3.5 note: This commit broke master and it must be merged along
with change I8e86dd38e3a871d55ffc5cb67d5f4596ec4dc3a6.
Change-Id: I52967e30f677e4537b83c2db442963e3eadecb55
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1136897
Signed-off-by: Mooli Tayer <mtayer(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31293
Reviewed-by: Dima Kuznetsov <dkuznets(a)redhat.com>
Reviewed-by: Yaniv Bronhaim <ybronhei(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/tool/__init__.py
M lib/vdsm/tool/configurator.py
M lib/vdsm/tool/configurators/libvirt.py
M lib/vdsm/tool/configurators/sanlock.py
4 files changed, 20 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/32408/1
diff --git a/lib/vdsm/tool/__init__.py b/lib/vdsm/tool/__init__.py
index 008e41f..b9dd9ac 100644
--- a/lib/vdsm/tool/__init__.py
+++ b/lib/vdsm/tool/__init__.py
@@ -16,6 +16,8 @@
#
# Refer to the README and COPYING files for full details of the license
#
+import functools
+import os
class expose(object):
@@ -27,6 +29,15 @@
return fun
+def requiresRoot(func):
+ @functools.wraps(func)
+ def wrapper(*args, **kwargs):
+ if os.getuid() != 0:
+ raise NotRootError()
+ func(*args, **kwargs)
+ return wrapper
+
+
class UsageError(RuntimeError):
""" Raise on runtime when usage is invalid """
diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index 7d3752e..9160d75 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -22,7 +22,11 @@
import sys
import traceback
-from . import service, expose, UsageError
+from . import \
+ service, \
+ expose, \
+ UsageError, \
+ requiresRoot
from .configurators import \
certificates, \
CONFIGURED, \
@@ -41,6 +45,7 @@
@expose("configure")
+@requiresRoot
def configure(*args):
"""
configure [-h|...]
@@ -87,6 +92,7 @@
@expose("is-configured")
+@requiresRoot
def isconfigured(*args):
"""
is-configured [-h|...]
@@ -122,6 +128,7 @@
@expose("validate-config")
+@requiresRoot
def validate_config(*args):
"""
validate-config [-h|...]
@@ -144,6 +151,7 @@
@expose("remove-config")
+@requiresRoot
def remove_config(*args):
"""
Remove vdsm configuration from conf files
diff --git a/lib/vdsm/tool/configurators/libvirt.py b/lib/vdsm/tool/configurators/libvirt.py
index 86f24e4..0f88b32 100644
--- a/lib/vdsm/tool/configurators/libvirt.py
+++ b/lib/vdsm/tool/configurators/libvirt.py
@@ -61,9 +61,6 @@
return ["vdsmd", "supervdsmd", "libvirtd"]
def configure(self):
- if os.getuid() != 0:
- raise NotRootError()
-
self._sysvToUpstart()
if utils.isOvirtNode():
diff --git a/lib/vdsm/tool/configurators/sanlock.py b/lib/vdsm/tool/configurators/sanlock.py
index 9225252..0c847a3 100644
--- a/lib/vdsm/tool/configurators/sanlock.py
+++ b/lib/vdsm/tool/configurators/sanlock.py
@@ -46,9 +46,6 @@
"""
Configure sanlock process groups
"""
- if os.getuid() != 0:
- raise NotRootError()
-
rc, out, err = utils.execCmd(
(
'/usr/sbin/usermod',
--
To view, visit http://gerrit.ovirt.org/32408
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I52967e30f677e4537b83c2db442963e3eadecb55
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: mooli tayer <mtayer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Dima Kuznetsov <dkuznets(a)redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
9 years, 8 months
Change in vdsm[ovirt-3.5]: Live Merge: add reconcileVolumeChain verb
by alitke＠redhat.com
Hello Piotr Kliczewski, Federico Simoncelli, Francesco Romani,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/32328
to review the following change.
Change subject: Live Merge: add reconcileVolumeChain verb
......................................................................
Live Merge: add reconcileVolumeChain verb
If a live merge operation is catastrophically interrupted (we completely
lose the host on which the VM was running during the merge) engine must
have a way to discover what happened by examining storage only.
When the active layer was being merged, it's critical to know whether
the the pivot completed or not. Choosing the wrong leaf volume could
result in data corruption. When an internal volume is being merged the
situation is less dire but providing a way for engine to resolve the
final status of the merge provides for a substantially better user
experience since the snapshot can be unlocked to allow a cold merge to
be attempted.
This new verb must run on SPM and is only valid for images which are not
in use by running VMs. It will use qemu-img to examine the actual
volume chain and will correct the vdsm metadata if needed. Finally, the
current volume chain is returned. This chain can be used by engine to
understand what happened with the interrupted merge command.
Change-Id: Ia8927a42d2bc9adcf7c6b26babb327a00e91e49e
Signed-off-by: Adam Litke <alitke(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31788
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M client/vdsClient.py
M vdsm/API.py
M vdsm/rpc/BindingXMLRPC.py
M vdsm/rpc/Bridge.py
M vdsm/rpc/vdsmapi-schema.json
M vdsm/storage/hsm.py
M vdsm/storage/image.py
M vdsm/storage/sp.py
8 files changed, 163 insertions(+), 23 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/32328/1
diff --git a/client/vdsClient.py b/client/vdsClient.py
index 2c09b28..e06137d 100644
--- a/client/vdsClient.py
+++ b/client/vdsClient.py
@@ -1374,6 +1374,17 @@
return ret['status']['code'], ret['status']['message']
+ def reconcileVolumeChain(self, args):
+ if len(args) != 4:
+ raise ValueError('Wrong number of parameters')
+
+ ret = self.s.reconcileVolumeChain(*args)
+ if 'volumes' in ret:
+ for v in ret['volumes']:
+ print v
+ return 0, ''
+ return ret['status']['code'], ret['status']['message']
+
def moveMultiImage(self, args):
spUUID = args[0]
srcDomUUID = args[1]
@@ -2465,6 +2476,10 @@
'<spUUID> <sdUUID> <imgUUID> [<volUUID>]',
'Teardown an image, releasing the prepared volumes.'
)),
+ 'reconcileVolumeChain': (serv.reconcileVolumeChain, (
+ '<spUUID> <sdUUID> <imgUUID> <leafVolUUID>',
+ 'Reconcile an image volume chain and return the current chain.'
+ )),
'moveMultiImage': (serv.moveMultiImage,
('<spUUID> <srcDomUUID> <dstDomUUID> '
'<imgList>({imgUUID=postzero,'
diff --git a/vdsm/API.py b/vdsm/API.py
index fc02b01..6ea76a5 100644
--- a/vdsm/API.py
+++ b/vdsm/API.py
@@ -888,6 +888,10 @@
methodArgs, callback, self._spUUID, self._sdUUID, self._UUID,
volUUID)
+ def reconcileVolumeChain(self, leafVolUUID):
+ return self._irs.reconcileVolumeChain(self._spUUID, self._sdUUID,
+ self._UUID, leafVolUUID)
+
class LVMVolumeGroup(APIBase):
ctorArgs = ['lvmvolumegroupID']
diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py
index 12a1b49..dc85fe8 100644
--- a/vdsm/rpc/BindingXMLRPC.py
+++ b/vdsm/rpc/BindingXMLRPC.py
@@ -696,6 +696,10 @@
image = API.Image(imgUUID, spUUID, sdUUID)
return image.teardown(volUUID)
+ def imageReconcileVolumeChain(self, spUUID, sdUUID, imgUUID, leafUUID):
+ image = API.Image(imgUUID, spUUID, sdUUID)
+ return image.reconcileVolumeChain(leafUUID)
+
def poolConnect(self, spUUID, hostID, scsiKey, msdUUID, masterVersion,
domainsMap=None, options=None):
pool = API.StoragePool(spUUID)
@@ -1038,6 +1042,7 @@
(self.imageDownload, 'downloadImage'),
(self.imagePrepare, 'prepareImage'),
(self.imageTeardown, 'teardownImage'),
+ (self.imageReconcileVolumeChain, 'reconcileVolumeChain'),
(self.poolConnect, 'connectStoragePool'),
(self.poolConnectStorageServer, 'connectStorageServer'),
(self.poolCreate, 'createStoragePool'),
diff --git a/vdsm/rpc/Bridge.py b/vdsm/rpc/Bridge.py
index 4c5204d..7a7494b 100644
--- a/vdsm/rpc/Bridge.py
+++ b/vdsm/rpc/Bridge.py
@@ -397,6 +397,7 @@
'Image_download': {'ret': 'uuid'},
'Image_mergeSnapshots': {'ret': 'uuid'},
'Image_move': {'ret': 'uuid'},
+ 'Image_reconcileVolumeChain': {'ret': 'volumes'},
'ISCSIConnection_discoverSendTargets': {'ret': 'fullTargets'},
'LVMVolumeGroup_create': {'ret': 'uuid'},
'LVMVolumeGroup_getInfo': {'ret': 'info'},
diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json
index c62513f..db441f5 100644
--- a/vdsm/rpc/vdsmapi-schema.json
+++ b/vdsm/rpc/vdsmapi-schema.json
@@ -4267,6 +4267,29 @@
'data': {'storagepoolID': 'UUID', 'storagedomainID': 'UUID',
'imageID': 'UUID', '*volumeID': 'UUID'}}
+##
+# @Image.reconcileVolumeChain:
+#
+# Reconcile an image volume chain and return the current chain.
+#
+# @storagepoolID: The UUID of the Storage Pool associated with the Image
+#
+# @storagedomainID: The UUID of the Storage Domain associated with the Image
+#
+# @imageID: The UUID of the Image
+#
+# @leafVolID: The UUID of the original leaf volume
+#
+# Returns:
+# A list of volume UUIDs representing the current volume chain
+#
+# Since: 4.16.0
+##
+{'command': {'class': 'Image', 'name': 'reconcileVolumeChain'},
+ 'data': {'storagepoolID': 'UUID', 'storagedomainID': 'UUID',
+ 'imageID': 'UUID', 'leafVolID': 'UUID'}
+ 'returns': ['UUID']}
+
## Category: @LVMVolumeGroup ###################################################
##
# @LVMVolumeGroup:
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index b6972af..5d0f1b6 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1812,36 +1812,34 @@
volumes. In this case we update the chain in the metadata LV only.
The LV tag will be fixed when the unlinked volume is deleted by an SPM.
"""
+ argsStr = ("sdUUID=%s, imgUUID=%s, volUUID=%s, newChain=%s" %
+ (sdUUID, imgUUID, volUUID, newChain))
+ vars.task.setDefaultException(se.StorageException("%s" % argsStr))
sdDom = sdCache.produce(sdUUID=sdUUID)
repoPath = os.path.join(self.storage_repository, sdDom.getPools()[0])
- img = image.Image(repoPath)
imageResourcesNamespace = sd.getNamespace(sdUUID, IMAGE_NAMESPACE)
with rmanager.acquireResource(imageResourcesNamespace, imgUUID,
rm.LockType.shared):
- curChain = img.getChain(sdUUID, imgUUID, volUUID)
- subChain = []
- for vol in curChain:
- if vol.volUUID not in newChain:
- subChain.insert(0, vol.volUUID)
- elif len(subChain) > 0:
- break
- self.log.debug("unlinking subchain: %s" % subChain)
- if len(subChain) == 0:
- return
+ image.Image(repoPath).syncVolumeChain(sdUUID, imgUUID, volUUID,
+ newChain)
- dstParent = sdDom.produceVolume(imgUUID, subChain[0]).getParent()
- subChainTailVol = sdDom.produceVolume(imgUUID, subChain[-1])
- if subChainTailVol.isLeaf():
- self.log.debug("Leaf volume is being removed from the chain. "
- "Marking it ILLEGAL to prevent data corruption")
- subChainTailVol.setLegality(volume.ILLEGAL_VOL)
- else:
- for childID in subChainTailVol.getChildren():
- self.log.debug("Setting parent of volume %s to %s",
- childID, dstParent)
- sdDom.produceVolume(imgUUID, childID). \
- setParentMeta(dstParent)
+ @public
+ def reconcileVolumeChain(self, spUUID, sdUUID, imgUUID, leafVolUUID):
+ """
+ In some situations (such as when a live merge is interrupted), the
+ vdsm volume chain could become out of sync with the actual chain as
+ understood by qemu. This API uses qemu-img to determine the correct
+ chain and synchronizes vdsm metadata accordingly. Returns the correct
+ volume chain. NOT for use on images of running VMs.
+ """
+ argsStr = ("spUUID=%s, sdUUID=%s, imgUUID=%s, leafVolUUID=%s" %
+ (spUUID, sdUUID, imgUUID, leafVolUUID))
+ vars.task.setDefaultException(se.StorageException("%s" % argsStr))
+ pool = self.getPool(spUUID)
+ sdCache.produce(sdUUID=sdUUID)
+ vars.task.getSharedLock(STORAGE, sdUUID)
+ return pool.reconcileVolumeChain(sdUUID, imgUUID, leafVolUUID)
@public
def mergeSnapshots(self, sdUUID, spUUID, vmUUID, imgUUID, ancestor,
diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index 37ee135..70f59a6 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -1056,6 +1056,78 @@
return accumulatedChainSize, chain
+ def syncVolumeChain(self, sdUUID, imgUUID, volUUID, actualChain):
+ """
+ Fix volume metadata to reflect the given actual chain. This function
+ is used to correct the volume chain linkage after a live merge.
+ """
+ curChain = self.getChain(sdUUID, imgUUID, volUUID)
+ subChain = []
+ for vol in curChain:
+ if vol.volUUID not in actualChain:
+ subChain.insert(0, vol.volUUID)
+ elif len(subChain) > 0:
+ break
+ if len(subChain) == 0:
+ return
+ self.log.debug("unlinking subchain: %s" % subChain)
+
+ sdDom = sdCache.produce(sdUUID=sdUUID)
+ dstParent = sdDom.produceVolume(imgUUID, subChain[0]).getParent()
+ subChainTailVol = sdDom.produceVolume(imgUUID, subChain[-1])
+ if subChainTailVol.isLeaf():
+ self.log.debug("Leaf volume is being removed from the chain. "
+ "Marking it ILLEGAL to prevent data corruption")
+ subChainTailVol.setLegality(volume.ILLEGAL_VOL)
+ else:
+ for childID in subChainTailVol.getChildren():
+ self.log.debug("Setting parent of volume %s to %s",
+ childID, dstParent)
+ sdDom.produceVolume(imgUUID, childID). \
+ setParentMeta(dstParent)
+
+ def reconcileVolumeChain(self, sdUUID, imgUUID, leafVolUUID):
+ """
+ Discover and return the actual volume chain of an offline image
+ according to the qemu-img info command and synchronize volume metadata.
+ """
+ # Prepare volumes
+ dom = sdCache.produce(sdUUID)
+ allVols = dom.getAllVolumes()
+ imgVolumes = sd.getVolsOfImage(allVols, imgUUID).keys()
+ dom.activateVolumes(imgUUID, imgVolumes)
+
+ # Walk the volume chain using qemu-img. Not safe for running VMs
+ actualVolumes = []
+ volUUID = leafVolUUID
+ while volUUID is not None:
+ actualVolumes.insert(0, volUUID)
+ vol = dom.produceVolume(imgUUID, volUUID)
+ qemuImgFormat = volume.fmt2str(vol.getFormat())
+ imgInfo = qemuimg.info(vol.volumePath, qemuImgFormat)
+ backingFile = imgInfo.get('backingfile')
+ if backingFile is not None:
+ volUUID = os.path.basename(backingFile)
+ else:
+ volUUID = None
+
+ # A merge of the active layer has copy and pivot phases.
+ # During copy, data is copied from the leaf into its parent. Writes
+ # are mirrored to both volumes. So even after copying is complete the
+ # volumes will remain consistent. Finally, the VM is pivoted from the
+ # old leaf to the new leaf and mirroring to the old leaf ceases. During
+ # mirroring and before pivoting, we mark the old leaf ILLEGAL so we
+ # know it's safe to delete in case the operation is interrupted.
+ vol = dom.produceVolume(imgUUID, leafVolUUID)
+ if vol.getLegality() == volume.ILLEGAL_VOL:
+ actualVolumes.remove(leafVolUUID)
+
+ # Now that we know the correct volume chain, sync the storge metadata
+ self.syncVolumeChain(sdUUID, imgUUID, actualVolumes[-1], actualVolumes)
+
+ dom.deactivateImage(imgUUID)
+ return actualVolumes
+
def merge(self, sdUUID, vmUUID, imgUUID, ancestor, successor, postZero):
"""Merge source volume to the destination volume.
'successor' - source volume UUID
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 01fcc5f..323f878 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -1707,6 +1707,28 @@
image.Image(self.poolPath).multiMove(
srcDomUUID, dstDomUUID, imgDict, vmUUID, force)
+ def reconcileVolumeChain(self, sdUUID, imgUUID, leafVolUUID):
+ """
+ Determines the actual volume chain for an offline image and returns it.
+ If the actual chain differs from storage metadata, the metadata is
+ corrected to reflect the actual chain.
+
+ :param sdUUID: The UUID of the storage domain that contains the image.
+ :type sdUUID: UUID
+ :param imgUUID: The UUID of the image to be checked.
+ :type imgUUID: UUID
+ :param leafVolUUID: The UUID of the last known leaf volume.
+ :type leafVolUUID: UUID
+ :returns: A dict with a list of volume UUIDs in the corrected chain
+ :rtype: dict
+ """
+ imageResourcesNamespace = sd.getNamespace(sdUUID, IMAGE_NAMESPACE)
+ with rmanager.acquireResource(imageResourcesNamespace, imgUUID,
+ rm.LockType.exclusive):
+ img = image.Image(self.poolPath)
+ chain = img.reconcileVolumeChain(sdUUID, imgUUID, leafVolUUID)
+ return dict(volumes=chain)
+
def mergeSnapshots(self, sdUUID, vmUUID, imgUUID, ancestor, successor,
postZero):
"""
--
To view, visit http://gerrit.ovirt.org/32328
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia8927a42d2bc9adcf7c6b26babb327a00e91e49e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
9 years, 8 months
Change in vdsm[ovirt-3.5]: virt: Resolve snapshot type after prepareVolumePath
by alitke＠redhat.com
Hello Federico Simoncelli, Francesco Romani,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/32327
to review the following change.
Change subject: virt: Resolve snapshot type after prepareVolumePath
......................................................................
virt: Resolve snapshot type after prepareVolumePath
The Drive.blockDev property depends on the presence of the volume path
locally. Therefore, it is not safe to call until after the new snapshot
volume path has been prepared. Use a lambda function to delay the call
until the information is needed.
Change-Id: Ia4726702eb329bf65f1f9c658067f7a01c2179e4
Signed-off-by: Adam Litke <alitke(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/32066
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 7 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/27/32327/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 94a6b76..9e4437a 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -4221,7 +4221,7 @@
snap = xml.dom.minidom.Element('domainsnapshot')
disks = xml.dom.minidom.Element('disks')
newDrives = {}
- snapTypes = {}
+ vmDrives = {}
if self.isMigrating():
return errCode['migInProgress']
@@ -4261,7 +4261,10 @@
newDrives[vmDevName]["poolID"] = vmDrive.poolID
newDrives[vmDevName]["name"] = vmDevName
newDrives[vmDevName]["format"] = "cow"
- snapTypes[vmDevName] = ('file', 'block')[vmDrive.blockDev]
+
+ # We need to keep track of the drive object because we cannot
+ # safely access the blockDev property until after prepareVolumePath
+ vmDrives[vmDevName] = vmDrive
# If all the drives are the current ones, return success
if len(newDrives) == 0:
@@ -4284,8 +4287,9 @@
_rollbackDrives(preparedDrives)
return errCode['snapshotErr']
+ snapType = 'block' if vmDrives[vmDevName].blockDev else 'file'
snapelem = _diskSnapshot(vmDevName, newDrives[vmDevName]["path"],
- snapTypes[vmDevName])
+ snapType)
disks.appendChild(snapelem)
snap.appendChild(disks)
--
To view, visit http://gerrit.ovirt.org/32327
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia4726702eb329bf65f1f9c658067f7a01c2179e4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
9 years, 8 months
Change in vdsm[ovirt-3.5]: Live Merge: Suspend disk stats collection during pivot
by alitke＠redhat.com
Hello Federico Simoncelli, Francesco Romani,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/32326
to review the following change.
Change subject: Live Merge: Suspend disk stats collection during pivot
......................................................................
Live Merge: Suspend disk stats collection during pivot
When pivoting to the new volume after an active layer merge, there is a
brief period where the vdsm metadata is out of sync. During this time,
the disk stats collection thread will raise an exception since the drive
cannot be referred to by its old path. Therefore we disable stats
collection during this short window until the metadata can be
synchronized.
Change-Id: I08689aec4d61871a568a6f92661b560fbf4d0b57
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1109918
Signed-off-by: Adam Litke <alitke(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31367
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 19 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/26/32326/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 8c18fd3..94a6b76 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -5931,11 +5931,25 @@
self.drive.imageID,
self.drive['volumeID'], newVols)
+ # A pivot changes the top volume being used for the VM Disk. Until
+ # we can correct our metadata following the pivot we should not
+ # attempt to collect disk stats.
+ # TODO: Stop collection only for the live merge disk
+ self.vm.stopDisksStatsCollection()
+
self.vm.log.info("Requesting pivot to complete active layer commit "
"(job %s)", self.jobId)
- flags = libvirt.VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT
- if self.vm._dom.blockJobAbort(self.drive.name, flags) != 0:
- raise RuntimeError("pivot failed")
+ try:
+ flags = libvirt.VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT
+ ret = self.vm._dom.blockJobAbort(self.drive.name, flags)
+ except:
+ self.vm.startDisksStatsCollection()
+ raise
+ else:
+ if ret != 0:
+ self.vm.log.error("Pivot failed for job %s (rc=%i)",
+ self.jobId, ret)
+ raise RuntimeError("pivot failed")
self.vm.log.info("Pivot completed (job %s)", self.jobId)
@utils.traceback()
@@ -5945,6 +5959,8 @@
self.vm.log.info("Synchronizing volume chain after live merge "
"(job %s)", self.jobId)
self.vm._syncVolumeChain(self.drive)
+ if self.doPivot:
+ self.vm.startDisksStatsCollection()
self.success = True
self.vm.log.info("Synchronization completed (job %s)", self.jobId)
--
To view, visit http://gerrit.ovirt.org/32326
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I08689aec4d61871a568a6f92661b560fbf4d0b57
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
9 years, 8 months
Change in vdsm[ovirt-3.5]: Live Merge: Simplify pivot flow
by alitke＠redhat.com
Hello Federico Simoncelli, Francesco Romani,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/32325
to review the following change.
Change subject: Live Merge: Simplify pivot flow
......................................................................
Live Merge: Simplify pivot flow
The pivot part of a Live Merge can be made a synchronous operation which
simplifies our flows. We're already doing cleanup in a thread so just
do the pivot and wait for it (very short) to complete and then continue
directly to the volume chain sync part of the cleanup.
This is much simpler than doing the pivot and waiting another sample
interval to try the sync portion.
Change-Id: I5e6a9ea7096b5d81f418754d411f135450bf572e
Signed-off-by: Adam Litke <alitke(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31849
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 42 insertions(+), 39 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/25/32325/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 34023bc..8c18fd3 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -5606,8 +5606,8 @@
return False
def queryBlockJobs(self):
- def startCleanup(job, drive):
- t = LiveMergeCleanupThread(self, job['jobID'], drive)
+ def startCleanup(job, drive, needPivot):
+ t = LiveMergeCleanupThread(self, job['jobID'], drive, needPivot)
t.start()
self._liveMergeCleanupThreads[job['jobID']] = t
@@ -5620,9 +5620,9 @@
jobID = storedJob['jobID']
cleanThread = self._liveMergeCleanupThreads.get(jobID)
if cleanThread and cleanThread.isSuccessful():
- # Handle successfully cleaned jobs early because the job
- # just needs to be untracked and the stored disk info might
- # be stale anyway (ie. after active layer commit).
+ # Handle successful jobs early because the job just needs
+ # to be untracked and the stored disk info might be stale
+ # anyway (ie. after active layer commit).
self.untrackBlockJob(jobID)
continue
@@ -5645,28 +5645,25 @@
entry['bandwidth'] = liveInfo['bandwidth']
entry['cur'] = str(liveInfo['cur'])
entry['end'] = str(liveInfo['end'])
- if self._activeLayerCommitReady(liveInfo):
- try:
- self.handleBlockJobEvent(jobID, drive, 'pivot')
- except Exception:
- # Just log it. We will retry next time
- self.log.error("Pivot failed for job %s", jobID)
+ doPivot = self._activeLayerCommitReady(liveInfo)
else:
# Libvirt has stopped reporting this job so we know it will
# never report it again.
+ doPivot = False
storedJob['gone'] = True
+ if not liveInfo or doPivot:
if not cleanThread:
# There is no cleanup thread so the job must have just
# ended. Spawn an async cleanup.
- startCleanup(storedJob, drive)
+ startCleanup(storedJob, drive, doPivot)
elif cleanThread.isAlive():
# Let previously started cleanup thread continue
self.log.debug("Still waiting for block job %s to be "
- "cleaned up", jobID)
+ "synchronized", jobID)
elif not cleanThread.isSuccessful():
# At this point we know the thread is not alive and the
# cleanup failed. Retry it with a new thread.
- startCleanup(storedJob, drive)
+ startCleanup(storedJob, drive, doPivot)
jobsRet[jobID] = entry
return jobsRet
@@ -5876,18 +5873,6 @@
if x['volumeID'] in volumes]
device['volumeChain'] = drive.volumeChain = newChain
- def handleBlockJobEvent(self, jobID, drive, mode):
- if mode == 'finished':
- self.log.info("Live merge job completed (job %s)", jobID)
- self._syncVolumeChain(drive)
- elif mode == 'pivot':
- self.log.info("Requesting pivot to complete active layer commit "
- "(job %s)", jobID)
- flags = libvirt.VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT
- self._dom.blockJobAbort(drive.name, flags)
- else:
- raise RuntimeError("Invalid mode: '%s'" % mode)
-
def _initLegacyConf(self):
self.conf['displayPort'] = GraphicsDevice.LIBVIRT_PORT_AUTOSELECT
self.conf['displaySecurePort'] = \
@@ -5925,30 +5910,48 @@
class LiveMergeCleanupThread(threading.Thread):
- def __init__(self, vm, jobId, drive):
+ def __init__(self, vm, jobId, drive, doPivot):
threading.Thread.__init__(self)
self.setDaemon(True)
self.vm = vm
self.jobId = jobId
self.drive = drive
+ self.doPivot = doPivot
self.success = False
+
+ def tryPivot(self):
+ # We call imageSyncVolumeChain which will mark the current leaf
+ # ILLEGAL. We do this before requesting a pivot so that we can
+ # properly recover the VM in case we crash. At this point the
+ # active layer contains the same data as its parent so the ILLEGAL
+ # flag indicates that the VM should be restarted using the parent.
+ newVols = [vol['volumeID'] for vol in self.drive.volumeChain
+ if vol['volumeID'] != self.drive.volumeID]
+ self.vm.cif.irs.imageSyncVolumeChain(self.drive.domainID,
+ self.drive.imageID,
+ self.drive['volumeID'], newVols)
+
+ self.vm.log.info("Requesting pivot to complete active layer commit "
+ "(job %s)", self.jobId)
+ flags = libvirt.VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT
+ if self.vm._dom.blockJobAbort(self.drive.name, flags) != 0:
+ raise RuntimeError("pivot failed")
+ self.vm.log.info("Pivot completed (job %s)", self.jobId)
@utils.traceback()
def run(self):
- self.vm.log.info("Starting live merge cleanup for job %s",
- self.jobId)
- try:
- self.vm.handleBlockJobEvent(self.jobId, self.drive, 'finished')
- except Exception:
- self.vm.log.warning("Cleanup failed for live merge job %s",
- self.jobId)
- raise
- else:
- self.success = True
- self.vm.log.info("Cleanup completed for live merge job %s",
- self.jobId)
+ if self.doPivot:
+ self.tryPivot()
+ self.vm.log.info("Synchronizing volume chain after live merge "
+ "(job %s)", self.jobId)
+ self.vm._syncVolumeChain(self.drive)
+ self.success = True
+ self.vm.log.info("Synchronization completed (job %s)", self.jobId)
def isSuccessful(self):
+ """
+ Returns True if this phase completed successfully.
+ """
return self.success
--
To view, visit http://gerrit.ovirt.org/32325
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5e6a9ea7096b5d81f418754d411f135450bf572e
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
9 years, 8 months
Change in vdsm[ovirt-3.5]: Live Merge: Mark active layer ILLEGAL before pivoting
by alitke＠redhat.com
Hello Nir Soffer, Federico Simoncelli, Francesco Romani,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/32324
to review the following change.
Change subject: Live Merge: Mark active layer ILLEGAL before pivoting
......................................................................
Live Merge: Mark active layer ILLEGAL before pivoting
A block commit of the active layer takes place in two phases. First,
data is copied from the current leaf to its parent. During this time,
all writes to the disk are mirrored to both volumes. When this process
converges the two volumes are and will remain identical due to ongoing
drive mirroring by qemu. At this point we instruct libvirt to perform a
pivot from the current leaf to the new leaf. Once this is complete we
synchronize our metadata and end the job.
If we are catastrophically interrupted (lose contact with the host)
between the pivot request and final metadata synchronization there is
currently no way to know whether the pivot actually happened. This
could be dangerous because restarting the VM on another host with the
wrong leaf volume could lead to data corruption.
The solution is to mark the current leaf ILLEGAL before requesting a
pivot. Since the volumes are identical at this point it will always be
safe to rerun the VM using the new leaf. A subsequent patch will take
advantage of this ILLEGAL marker to provide engine with a reliable and
correct volume chain for this recovery scenario.
Change-Id: I97ce8fbdfad7c81181a206f160ffdd647c552d4d
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1127294
Signed-off-by: Adam Litke <alitke(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/31787
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/storage/hsm.py
1 file changed, 11 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/32324/1
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index d743759..b6972af 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1831,9 +1831,17 @@
return
dstParent = sdDom.produceVolume(imgUUID, subChain[0]).getParent()
- children = sdDom.produceVolume(imgUUID, subChain[-1]).getChildren()
- for childID in children:
- sdDom.produceVolume(imgUUID, childID).setParentMeta(dstParent)
+ subChainTailVol = sdDom.produceVolume(imgUUID, subChain[-1])
+ if subChainTailVol.isLeaf():
+ self.log.debug("Leaf volume is being removed from the chain. "
+ "Marking it ILLEGAL to prevent data corruption")
+ subChainTailVol.setLegality(volume.ILLEGAL_VOL)
+ else:
+ for childID in subChainTailVol.getChildren():
+ self.log.debug("Setting parent of volume %s to %s",
+ childID, dstParent)
+ sdDom.produceVolume(imgUUID, childID). \
+ setParentMeta(dstParent)
@public
def mergeSnapshots(self, sdUUID, spUUID, vmUUID, imgUUID, ancestor,
--
To view, visit http://gerrit.ovirt.org/32324
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I97ce8fbdfad7c81181a206f160ffdd647c552d4d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
9 years, 8 months
Change in vdsm[ovirt-3.5]: Live Merge: Extend internal block volumes during merge
by alitke＠redhat.com
Hello Federico Simoncelli, Francesco Romani,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/32323
to review the following change.
Change subject: Live Merge: Extend internal block volumes during merge
......................................................................
Live Merge: Extend internal block volumes during merge
Libvirt will be exposing the high write watermark for internal volumes
directly in the domain XML. An <allocation/> element will be added to
each member of the image chain. This patch implements the missing logic
for _getMergeWriteWatermarks using this new API.
Change-Id: I3a9e0ebdb9c42df713c40e0fc5782945eb7228a8
Signed-off-by: Adam Litke <alitke(a)redhat.com>
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1109920
Reviewed-on: http://gerrit.ovirt.org/31268
Reviewed-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 34 insertions(+), 9 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/23/32323/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 0811723..34023bc 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -215,7 +215,8 @@
pass
-VolumeChainEntry = namedtuple('VolumeChainEntry', ['uuid', 'path'])
+VolumeChainEntry = namedtuple('VolumeChainEntry',
+ ['uuid', 'path', 'allocation'])
class VmStatsThread(sampling.AdvancedStatsThread):
@@ -2481,18 +2482,34 @@
self.conf['timeOffset'] = newTimeOffset
def _getMergeWriteWatermarks(self):
- # TODO: Adopt the future libvirt API when the following RFE is done
- # https://bugzilla.redhat.com/show_bug.cgi?id=1041569
- return {}
+ 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 = {}
- watermarks = self._getMergeWriteWatermarks()
+
+ # 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():
- if 'done' in job:
- # Skip active layer commits that are in the cleanup phase
+ 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
- drive = self._findDriveByUUIDs(job['disk'])
if not drive.blockDev or drive.format != 'cow':
continue
@@ -5765,13 +5782,21 @@
break
sourceAttr = ('file', 'dev')[drive.blockDev]
path = sourceXML.getAttribute(sourceAttr)
- entry = VolumeChainEntry(pathToVolID(drive, path), path)
+ 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 "
"chain for drive %s", drive.name)
break
diskXML = bsXML
+ entry = VolumeChainEntry(pathToVolID(drive, path), path, alloc)
volChain.insert(0, entry)
return volChain or None
--
To view, visit http://gerrit.ovirt.org/32323
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3a9e0ebdb9c42df713c40e0fc5782945eb7228a8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
9 years, 8 months
