New patch submitted by Igor Lvovsky (ilvovsky(a)redhat.com)
You can review this change at: http://gerrit.usersys.redhat.com/692
commit 3a09f874889cb372ee84b04a234f7b653458e26f
Author: Igor Lvovsky <ilvovsky(a)redhat.com>
Date: Tue Jul 12 15:18:45 2011 +0300
Change getChain() to return dictionary with chain itself and its template.
This change will avoid unneeded produceVolume and improve
behaviour of different flows.
Change-Id: Ife46d592171aedea087e0d6462b735cd7e0d75e0
diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index 1c70a34..102ea6c 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -104,7 +104,7 @@ class Image:
# 1. Remove template's image: Create 'fake' template instead of deleted one
# 2. Remove regular image: Remove parent-'fake' template if nobody need it already
try:
- pvol = self.getTemplate(sdUUID=sdUUID, imgUUID=imgUUID)
+ pvol = self.getChain(sdUUID, imgUUID)['template']
# 1. If we required to delete template's image that have VMs
# based on it, we should create similar 'fake' template instead
if pvol:
@@ -269,17 +269,16 @@ class Image:
return newImgUUID
- def __chainSizeCalc(self, sdUUID, imgUUID, volUUID, size):
+ def __chainSizeCalc(self, sdUUID, imgUUID, size):
"""
Compute an estimate of the whole chain size
using the sum of the actual size of the chain's volumes
"""
- chain = self.getChain(sdUUID, imgUUID, volUUID)
+ chainDict = self.getChain(sdUUID, imgUUID)
newsize = 0
- template = chain[0].getParentVolume()
- if template:
- newsize = template.getVolumeSize()
- for vol in chain:
+ if chainDict['template']:
+ newsize = chainDict['template'].getVolumeSize()
+ for vol in chainDict['chain']:
newsize += vol.getVolumeSize()
if newsize > size:
newsize = size
@@ -301,12 +300,14 @@ class Image:
newsize = int(newsize * 1.1) # allocate %10 more for cow metadata
return newsize
- def getChain(self, sdUUID, imgUUID, volUUID=None):
+ def getChain(self, sdUUID, imgUUID):
"""
- Return the chain of volumes of image as a sorted list
- (not including a shared base (template) if any)
+ Return the dictionary with chain of volumes of image as a sorted list
+ (not including a shared base (template)) and the template if any
"""
- chain = []
+ # 'chain' - chain of volumes of image as a sorted list
+ # 'template' - shared base (template) if any
+ chainDict = {'chain':[], 'template':None}
# Find all volumes of image
volclass = SDF.produce(sdUUID).getVolumeClass()
uuidlist = volclass.getImageVolumes(self.repoPath, sdUUID, imgUUID)
@@ -316,14 +317,13 @@ class Image:
srcVol = volclass(self.repoPath, sdUUID, imgUUID, uuidlist[0])
# For template image include only one volume (template itself)
if len(uuidlist) == 1 and srcVol.isShared():
- return [srcVol]
+ return {'chain':[srcVol], 'template':srcVol}
# find the leaf
for vol in uuidlist:
srcVol = volclass(self.repoPath, sdUUID, imgUUID, vol)
if srcVol.isLeaf():
- if not volUUID or volUUID == srcVol.volUUID:
- break
+ break
srcVol = None
if not srcVol:
@@ -332,38 +332,25 @@ class Image:
# Build up the sorted (parent->child) chain
while not srcVol.isShared():
- chain.insert(0, srcVol)
+ chainDict['chain'].insert(0, srcVol)
if srcVol.getParent() == volume.BLANK_UUID:
break
srcVol = srcVol.getParentVolume()
- self.log.info("sdUUID=%s imgUUID=%s chain=%s ", sdUUID, imgUUID, str(chain))
- return chain
+ if srcVol.isShared():
+ chainDict['template'] = srcVol
- def getTemplate(self, sdUUID, imgUUID):
- """
- Return template of the image
- """
- tmpl = None
- # 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()
- if pvol:
- tmpl = pvol
- elif chain[0].isShared():
- tmpl = chain[0]
-
- return tmpl
+ self.log.info("sdUUID=%s imgUUID=%s chainDict=%s ", sdUUID, imgUUID, str(chainDict))
+ return chainDict
def validate(self, srcSdUUID, dstSdUUID, imgUUID, op=MOVE_OP):
"""
Validate template on destination domain
"""
- # Find all volumes of source image
- chain = self.getChain(srcSdUUID, imgUUID)
- leafVol = chain[-1]
srcDom = SDF.produce(srcSdUUID)
+ # Find all volumes of source image
+ chainDict = self.getChain(srcSdUUID, imgUUID)
+ leafVol = chainDict['chain'][-1]
# Avoid move template's image if there is a VM based on it (except 'Backup' domain)
if op == MOVE_OP and leafVol.isShared() and not srcDom.isBackup():
chList = leafVol.getAllChildrenList(self.repoPath, srcSdUUID, imgUUID, leafVol.volUUID)
@@ -371,18 +358,16 @@ class Image:
raise se.MoveTemplateImageError(imgUUID)
# check if the chain is build above a template, or it is a standalone
- pvol = chain[0].getParentVolume()
+ pvol = chainDict['template']
if pvol: # this is a shared template based chain
- if not pvol.isShared():
- raise se.ImageIsNotLegalChain("Base image parent vol %s is not shared" % pvol.volUUID)
pimg = pvol.getImage() # pimg == template image
try:
volclass = SDF.produce(dstSdUUID).getVolumeClass()
# Validate that the destination template exists and accessible
volclass(self.repoPath, dstSdUUID, pimg, pvol.volUUID)
- except se.StorageException, e:
- self.log.error("Unexpected error", exc_info=True)
- raise se.CouldNotValideTemplateOnTargetDomain("Template %s Destination domain %s: %s" % (pimg, dstSdUUID, str(e)))
+ except se.StorageException:
+ self.log.error("Cannot validate template %s on target domain %s", pimg, dstSdUUID, exc_info=True)
+ raise se.CouldNotValideTemplateOnTargetDomain()
def __templateRelink(self, destDom, imgUUID, volUUID):
"""
@@ -473,20 +458,15 @@ class Image:
def _createTargetImage(self, destDom, srcSdUUID, imgUUID):
# Before actual data copying we need perform several operation
# such as: create all volumes, create fake template if needed, ...
- try:
- # Find all volumes of source image
- srcChain = self.getChain(srcSdUUID, imgUUID)
- except se.StorageException:
- self.log.error("Unexpected error", exc_info=True)
- raise
- except Exception, e:
- self.log.error("Unexpected error", exc_info=True)
- raise se.SourceImageActionError(imgUUID, srcSdUUID, str(e))
+
+ # Find all volumes of source image
+ srcChainDict = self.getChain(srcSdUUID, imgUUID)
+ srcChain = srcChainDict['chain']
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 = srcChainDict['template']
if pvol:
# find out parent volume parameters
volParams = pvol.getVolumeParams()
@@ -686,11 +666,9 @@ class Image:
"""
if not self.isLegal(sdUUID, imgUUID):
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()
- if pvol:
- if not pvol.isLegal() or pvol.isFake():
+ chainDict = self.getChain(sdUUID, imgUUID)
+ if chainDict['template']:
+ if not chainDict['template'].isLegal() or chainDict['template'].isFake():
raise se.ImageIsNotLegalChain(imgUUID)
def copy(self, sdUUID, vmUUID, srcImgUUID, srcVolUUID, dstImgUUID, dstVolUUID,
@@ -735,8 +713,7 @@ class Image:
# using the sum of the actual size of the chain's volumes
if volParams['volFormat'] != volume.COW_FORMAT or volParams['prealloc'] != volume.SPARSE_VOL:
raise se.IncorrectFormat(self)
- volParams['apparentsize'] = self.__chainSizeCalc(sdUUID, srcImgUUID,
- srcVolUUID, volParams['size'])
+ volParams['apparentsize'] = self.__chainSizeCalc(sdUUID, srcImgUUID, volParams['size'])
# Find out dest volume parameters
if preallocate in [volume.PREALLOCATED_VOL, volume.SPARSE_VOL]:
diff --git a/vdsm/storage/resourceFactories.py b/vdsm/storage/resourceFactories.py
index ce3ec70..07df0f0 100644
--- a/vdsm/storage/resourceFactories.py
+++ b/vdsm/storage/resourceFactories.py
@@ -106,20 +106,18 @@ class ImageResourceFactory(rm.SimpleResourceFactory):
# Get the list of the volumes
repoPath = os.path.join(self.storage_repository, dom.getPools()[0])
try:
- chain = image.Image(repoPath).getChain(sdUUID=self.sdUUID, imgUUID=resourceName)
+ chainDict = image.Image(repoPath).getChain(sdUUID=self.sdUUID, imgUUID=resourceName)
+ chain = chainDict['chain']
except se.ImageDoesNotExistInSD:
log.debug("Image %s does not exist in domain %s", resourceName, self.sdUUID)
return []
- # check if the chain is build above a template, or it is a standalone
- pvol = chain[0].getParentVolume()
- if pvol:
- template = pvol.volUUID
- elif chain[0].isShared():
- # Image of template itself,
- # with no other volumes in chain
- template = chain[0].volUUID
- del chain[:]
+ # Image of template itself,
+ # with no other volumes in chain
+ if chainDict['template']:
+ template = chainDict['template'].volUUID
+ if chainDict['template'] in chain:
+ del chain[:]
volUUIDChain = [vol.volUUID for vol in chain]
volUUIDChain.sort()
New patch submitted by Eduardo Warszawski (ewarszaw(a)redhat.com)
You can review this change at: http://gerrit.usersys.redhat.com/908
commit 68353143d2d26a0f1e6e24e972e85970b9c17c23
Author: Eduardo Warszawski <ewarszaw(a)redhat.com>
Date: Wed Sep 7 14:00:04 2011 +0300
BZ#736034 - Add metadataignore switch to pvcreate.
Change-Id: Ib040902610fd2c5b0de04de95b57f0bfbdd1e11e
diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 921cf3b..7550728 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -707,7 +707,7 @@ def _initpv(device, metadataSize=0):
metadatasize = str(metadataSize) + 'm'
cmd = ["pvcreate", "--metadatasize", metadatasize, device]
else:
- cmd = ["pvcreate", "--pvmetadatacopies", "0", device]
+ cmd = ["pvcreate", "--pvmetadatacopies", "0", "--metadataignore", "y", device]
#pvcreate on a dev that is already a PV but not in a VG returns rc = 0.
#The device is created with the new parameters.
Haim Ateya has posted comments on this change.
Change subject: [WIP]Changed vdsm-reg \ vds_bootstrap so it creates deafault bridge named engine
......................................................................
Patch Set 1: I would prefer that you didn't submit this
default bridge name engine is not good since 'engine' is for upstream while 'rhevm' is for downstream, so it can't be hard-coded.
--
To view, visit http://gerrit.usersys.redhat.com/1073
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I03ffbf6e90f7b91c54ee0f2ac98d14f9a8e3a488
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Igor Lvovsky has posted comments on this change.
Change subject: BZ#744704 - Add rollback support for complex tasks
......................................................................
Patch Set 3: (3 inline comments)
....................................................
File vdsm/storage/task.py
Line 1053: raise TypeError("recovery param %s(%s) must be Recovery object" % (repr(recovery), type(recovery)))
Line 1054: if not recovery.name:
Line 1055: raise ValueError("replaceRecoveries: name is required")
Line 1056: recovery.setOwnerTask(self)
Line 1057: rec = stubRecoveryObj
How rec=[] will help me? I need to compare rollback's name to SENTINEL_ROLLBACK.
Line 1058: try:
Line 1059: while (rec.name != SENTINEL_ROLLBACK):
Line 1060: rec = self.popRecovery()
Line 1061: rec = self.recoveries
Line 1056: recovery.setOwnerTask(self)
Line 1057: rec = stubRecoveryObj
Line 1058: try:
Line 1059: while (rec.name != SENTINEL_ROLLBACK):
Line 1060: rec = self.popRecovery()
It used as 'while' condition
Line 1061: rec = self.recoveries
Line 1062: except Exception:
Line 1063: rec = []
Line 1064: rec.append(recovery)
Line 1060: rec = self.popRecovery()
Line 1061: rec = self.recoveries
Line 1062: except Exception:
Line 1063: rec = []
Line 1064: rec.append(recovery)
I don't see any mess here, but if it bothering you I can fix this part
Line 1065: self.recoveries = rec
Line 1066: self.persist()
Line 1067:
Line 1068:
--
To view, visit http://gerrit.usersys.redhat.com/1041
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie36ac02475f3f6438a63fbcbd26730862216cebd
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Dan Kenigsberg has posted comments on this change.
Change subject: Remove redundant StoragePool.spmStarted state.
......................................................................
Patch Set 2: Looks good to me, but someone else must approve
I like it when you remove code ;-)
--
To view, visit http://gerrit.usersys/1069
To unsubscribe, visit http://gerrit.usersys/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I47f29d14bd63e9190a94b455711e23d800575f24
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo Warszawski <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Saggi Mizrahi has submitted this change and it was merged.
Change subject: isMounted() now handles / at end of export in certain distros
......................................................................
isMounted() now handles / at end of export in certain distros
Change-Id: I1cbba23b8a2d89a037ba809a86591ab33239ce24
---
M vdsm/storage/fileUtils.py
1 file changed, 8 insertions(+), 0 deletions(-)
Approvals:
Igor Lvovsky: Looks good to me, but someone else must approve
Haim Ateya: Verified
Saggi Mizrahi: Looks good to me, approved
--
To view, visit http://gerrit.usersys.redhat.com/1072
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1cbba23b8a2d89a037ba809a86591ab33239ce24
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Saggi Mizrahi has posted comments on this change.
Change subject: isMounted() now handles / at end of export in certain distros
......................................................................
Patch Set 2: Looks good to me, approved
--
To view, visit http://gerrit.usersys.redhat.com/1072
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cbba23b8a2d89a037ba809a86591ab33239ce24
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya(a)redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>