Change in vdsm[master]: upgrade: reallocate the metadata slots when needed

[Federico Simoncelli]

Federico Simoncelli has uploaded a new change for review.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

upgrade: reallocate the metadata slots when needed

Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Signed-off-by: Federico Simoncelli 
---
M vdsm/storage/imageRepository/formatConverter.py
1 file changed, 28 insertions(+), 2 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/60/9660/1

diff --git a/vdsm/storage/imageRepository/formatConverter.py b/vdsm/storage/imageRepository/formatConverter.py
index cd32a62..175f943 100644
--- a/vdsm/storage/imageRepository/formatConverter.py
+++ b/vdsm/storage/imageRepository/formatConverter.py
@@ -29,6 +29,9 @@
 from storage import volume

+BLKVOL_MAX_OFFSET = sd.LEASES_SIZE - blockVolume.RESERVED_LEASES - 1
+
+
 def __convertDomainMetadataToTags(domain, targetVersion):
     log = logging.getLogger('Storage.DomainMetadataToTags')

@@ -131,11 +134,31 @@
         # see volume.setrw for more details.
         vol._setrw(True)

+    def v3ReallocateMetadataSlot(domain, vol):
+        metaOffset = vol.getMetaOffset()
+
+        if metaOffset < BLKVOL_MAX_OFFSET:
+            return
+
+        log.debug("Reallocating metadata slot for volume %s", vol.volUUID)
+        newMetaOffset = domain.getFreeMetadataSlot()
+
+        metaContent = vol.getMetadata()
+        log.debug("Copying the metadata for volume %s to the new slot %s",
+                  vol.volUUID, newMetaOffset)
+        vol.createMetadata((domain.sdUUID, newMetaOffset), metaContent)
+
+        log.debug("Switching the metadata slot for volume %s to %s",
+                  vol.volUUID, newMetaOffset)
+        vol.changeVolumeTag(TAG_PREFIX_MD, newMetaOffset);
+
     try:
         if isMsd:
             log.debug("Acquiring the cluster lock for domain %s with "
                       "host id: %s", domain.sdUUID, hostId)
             newClusterLock.acquire(hostId)
+
+        isBlockDomain = domain.getStorageType() in sd.BLOCK_DOMAIN_TYPES

allVolumes = domain.getAllVolumes()
         allImages = set()
@@ -149,6 +172,10 @@
             # The first imgUUID is the imgUUID of the template or the
             # only imgUUID where the volUUID appears.
             vol = domain.produceVolume(imgUUIDs[0], volUUID)
+
+            if isBlockDomain:
+                v3ReallocateMetadataSlot(domain, vol)
+
             v3UpgradeVolumePermissions(vol)

log.debug("Creating the volume lease for %s", volUUID)
@@ -214,8 +241,7 @@
                   "version %s for domain %s", currentVersion, targetVersion,
                   domain.sdUUID)

-        if (currentVersion not in blockSD.VERS_METADATA_TAG
-                        and domain.getStorageType() in sd.BLOCK_DOMAIN_TYPES):
+        if (isBlockDomain and currentVersion not in blockSD.VERS_METADATA_TAG):
             __convertDomainMetadataToTags(domain, targetVersion)
         else:
             domain.setMetaParam(sd.DMDK_VERSION, targetVersion)

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 1: (1 inline comment)

....................................................
File vdsm/storage/imageRepository/formatConverter.py
Line 133:         # Using the internal call to skip the domain V3 validation,
Line 134:         # see volume.setrw for more details.
Line 135:         vol._setrw(True)
Line 136: 
Line 137:     def v3ReallocateMetadataSlot(domain, vol):
Actually now that I think of it we probably need two different reallocations depending on the domain version (V1/V2).
Line 138:         metaOffset = vol.getMetaOffset()
Line 139: 
Line 140:         if metaOffset < BLKVOL_MAX_OFFSET:
Line 141:             return

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli

Ayal Baron has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 4: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/206/ (2/2)

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/240/ (1/2)

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 5: Verified

Verified on a storage domain containing _remove_me volumes and broken images.

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 5: I would prefer that you didn't submit this

Build Unstable

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/206/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/240/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 6:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/207/ (1/2)

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 6:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/241/ (2/2)

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 6: Verified

Verified on a storage domain containing _remove_me volumes and broken images.

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 6:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/207/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/241/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 6: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

Saggi Mizrahi has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 6: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/imageRepository/formatConverter.py
Line 156: 
Line 157:             if metaOffset < metaMaxSlot:
Line 158:                 continue
Line 159: 
Line 160:             log.debug("Reallocating metadata slot %s for volume %s",
This happens per volume, on domains with a lot of volumes this will flood the log with useless junk.
Only log this information in case something fails.

instead of
log.debug("Domain something on vol '%s'", volId)
change to:
try:
   doSomething()
except:
   log.error("Something failed for vol '%s'", volId)
Line 161:                       metaOffset, vol.volUUID)
Line 162:             metaContent = vol.getMetadata()
Line 163: 
Line 164:             with vol._tagCreateLock:

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 6: (1 inline comment)

....................................................
File vdsm/storage/imageRepository/formatConverter.py
Line 156: 
Line 157:             if metaOffset < metaMaxSlot:
Line 158:                 continue
Line 159: 
Line 160:             log.debug("Reallocating metadata slot %s for volume %s",
I disagree.  Upgrade rarely happens and when it does you want to be able to track what's going on.  Moving the md is an important part of it.  Even if we have a few hundred lines of this, it's no big deal.
An LVM VG cannot really have many more than that anyway so this shouldn't be an issue.
Line 161:                       metaOffset, vol.volUUID)
Line 162:             metaContent = vol.getMetadata()
Line 163: 
Line 164:             with vol._tagCreateLock:

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 6: (1 inline comment)

....................................................
File vdsm/storage/imageRepository/formatConverter.py
Line 156: 
Line 157:             if metaOffset < metaMaxSlot:
Line 158:                 continue
Line 159: 
Line 160:             log.debug("Reallocating metadata slot %s for volume %s",
Please consider that it's log.debug. Try to run any software with debugging enabled and look at how much output (aka "useless junk") you'll get.
Line 161:                       metaOffset, vol.volUUID)
Line 162:             metaContent = vol.getMetadata()
Line 163: 
Line 164:             with vol._tagCreateLock:

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

Patch Set 6:

Satisfied with Ayal and Federico's replies to Saggi's log concerns.

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: upgrade: reallocate the metadata slots when needed
......................................................................

upgrade: reallocate the metadata slots when needed

Few vdsm releases (4.9 prior 496c0c3, BZ#732980) generated metadata
offsets higher than 1947 (LEASES_SIZE - RESERVED_LEASES - 1).
This patch reallocates such slots to free ones in order to use the
same offsets for the volume resource leases.

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=882276
Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Signed-off-by: Federico Simoncelli 
---
M vdsm/storage/blockSD.py
M vdsm/storage/imageRepository/formatConverter.py
2 files changed, 70 insertions(+), 6 deletions(-)

Approvals:
  Ayal Baron: Looks good to me, approved
  Federico Simoncelli: Verified
  Dan Kenigsberg:

Objections:
  Saggi Mizrahi: I would prefer that you didn't submit this

--
To view, visit http://gerrit.ovirt.org/9660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I15b6a85969a10b5e2b852031fc510aabcc2d5276
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Saggi Mizrahi 
Gerrit-Reviewer: oVirt Jenkins CI Server