Change in vdsm[master]: FakeLVM: Simplify storage of lv metadata

Adam Litke has uploaded a new change for review.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................

FakeLVM: Simplify storage of lv metadata

Nir pointed out that storing the LV metadata as a dictionary of
dictionaries (first level is vg_name and second level is lv_name) can be
error prone.  Instead, let's store in a single level dictionary using
the tuple (vg_name, lv_name) as the key.

Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Signed-off-by: Adam Litke <alitke(a)redhat.com&gt;
---
M tests/storagefakelib.py
1 file changed, 8 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/48215/1

diff --git a/tests/storagefakelib.py b/tests/storagefakelib.py
index e6e2d06..263c163 100644
--- a/tests/storagefakelib.py
+++ b/tests/storagefakelib.py
@@ -110,18 +110,17 @@
                      opened=False,
                      active=bool(activate))
 
-        vg_dict = self.lvmd.setdefault(vgName, {})
-        vg_dict[lvName] = lv_md
+        self.lvmd[(vgName, lvName)] = lv_md
         lv_count = int(self.vgmd[vgName]['lv_count']) + 1
         self.vgmd[vgName]['lv_count'] = str(lv_count)
 
     def activateLVs(self, vgName, lvNames):
         for lv in lvNames:
-            self.lvmd[vgName][lv]['active'] = True
-            self.lvmd[vgName][lv]['attr']['state'] = 'a'
+            self.lvmd[(vgName, lv)]['active'] = True
+            self.lvmd[(vgName, lv)]['attr']['state'] = 'a'
 
     def addtag(self, vg, lv, tag):
-        self.lvmd[vg][lv]['tags'] += (tag,)
+        self.lvmd[(vg, lv)]['tags'] += (tag,)
 
     def lvPath(self, vgName, lvName):
         return os.path.join(self.root, "dev", vgName, lvName)
@@ -137,15 +136,15 @@
         return real_lvm.VG(**vg_md)
 
     def _getLV(self, vgName, lvName):
-        lv_md = deepcopy(self.lvmd[vgName][lvName])
+        lv_md = deepcopy(self.lvmd[(vgName, lvName)])
         lv_attr = real_lvm.LV_ATTR(**lv_md['attr'])
         lv_md['attr'] = lv_attr
         return real_lvm.LV(**lv_md)
 
     def getLV(self, vgName, lvName=None):
         if lvName is None:
-            return [self._getLV(vgName, lv_name)
-                    for lv_name in self.lvmd[vgName].keys()]
+            return [self._getLV(vgName, lv)
+                    for (vg, lv) in self.lvmd.keys() if vg == vgName]
         else:
             return self._getLV(vgName, lvName)
 
@@ -153,7 +152,7 @@
         volpath = self.lvPath(vg_name, lv_name)
         os.makedirs(os.path.dirname(volpath))
         with open(volpath, "w") as f:
-            f.truncate(int(self.lvmd[vg_name][lv_name]['size']))
+            f.truncate(int(self.lvmd[(vg_name, lv_name)]['size']))
 
     def _create_pv(self, pv_name, vg_name, size):
         # pe_start is difficult to calculate correctly but since it's not


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;

automation(a)ovirt.org has posted comments on this change.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................


Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Adam Litke has posted comments on this change.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................


Patch Set 2:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.ovirt.org/#/c/48215/2/tests/storagefakelib.py
File tests/storagefakelib.py:

Line 147: 
Line 148:     def getLV(self, vgName, lvName=None):
Line 149:         if lvName is None:
Line 150:             return [self._getLV(vgName, lv)
Line 151:                     for (vg, lv) in self.lvmd.keys() if vg == vgName]
Should be:

    for vg, lv, in self.lvmd if vg == vgName
Line 152:         else:
Line 153:             return self._getLV(vgName, lvName)
Line 154: 
Line 155:     def fake_lv_symlink_create(self, vg_name, lv_name):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

automation(a)ovirt.org has posted comments on this change.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................


Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Adam Litke has posted comments on this change.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................


Patch Set 3: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................


FakeLVM: Simplify storage of lv metadata

Nir pointed out that storing the LV metadata as a dictionary of
dictionaries (first level is vg_name and second level is lv_name) can be
error prone.  Instead, let's store in a single level dictionary using
the tuple (vg_name, lv_name) as the key.

Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Signed-off-by: Adam Litke <alitke(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/48215
Continuous-Integration: Jenkins CI
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
M tests/storagefakelib.py
1 file changed, 8 insertions(+), 9 deletions(-)

Approvals:
  Adam Litke: Verified
  Nir Soffer: Looks good to me, approved
  Jenkins CI: Passed CI tests



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;

gerrit-hooks has posted comments on this change.

Change subject: FakeLVM: Simplify storage of lv metadata
......................................................................


Patch Set 4:

* Update tracker: IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e6c0c9af11ecdc88d294848cf244d0d639ec65d
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com&gt;
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No