Change in vdsm[master]: blockSD: Fix off-by-one error in slot allocation

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: blockSD: Fix off-by-one error in slot allocation
......................................................................

blockSD: Fix off-by-one error in slot allocation

If one slot was free between two occupied slots it was skipped during
allocation. This patch Fixes the calculation and add missing tests
cases.

Change-Id: I400d24f843dd71c233c2bb8139300129a7d1aebf
Bug-Url: https://bugzilla.redhat.com/1386732
Reported-By: nijin ashok <nashok(a)redhat.com&gt;
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
M tests/domain_manifest_test.py
M vdsm/storage/blockSD.py
2 files changed, 17 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/49/65649/1

diff --git a/tests/domain_manifest_test.py b/tests/domain_manifest_test.py
index e333d79..494ef8c 100644
--- a/tests/domain_manifest_test.py
+++ b/tests/domain_manifest_test.py
@@ -23,6 +23,9 @@
 from vdsm.storage import constants as sc
 
 from testlib import VdsmTestCase, recorded
+from testlib import make_uuid
+from testlib import expandPermutations, permutations
+
 from storagetestlib import (
     make_file_volume,
     fake_block_env,
@@ -144,19 +147,27 @@
             self.assertEquals(1024, new_manifest.phyBlkSize)
 
 
+@expandPermutations
 class BlockDomainMetadataSlotTests(VdsmTestCase):
 
-    def test_metaslot_selection(self):
+    @permutations([
+        # used_slots, free_slot
+        # Note: the first 4 slots (0-3) are reserved for domain metadata
+        ([], 4),
+        ([4], 5),
+        ([4, 6], 5),
+        ([4, 7], 5),
+    ])
+    def test_metaslot_selection(self, used_slots, free_slot):
         with fake_block_env() as env:
-            lvs = ('0b6287f0-3679-4c4d-8be5-9bbfe3ec9c1f',
-                   'ea13af29-b64a-4d1a-b35f-3e6ab15c3b04')
-            for lv, offset in zip(lvs, [4, 7]):
+            for offset in used_slots:
+                lv = make_uuid()
                 sduuid = env.sd_manifest.sdUUID
                 env.lvm.createLV(sduuid, lv, VOLSIZE / MB)
                 tag = sc.TAG_PREFIX_MD + str(offset)
                 env.lvm.addtag(sduuid, lv, tag)
             with env.sd_manifest.acquireVolumeMetadataSlot(None, 1) as mdSlot:
-                self.assertEqual(mdSlot, 5)
+                self.assertEqual(mdSlot, free_slot)
 
     def test_metaslot_lock(self):
         with fake_block_env() as env:
diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index f39d41a..f486681 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -751,7 +751,7 @@
         freeSlot = (SD_METADATA_SIZE + self.logBlkSize - 1) / self.logBlkSize
 
         for offset, size in occupiedSlots:
-            if offset - freeSlot > slotSize:
+            if offset >= freeSlot + slotSize:
                 break
 
             freeSlot = offset + size


-- 
To view, visit https://gerrit.ovirt.org/65649
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I400d24f843dd71c233c2bb8139300129a7d1aebf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;

gerrit-hooks has posted comments on this change.

Change subject: blockSD: Fix off-by-one error in slot allocation
......................................................................


Patch Set 1:

* Update Tracker::#1386732::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1386732::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/65649
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I400d24f843dd71c233c2bb8139300129a7d1aebf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

gerrit-hooks has posted comments on this change.

Change subject: blockSD: Fix off-by-one error in slot allocation
......................................................................


Patch Set 2:

* Update Tracker::#1386732::IGNORE, not relevant for Red Hat classification
* Check Bug-Url::IGNORE, not relevant for branch: master
* Check Public Bug::#1386732::OK, public bug
* Check Product::IGNORE, not relevant for branch: master
* Check TM::IGNORE, not relevant for branch: master
* Check merged to previous::IGNORE,         Not in stable branch (['ovirt-3.6',
'ovirt-4.0'])

-- 
To view, visit https://gerrit.ovirt.org/65649
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I400d24f843dd71c233c2bb8139300129a7d1aebf
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Jenkins CI RO]

Jenkins CI has posted comments on this change.

Change subject: blockSD: Fix off-by-one error in slot allocation
......................................................................


Patch Set 2: Continuous-Integration+1

Propagate review hook: Continuous Integration value inherited from patch 1

-- 
To view, visit https://gerrit.ovirt.org/65649
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I400d24f843dd71c233c2bb8139300129a7d1aebf
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Tal Nisan <tnisan(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No