Change in vdsm[master]: lvm: Check if device in VG before extend
by frolland＠redhat.com
Hello Fred Rolland,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/45946
to review the following change.
Change subject: lvm: Check if device in VG before extend
......................................................................
lvm: Check if device in VG before extend
Before extending a VG, VDSM needs to check that none of
the additional devices are not already part of the VG.
Change-Id: Ifec2503094d6a0ecdd32e5e62f9c01a1469145f3
Backport-To: 3.6
Bug-Url: https://bugzilla.redhat.com/1261531
Signed-off-by: Fred Rolland <frolland(a)redhat.com>
---
M vdsm/storage/lvm.py
1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/45946/1
diff --git a/vdsm/storage/lvm.py b/vdsm/storage/lvm.py
index 0394ce9..b56b92e 100644
--- a/vdsm/storage/lvm.py
+++ b/vdsm/storage/lvm.py
@@ -983,6 +983,12 @@
pvs = [_fqpvname(pdev) for pdev in _normalizeargs(devices)]
_checkpvsblksize(pvs, getVGBlockSizes(vgName))
vg = _lvminfo.getVg(vgName)
+
+ member_pvs = set(vg.pv_name).intersection(pvs)
+ if member_pvs:
+ log.error("These PVs already belong to VG %s", member_pvs)
+ raise se.VolumeGroupExtendError(vgName, pvs)
+
# Format extension PVs as all the other already in the VG
_initpvs(pvs, int(vg.vg_mda_size) / 2 ** 20, force)
--
To view, visit https://gerrit.ovirt.org/45946
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifec2503094d6a0ecdd32e5e62f9c01a1469145f3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland <frolland(a)redhat.com>
Gerrit-Reviewer: Fred Rolland <frolland(a)redhat.com>
8 years, 8 months
Change in vdsm[master]: spec: Require lvm >= 2.02.100-8
by tnisan＠redhat.com
Tal Nisan has uploaded a new change for review.
Change subject: spec: Require lvm >= 2.02.100-8
......................................................................
spec: Require lvm >= 2.02.100-8
Change-Id: Iee80ffbbee55768aa632725c9f129813a8815d4b
Signed-off-by: Tal Nisan <tnisan(a)redhat.com>
Bug-Url: https://bugzilla.redhat.com/1127117
---
M vdsm.spec.in
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/92/37492/1
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 9c810ad..8a1389c 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -232,7 +232,7 @@
Requires: policycoreutils >= 2.0.83-19.30
Requires: policycoreutils-python >= 2.0.83-19.47.el6_6.1
Requires: selinux-policy-targeted >= 3.7.19-260.el6_6.2
-Requires: lvm2 >= 2.02.100-5
+Requires: lvm2 >= 2.02.100-8
Requires: logrotate < 3.8.0
%endif
%else
--
To view, visit http://gerrit.ovirt.org/37492
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iee80ffbbee55768aa632725c9f129813a8815d4b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com>
8 years, 8 months
Change in vdsm[master]: config: Improve options documentation
by Nir Soffer
Nir Soffer has uploaded a new change for review.
Change subject: config: Improve options documentation
......................................................................
config: Improve options documentation
Fix bogus documentation for vm_watermark_interval, which seems to
confuse both users and developers. This option has nothing to do with
statistics.
Add the missing documentation for the highly confusing
volume_utilization_percent and volume_utilization_chunk_mb.
Change-Id: I74b729a1752e15a1f57e3d64b4355f1bab7df559
Bug-Url: https://bugzilla.redhat.com/1195421
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M lib/vdsm/config.py.in
1 file changed, 10 insertions(+), 3 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/45167/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 8f2e519..79fb474 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -144,7 +144,8 @@
'expected to freeze during cluster failover.'),
('vm_watermark_interval', '2',
- 'How often should we sample each vm for statistics (seconds).'),
+ 'How often should we sample drive watermark on block storage for '
+ 'atomatic extension of thin provisioned volumes (seconds).'),
('vm_sample_interval', '15', None),
@@ -230,9 +231,15 @@
('irsd', '%(images)s/irsd', None),
- ('volume_utilization_percent', '50', None),
+ ('volume_utilization_percent', '50',
+ 'Togegther with volume_utilization_chunk_mb, set the minimal free '
+ 'space before a thin provisioned block volume is extended. Use '
+ 'lower values to extend earlier.'),
- ('volume_utilization_chunk_mb', '1024', None),
+ ('volume_utilization_chunk_mb', '1024',
+ 'Size of extension chunk in megabytes, and together with '
+ 'volume_utilization_percent, set the free space limit. Use higher '
+ 'values to extend in bigger chunks.'),
('vol_size_sample_interval', '60',
'How often should the volume size be checked (seconds).'),
--
To view, visit https://gerrit.ovirt.org/45167
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I74b729a1752e15a1f57e3d64b4355f1bab7df559
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
8 years, 8 months
Change in vdsm[master]: migration: Add incoming migration semaphore
by Martin Polednik
Martin Polednik has posted comments on this change.
Change subject: migration: Add incoming migration semaphore
......................................................................
Patch Set 3:
(3 comments)
https://gerrit.ovirt.org/#/c/45954/3/lib/vdsm/utils.py
File lib/vdsm/utils.py:
Line 1272: yield
Line 1273:
Line 1274:
Line 1275: @contextmanager
Line 1276: def acquired(resource, block):
> still not sure it is a good approach.
simulates standard lock quite nice (do you know why acquire has no contextmgr by default?)
Line 1277: success = resource.acquire(block)
Line 1278: try:
Line 1279: yield success
Line 1280: except Exception:
Line 1273:
Line 1274:
Line 1275: @contextmanager
Line 1276: def acquired(resource, block):
Line 1277: success = resource.acquire(block)
> success is not a good name here. (And now I lack suggestions!)
imho lock (since in this context it really is a lock to resource)
Line 1278: try:
Line 1279: yield success
Line 1280: except Exception:
Line 1281: resource.release()
https://gerrit.ovirt.org/#/c/45954/3/vdsm/API.py
File vdsm/API.py:
Line 571: :type params: dict
Line 572: """
Line 573: self.log.debug('Migration create')
Line 574:
Line 575: with utils.acquire(migration.incomingMigrations,
> acquire or acquired? (see utils.py)
I'd say it's acquired - the conditional sounds very nice then (if acquired or if not acquired)
Line 576: block=False) as acquired:
Line 577: if not acquired:
Line 578: return response.error('migrationLimit')
Line 579:
--
To view, visit https://gerrit.ovirt.org/45954
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8952f732033ed160292b11fbc0c4deac099b2b3e
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbetak(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan(a)redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjelinek(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 8 months
Change in vdsm[ovirt-3.5]: vm.py: State saving in hotunplugDisk.
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: vm.py: State saving in hotunplugDisk.
......................................................................
Patch Set 2: Code-Review-1
(3 comments)
https://gerrit.ovirt.org/#/c/45928/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 4038
Line 4039
Line 4040
Line 4041
Line 4042
In 3.5 you must remove the domainID from the sdIds list.
Line 4044: self._dom.detachDevice(driveXml)
Line 4045: except libvirt.libvirtError as e:
Line 4046: self.log.error("Hotunplug failed", exc_info=True)
Line 4047: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
Line 4048: return response.error('noVM')
response.error() is not available in 3.5, look around to see how it was done in 3.5.
Line 4049: return response.error('hotunplugDisk', e.message)
Line 4050: else:
Line 4051: self._devices[hwclass.DISK].remove(drive)
Line 4052:
Line 4046: self.log.error("Hotunplug failed", exc_info=True)
Line 4047: if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
Line 4048: return response.error('noVM')
Line 4049: return response.error('hotunplugDisk', e.message)
Line 4050: else:
You must update the sdIds list as was done in the old code.
Line 4051: self._devices[hwclass.DISK].remove(drive)
Line 4052:
Line 4053: # Find and remove disk device from vm's conf
Line 4054: for dev in self.conf['devices'][:]:
--
To view, visit https://gerrit.ovirt.org/45928
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cf18186cbba33d7e74fd15651ffec3149c98e1d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Tal Nisan <tnisan(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 8 months
Change in vdsm[master]: utils.retry: Support system time changes while retry() is ru...
by Nir Soffer
Nir Soffer has posted comments on this change.
Change subject: utils.retry: Support system time changes while retry() is running
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.ovirt.org/46075
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bc072bc20e8f5ae2db3fd1e48d91cdce9d469f8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh.dev(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 8 months
Change in vdsm[master]: sp: startSpm - ignore InquireNotSupportedError()
by laravot＠redhat.com
Liron Aravot has uploaded a new change for review.
Change subject: sp: startSpm - ignore InquireNotSupportedError()
......................................................................
sp: startSpm - ignore InquireNotSupportedError()
Currently startSpm() is responsible for the pool upgrade, it attempts to
upgrade the pool domains to the desired version. During the execution of
startSpm() we attempt to retrieve the current spm status in order to compare
it with the engine sent parameters - and in case of difference we log
the info to the user.
When a DC upgrade is performed when the DC is down from a version that uses
V1 as the domains version to a version that uses the StoragePoolMemoryBackend
we'll encounter a problem - because the StoragePoolMemoryBackend uses
the information from the clusterlock only and the current clusterlock
that is used on the domain might not support inquiring (safelease for
example).
As we use the inquired information just to display a warning, in case of
a clusterlock that doesn't support inquiring we should just log it to
the user and continue with starting the spm.
Change-Id: I082dc83ea410768db3819e7259089c20c2614b07
Bug-Url: https://bugzilla.redhat.com/1242092
Signed-off-by: Liron Aravot <laravot(a)redhat.com>
---
M vdsm/storage/sp.py
1 file changed, 11 insertions(+), 5 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/45764/1
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 4a8b67c..22d9571 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -237,7 +237,6 @@
raise se.MiscOperationInProgress("spm start %s" % self.spUUID)
self.updateMonitoringThreads()
- oldlver, oldid = self._backend.getSpmStatus()
masterDomVersion = self.getVersion()
# If no specific domain version was specified use current master
# domain version
@@ -249,10 +248,17 @@
self.masterDomain.sdUUID, curVer=masterDomVersion,
expVer=expectedDomVersion)
- if int(oldlver) != int(prevLVER) or int(oldid) != int(prevID):
- self.log.info("expected previd:%s lver:%s got request for "
- "previd:%s lver:%s" %
- (oldid, oldlver, prevID, prevLVER))
+ oldlver = LVER_INVALID
+ try:
+ oldlver, oldid = self._backend.getSpmStatus()
+ except se.InquireNotSupportedError:
+ self.log.info("cluster lock inquire isn't supported. "
+ "proceeding with startSpm()")
+ else:
+ if int(oldlver) != int(prevLVER) or int(oldid) != int(prevID):
+ self.log.info("expected previd:%s lver:%s got request for "
+ "previd:%s lver:%s" %
+ (oldid, oldlver, prevID, prevLVER))
self.spmRole = SPM_CONTEND
--
To view, visit https://gerrit.ovirt.org/45764
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I082dc83ea410768db3819e7259089c20c2614b07
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <laravot(a)redhat.com>
8 years, 8 months
Change in vdsm[master]: utils.retry: Support system time changes while retry() is ru...
by automation＠ovirt.org
automation(a)ovirt.org has posted comments on this change.
Change subject: utils.retry: Support system time changes while retry() is running
......................................................................
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/46075
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bc072bc20e8f5ae2db3fd1e48d91cdce9d469f8
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh.dev(a)gmail.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 8 months
Change in vdsm[master]: utils.retry: Support system time changes while retry() is ru...
by automation＠ovirt.org
automation(a)ovirt.org has posted comments on this change.
Change subject: utils.retry: Support system time changes while retry() is running
......................................................................
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/46075
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bc072bc20e8f5ae2db3fd1e48d91cdce9d469f8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <edwardh.dev(a)gmail.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 8 months
Change in vdsm[master]: migration: Add incoming migration semaphore
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: migration: Add incoming migration semaphore
......................................................................
Patch Set 3:
(8 comments)
initial review. Most important point is to split the retry logic into another patch. No score, since still draft.
https://gerrit.ovirt.org/#/c/45954/3//COMMIT_MSG
Commit Message:
Line 10:
Line 11: 'migrationCreate' werb can now return new error status to signify full
Line 12: occupation.
Line 13:
Line 14: Added handling of this error to MigrationSourceThread with retry logic,
please split the introduction of retry logic into another patch.
Line 15: that also enables user to cancell awaiting migrations.
Line 16:
Line 17: Wiki: http://www.ovirt.org/Features/Migration_Enhancements
Line 18: Change-Id: I8952f732033ed160292b11fbc0c4deac099b2b3e
https://gerrit.ovirt.org/#/c/45954/3/lib/vdsm/utils.py
File lib/vdsm/utils.py:
Line 1272: yield
Line 1273:
Line 1274:
Line 1275: @contextmanager
Line 1276: def acquired(resource, block):
still not sure it is a good approach.
Line 1277: success = resource.acquire(block)
Line 1278: try:
Line 1279: yield success
Line 1280: except Exception:
Line 1273:
Line 1274:
Line 1275: @contextmanager
Line 1276: def acquired(resource, block):
Line 1277: success = resource.acquire(block)
success is not a good name here. (And now I lack suggestions!)
Line 1278: try:
Line 1279: yield success
Line 1280: except Exception:
Line 1281: resource.release()
https://gerrit.ovirt.org/#/c/45954/3/vdsm/API.py
File vdsm/API.py:
Line 571: :type params: dict
Line 572: """
Line 573: self.log.debug('Migration create')
Line 574:
Line 575: with utils.acquire(migration.incomingMigrations,
acquire or acquired? (see utils.py)
Line 576: block=False) as acquired:
Line 577: if not acquired:
Line 578: return response.error('migrationLimit')
Line 579:
https://gerrit.ovirt.org/#/c/45954/3/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 56:
Line 57: mig = min(config.getint('vars', 'max_incoming_migrations'),
Line 58: caps.CpuTopology().cores())
Line 59:
Line 60: incomingMigrations = threading.BoundedSemaphore(mig)
let's avoid this temporary, please.
Line 61:
Line 62:
Line 63: class MigrationDestinationSetupError(RuntimeError):
Line 64: """
Line 342: startTime += destCreationTime
Line 343: self.log.info('Creation of destination VM took: %d seconds',
Line 344: destCreationTime)
Line 345:
Line 346: if result['status'] == errCode['migrationLimit']['status']:
please consider use (and probably enhance) response.is_error.
Line 347: # we failed, but migration was cancelled in the meantime
Line 348: # so don't try again
Line 349: if self._migrationCanceledEvt:
Line 350: break
Line 353: # the destination is busy with incoming migrations
Line 354: # release semaphore and give other outgoing migrations
Line 355: # a chance
Line 356: time.sleep(5)
Line 357: SourceThread._ongoingMigrations.acquire()
I understand why you do this, but this code is a bit harder to follow, and it was not a textbook example of clarity before. Let's try to find another way.
Maybe moving the semaphore acquiring thing in this method will help.
Line 358: else:
Line 359: break
Line 360:
Line 361: if self._migrationCanceledEvt:
https://gerrit.ovirt.org/#/c/45954/3/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 2721: fromSnapshot = self.conf.pop('restoreFromSnapshot', False)
Line 2722: hooks.after_vm_dehibernate(self._dom.XMLDesc(0), self.conf,
Line 2723: {'FROM_SNAPSHOT': fromSnapshot})
Line 2724: elif 'migrationDest' in self.conf:
Line 2725: migration.incomingMigrations.release()
Again, I understand why you did this, but let's invest some time finding one alternative
Line 2726: waitMigration = True
Line 2727: if self.recovering:
Line 2728: try:
Line 2729: if self._isDomainRunning():
--
To view, visit https://gerrit.ovirt.org/45954
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8952f732033ed160292b11fbc0c4deac099b2b3e
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbetak(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Martin Betak <mbetak(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan(a)redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjelinek(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 8 months
