Change in vdsm[master]: [WIP] Create storage domain using command type 1

Yeela Kaplan has uploaded a new change for review.

Change subject: [WIP] Create storage domain using command type 1
......................................................................

[WIP] Create storage domain using command type 1

All bootstrap operaions are executed using command type 1.

Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com&gt;
---
M vdsm/storage/blockSD.py
M vdsm/storage/lvm.py
2 files changed, 15 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/23646/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 55bd796..7980c80 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -517,7 +517,7 @@
 
         # Create metadata service volume
         metasize = cls.metaSize(vgName)
-        lvm.createLV(vgName, sd.METADATA, "%s" % (metasize))
+        lvm.createLV(vgName, sd.METADATA, "%s" % (metasize), safe=False)
         # Create the mapping right now so the index 0 is guaranteed
         # to belong to the metadata volume. Since the metadata is at
         # least SDMETADATA/METASIZE units, we know we can use the first
@@ -526,11 +526,11 @@
         mapping = cls.getMetaDataMapping(vgName)
 
         # Create the rest of the BlockSD internal volumes
-        lvm.createLV(vgName, sd.LEASES, sd.LEASES_SIZE)
-        lvm.createLV(vgName, sd.IDS, sd.IDS_SIZE)
-        lvm.createLV(vgName, sd.INBOX, sd.INBOX_SIZE)
-        lvm.createLV(vgName, sd.OUTBOX, sd.OUTBOX_SIZE)
-        lvm.createLV(vgName, MASTERLV, MASTERLV_SIZE)
+        lvm.createLV(vgName, sd.LEASES, sd.LEASES_SIZE, safe=False)
+        lvm.createLV(vgName, sd.IDS, sd.IDS_SIZE, safe=False)
+        lvm.createLV(vgName, sd.INBOX, sd.INBOX_SIZE, safe=False)
+        lvm.createLV(vgName, sd.OUTBOX, sd.OUTBOX_SIZE, safe=False)
+        lvm.createLV(vgName, MASTERLV, MASTERLV_SIZE, safe=False)
 
         # Create VMS file system
         _createVMSfs(os.path.join("/dev", vgName, MASTERLV))
@@ -591,7 +591,7 @@
         # Mark VG with Storage Domain Tag
         try:
             lvm.replaceVGTag(vgName, STORAGE_UNREADY_DOMAIN_TAG,
-                             STORAGE_DOMAIN_TAG)
+                             STORAGE_DOMAIN_TAG, safe=False)
         except se.StorageException:
             raise se.VolumeGroupUninitialized(vgName)
 
diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 932d69e..0f96df6 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -257,7 +257,7 @@
 
             return self._extraCfg
 
-    def _addExtraCfg(self, cmd, devices=tuple(), safe):
+    def _addExtraCfg(self, cmd, devices=tuple(), safe=True):
         newcmd = [constants.EXT_LVM, cmd[0]]
         if devices:
             conf = _buildConfig(devices)
@@ -656,6 +656,7 @@
     globals()["_current_lvmconf"] =
_current_lvmconf.replace("locking_type=4",
                                                              "locking_type=1")
     log.debug("### _current_lvmconf %s",
globals()["_current_lvmconf"])
+
 
 def bootstrap(refreshlvs=()):
     """
@@ -1061,7 +1062,7 @@
 
 
 def createLV(vgName, lvName, size, activate=True, contiguous=False,
-             initialTag=None):
+             initialTag=None, safe=True):
     """
     Size units: MB (1024 ** 2 = 2 ** 20)B.
     """
@@ -1078,7 +1079,7 @@
     if initialTag is not None:
         cmd.extend(("--addtag", initialTag))
     cmd.extend(("--name", lvName, vgName))
-    rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))
+    rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )), safe)
 
     if rc == 0:
         _lvminfo._invalidatevgs(vgName)
@@ -1280,7 +1281,7 @@
     return os.path.exists(lvPath(vgName, lvName))
 
 
-def changeVGTags(vgName, delTags=(), addTags=()):
+def changeVGTags(vgName, delTags=(), addTags=(), safe=True):
     delTags = set(delTags)
     addTags = set(addTags)
     if delTags.intersection(addTags):
@@ -1296,7 +1297,7 @@
         cmd.extend(("--addtag", tag))
 
     cmd.append(vgName)
-    rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )))
+    rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )), safe)
     _lvminfo._invalidatevgs(vgName)
     if rc != 0:
         raise se.VolumeGroupReplaceTagError(
@@ -1321,8 +1322,8 @@
         raise se.VolumeGroupRemoveTagError(vgName)
 
 
-def replaceVGTag(vg, oldTag, newTag):
-    changeVGTags(vg, [oldTag], [newTag])
+def replaceVGTag(vg, oldTag, newTag, safe=True):
+    changeVGTags(vg, [oldTag], [newTag], safe)
 
 
 def addVGTags(vgName, tags):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;

Change in vdsm[master]: Create storage domain using command type 1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Create storage domain using command type 1
......................................................................


Patch Set 8:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6938/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7728/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7840/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Create storage domain using command type 1

Yeela Kaplan has posted comments on this change.

Change subject: Create storage domain using command type 1
......................................................................


Patch Set 7:

(6 comments)

http://gerrit.ovirt.org/#/c/23646/7/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 132:     return LVM_ENC_ESCAPE.sub(lambda c: unichr(int(c.groups()[0])), s)
Line 133: 
Line 134: 
Line 135: def encodeVgTags(tagsDict):
Line 136:         tags = [VGTagMetadataRW.METADATA_TAG_PREFIX +
>  More efficient will be to build the tuple directly. 
Done
Line 137:                 lvmTagEncode("%s=%s" % (k, v))
Line 138:                 for k, v in tagsDict.items()]
Line 139:         return tuple(tags)
Line 140: 


Line 570:         #         no one reads it from here anyway
Line 571:         initialMetadata = {
Line 572:             sd.DMDK_VERSION: version,
Line 573:             sd.DMDK_SDUUID: sdUUID,
Line 574:             sd.DMDK_TYPE: BLOCK_SD_MD_FIELDS[sd.DMDK_TYPE][1](storageType),
>  Add a comment please explaining the use of the MD encoding please.
Done
Line 575:             sd.DMDK_CLASS: BLOCK_SD_MD_FIELDS[sd.DMDK_CLASS][1](domClass),
Line 576:             sd.DMDK_DESCRIPTION: domainName,
Line 577:             sd.DMDK_ROLE: sd.REGULAR_DOMAIN,
Line 578:             sd.DMDK_POOLS: BLOCK_SD_MD_FIELDS[sd.DMDK_POOLS][1]([]),


Line 599: 
Line 600:         # Mark VG with Storage Domain Tag
Line 601:         try:
Line 602:             lvm.replaceVGTag(vgName, STORAGE_UNREADY_DOMAIN_TAG,
Line 603:                              STORAGE_DOMAIN_TAG, safe=False)
>  safe=False? IMHO it is not required anymore (previous version). Unify
> all t 
Done
Line 604:         except se.StorageException:
Line 605:             raise se.VolumeGroupUninitialized(vgName)
Line 606: 
Line 607:         bsd = BlockStorageDomain(sdUUID)


http://gerrit.ovirt.org/#/c/23646/7/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 1052: # Public Logical volume interface
Line 1053: #
Line 1054: 
Line 1055: 
Line 1056: def _completeCreateLV(rc, vgName, lvName, activate=True):
>  I really dislike the "complete" part of the name.
Done
Line 1057:     if rc == 0:
Line 1058:         _lvminfo._invalidatevgs(vgName)
Line 1059:         _lvminfo._invalidatelvs(vgName, lvName)
Line 1060:     else:


Line 1090:     rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )), rw=True)
Line 1091:     _completeCreateLV(rc, vgName, lvName)
Line 1092: 
Line 1093: 
Line 1094: def createLV(vgName, lvName, size, activate=True, contiguous=False,
>  The contiguous flag can be dropped, unifiying the cmd part of the 3
> .*creat 
Why is that?
Line 1095:              initialTag=None):
Line 1096:     """
Line 1097:     Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1098:     """


Line 1091:     _completeCreateLV(rc, vgName, lvName)
Line 1092: 
Line 1093: 
Line 1094: def createLV(vgName, lvName, size, activate=True, contiguous=False,
Line 1095:              initialTag=None):
>  For regular LVs, not special, image related ones the initialTag is
> mandator 
Done
Line 1096:     """
Line 1097:     Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1098:     """
Line 1099:     # WARNING! From man vgs:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Create storage domain using command type 1

Eduardo has posted comments on this change.

Change subject: Create storage domain using command type 1
......................................................................


Patch Set 7: Code-Review-1

(6 comments)

http://gerrit.ovirt.org/#/c/23646/7/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 132:     return LVM_ENC_ESCAPE.sub(lambda c: unichr(int(c.groups()[0])), s)
Line 133: 
Line 134: 
Line 135: def encodeVgTags(tagsDict):
Line 136:         tags = [VGTagMetadataRW.METADATA_TAG_PREFIX +
More efficient will be to build the tuple directly.
Line 137:                 lvmTagEncode("%s=%s" % (k, v))
Line 138:                 for k, v in tagsDict.items()]
Line 139:         return tuple(tags)
Line 140: 


Line 570:         #         no one reads it from here anyway
Line 571:         initialMetadata = {
Line 572:             sd.DMDK_VERSION: version,
Line 573:             sd.DMDK_SDUUID: sdUUID,
Line 574:             sd.DMDK_TYPE: BLOCK_SD_MD_FIELDS[sd.DMDK_TYPE][1](storageType),
Add a comment please explaining the use of the MD encoding please.
Line 575:             sd.DMDK_CLASS: BLOCK_SD_MD_FIELDS[sd.DMDK_CLASS][1](domClass),
Line 576:             sd.DMDK_DESCRIPTION: domainName,
Line 577:             sd.DMDK_ROLE: sd.REGULAR_DOMAIN,
Line 578:             sd.DMDK_POOLS: BLOCK_SD_MD_FIELDS[sd.DMDK_POOLS][1]([]),


Line 599: 
Line 600:         # Mark VG with Storage Domain Tag
Line 601:         try:
Line 602:             lvm.replaceVGTag(vgName, STORAGE_UNREADY_DOMAIN_TAG,
Line 603:                              STORAGE_DOMAIN_TAG, safe=False)
safe=False? IMHO it is not required anymore (previous version). Unify all this tag changes
please.
Line 604:         except se.StorageException:
Line 605:             raise se.VolumeGroupUninitialized(vgName)
Line 606: 
Line 607:         bsd = BlockStorageDomain(sdUUID)


http://gerrit.ovirt.org/#/c/23646/7/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 1052: # Public Logical volume interface
Line 1053: #
Line 1054: 
Line 1055: 
Line 1056: def _completeCreateLV(rc, vgName, lvName, activate=True):
I really dislike the "complete" part of the name.
Line 1057:     if rc == 0:
Line 1058:         _lvminfo._invalidatevgs(vgName)
Line 1059:         _lvminfo._invalidatelvs(vgName, lvName)
Line 1060:     else:


Line 1090:     rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName, )), rw=True)
Line 1091:     _completeCreateLV(rc, vgName, lvName)
Line 1092: 
Line 1093: 
Line 1094: def createLV(vgName, lvName, size, activate=True, contiguous=False,
The contiguous flag can be dropped, unifiying the cmd part of the 3 .*createLV functions
in the internal part.
Line 1095:              initialTag=None):
Line 1096:     """
Line 1097:     Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1098:     """


Line 1091:     _completeCreateLV(rc, vgName, lvName)
Line 1092: 
Line 1093: 
Line 1094: def createLV(vgName, lvName, size, activate=True, contiguous=False,
Line 1095:              initialTag=None):
For regular LVs, not special, image related ones the initialTag is mandatory, simplifiying
the code.
Line 1096:     """
Line 1097:     Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1098:     """
Line 1099:     # WARNING! From man vgs:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Eduardo has posted comments on this change.

Change subject: [WIP] Create storage domain using command type 1
......................................................................


Patch Set 2: Code-Review-1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Eduardo has posted comments on this change.

Change subject: [WIP] Create storage domain using command type 1
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.ovirt.org/#/c/23646/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 516:             raise se.StorageDomainIsMadeFromTooManyPVs()
Line 517: 
Line 518:         # Create metadata service volume
Line 519:         metasize = cls.metaSize(vgName)
Line 520:         lvm.createLV(vgName, sd.METADATA, "%s" % (metasize),
safe=False)
Look at the lvm code comment.
Line 521:         # Create the mapping right now so the index 0 is guaranteed
Line 522:         # to belong to the metadata volume. Since the metadata is at
Line 523:         # least SDMETADATA/METASIZE units, we know we can use the first
Line 524:         # SDMETADATA bytes of the metadata volume for the SD metadata.


http://gerrit.ovirt.org/#/c/23646/2/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 654:     """ This fuction is so restrictive as we can
do."""
Line 655:     log.debug("### _current_lvmconf %s",
globals()["_current_lvmconf"])
Line 656:     globals()["_current_lvmconf"] =
_current_lvmconf.replace("locking_type=4",
Line 657:                                                             
"locking_type=1")
Line 658:     log.debug("### _current_lvmconf %s",
globals()["_current_lvmconf"])
These logs should be updated before production.
Line 659: 
Line 660: 
Line 661: def bootstrap(refreshlvs=()):
Line 662:     """


Line 1061: #
Line 1062: 
Line 1063: 
Line 1064: def createLV(vgName, lvName, size, activate=True, contiguous=False,
Line 1065:              initialTag=None, safe=True):
createLV operates on an existing VG making "safe" parameters misuse very
dangerous at this time.

If the intention is to use this parameter only when the SD is created may be an
alternative method should be found lioke spliting the function for createLV() for images
volumes and createSpecialVolumes which are created only at SD creation and never modified,
making this parameter redundant.
Line 1066:     """
Line 1067:     Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1068:     """
Line 1069:     # WARNING! From man vgs:


Line 1280:     """
Line 1281:     return os.path.exists(lvPath(vgName, lvName))
Line 1282: 
Line 1283: 
Line 1284: def changeVGTags(vgName, delTags=(), addTags=(), safe=True):
Again, this change is dangerous and very misleading.

IMHO separating the only valid case for a non-SPM doing this operation (create) will be
more in line with the purpose of this series.

The new function should be more simple than the actual changeVGTags since it operates in a
known set of tags.

Please note that actually createSD() is the only user of replaceVGtags . Therefore you can
change it there and the split is natural and you need not to change changeVGTags()
function.
Line 1285:     delTags = set(delTags)
Line 1286:     addTags = set(addTags)
Line 1287:     if delTags.intersection(addTags):
Line 1288:         raise se.VolumeGroupReplaceTagError(


Line 1321:     if rc != 0:
Line 1322:         raise se.VolumeGroupRemoveTagError(vgName)
Line 1323: 
Line 1324: 
Line 1325: def replaceVGTag(vg, oldTag, newTag, safe=True):
Please see above.
Line 1326:     changeVGTags(vg, [oldTag], [newTag], safe)
Line 1327: 
Line 1328: 
Line 1329: def addVGTags(vgName, tags):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Yeela Kaplan has posted comments on this change.

Change subject: [WIP] Create storage domain using command type 1
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.ovirt.org/#/c/23646/2/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 516:             raise se.StorageDomainIsMadeFromTooManyPVs()
Line 517: 
Line 518:         # Create metadata service volume
Line 519:         metasize = cls.metaSize(vgName)
Line 520:         lvm.createLV(vgName, sd.METADATA, "%s" % (metasize),
safe=False)
>  Look at the lvm code comment. 
Done
Line 521:         # Create the mapping right now so the index 0 is guaranteed
Line 522:         # to belong to the metadata volume. Since the metadata is at
Line 523:         # least SDMETADATA/METASIZE units, we know we can use the first
Line 524:         # SDMETADATA bytes of the metadata volume for the SD metadata.


http://gerrit.ovirt.org/#/c/23646/2/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 654:     """ This fuction is so restrictive as we can
do."""
Line 655:     log.debug("### _current_lvmconf %s",
globals()["_current_lvmconf"])
Line 656:     globals()["_current_lvmconf"] =
_current_lvmconf.replace("locking_type=4",
Line 657:                                                             
"locking_type=1")
Line 658:     log.debug("### _current_lvmconf %s",
globals()["_current_lvmconf"])
>  These logs should be updated before production. 
Noted, thanks/
Line 659: 
Line 660: 
Line 661: def bootstrap(refreshlvs=()):
Line 662:     """


Line 1061: #
Line 1062: 
Line 1063: 
Line 1064: def createLV(vgName, lvName, size, activate=True, contiguous=False,
Line 1065:              initialTag=None, safe=True):
>  createLV operates on an existing VG making "safe"
> parameters misuse very da 
Done
Line 1066:     """
Line 1067:     Size units: MB (1024 ** 2 = 2 ** 20)B.
Line 1068:     """
Line 1069:     # WARNING! From man vgs:


Line 1280:     """
Line 1281:     return os.path.exists(lvPath(vgName, lvName))
Line 1282: 
Line 1283: 
Line 1284: def changeVGTags(vgName, delTags=(), addTags=(), safe=True):
>  Again, this change is dangerous and very misleading. 
create is
not the only valid case non-SPM doing this operation,

Please see following patches.
Line 1285:     delTags = set(delTags)
Line 1286:     addTags = set(addTags)
Line 1287:     if delTags.intersection(addTags):
Line 1288:         raise se.VolumeGroupReplaceTagError(


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Eduardo has posted comments on this change.

Change subject: [WIP] Create storage domain using command type 1
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.ovirt.org/#/c/23646/2/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 1280:     """
Line 1281:     return os.path.exists(lvPath(vgName, lvName))
Line 1282: 
Line 1283: 
Line 1284: def changeVGTags(vgName, delTags=(), addTags=(), safe=True):
>  create is not the only valid case non-SPM doing this operation,
Please look at the comments in the last patch of this series.
changeVGtags() can be avoided in the destroySP().

In addition destroyStoragePool can remove the entire MD of the domain or remove it
enterily without changes to the subsequent formatStorageDomain(). The actual logic is
inconsistent.

In any case changeVGTags with locking type 1 without being SPM is not required.
Line 1285:     delTags = set(delTags)
Line 1286:     addTags = set(addTags)
Line 1287:     if delTags.intersection(addTags):
Line 1288:         raise se.VolumeGroupReplaceTagError(


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes

Change in vdsm[master]: Create storage domain using command type 1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Create storage domain using command type 1
......................................................................


Patch Set 7:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6801/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7591/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7701/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Create storage domain using command type 1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Create storage domain using command type 1
......................................................................


Patch Set 6:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6509/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7293/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7411/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Create storage domain using command type 1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Create storage domain using command type 1
......................................................................


Patch Set 5: Verified-1

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6501/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7285/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7403/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Create storage domain using command type 1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Create storage domain using command type 1
......................................................................


Patch Set 4: Verified-1

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6485/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7269/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7387/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Create storage domain using command type 1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Create storage domain using command type 1
......................................................................


Patch Set 3: Verified-1

Build Failed 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6480/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7264/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7382/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Create storage domain using command type 1

Itamar Heim has abandoned this change.

Change subject: Create storage domain using command type 1
......................................................................


Abandoned

no activity. please restore if/when still relevant.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)gmail.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: [WIP] Create storage domain using command type 1
......................................................................


Patch Set 2:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6324/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/7214/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/7107/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: [WIP] Create storage domain using command type 1
......................................................................


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/6047/ : SUCCESS

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6939/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Change in vdsm[master]: Create storage domain using command type 1

Itamar Heim has posted comments on this change.

Change subject: Create storage domain using command type 1
......................................................................


Patch Set 8:

ping

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I127af299086ec5572d29686451d4892c9ff0330d
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo <ewarszaw(a)gmail.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No