Change in vdsm[master]: migration: added support for convergance schedule
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: migration: added support for convergance schedule
......................................................................
Patch Set 4:
(3 comments)
https://gerrit.ovirt.org/#/c/46940/4/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 399: self._vm.log.info('starting migration to %s '
Line 400: 'with miguri %s', duri, muri)
Line 401:
Line 402: self._monitorThread = MonitorThread(self._vm, startTime, self._convergenceSchedule)
Line 403: self._do_perform_migration(self, duri, muri)
> actually... :) the _do_perform_migration is assigned to self in the constru
Disclosure: I actually missed the _do in the first review :)
But still, let's try to make this more readable
Line 404:
Line 405: self.log.info("migration took %d seconds to complete",
Line 406: (time.time() - startTime) + destCreationTime)
Line 407:
Line 594: self._vm.log.debug('stopping migration monitor thread')
Line 595: self._stop.set()
Line 596:
Line 597: def _next_action(self, stalling):
Line 598: head, tail = self._convSchedule[0], self._convSchedule[1:]
> I need to assign the tail to the conv_schedule only if head.stallingLimit <
Right, I overlooked the code. Then maybe something like
head = self._convSchedule[0] # peek the head
# log
if head.stallingLimit < stalling:
self._execute_next_step(stalling, head.actionWithParams)
# log
self._convSchedule.pop(0) # consume the head
Line 599: self._vm.log.debug('Stalling for %d seconds, '
Line 600: 'trying to make next action: '
Line 601: '%s, tail: %s', stalling, head, tail)
Line 602: if head.stallingLimit < stalling:
Line 605: self._convSchedule = tail
Line 606:
Line 607: def _execute_next_step(self, stalling, actionWithParams):
Line 608: action = str(actionWithParams.action)
Line 609: if action == CONVERGENCE_SCHEDULE_SET_DOWNTIME:
> hm, do you mean to provide some map like convergance_actions which would ma
Yes, this is what I meant.
I don't have strong feelings here, we can do this in a later patch if we find a good way. I just wanted to make sure you considered this option, it is fine if you did and choose something else.
Line 610: downtime = actionWithParams.params[0]
Line 611: self.log.warn('Stalling for %d, setting downtime to %d',
Line 612: stalling, downtime)
Line 613: self._vm._dom.migrateSetMaxDowntime(int(downtime), 0)
--
To view, visit https://gerrit.ovirt.org/46940
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I989cff12d08ef1cab36bd10df7daaa999a8dac14
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjelinek(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjelinek(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 6 months
Change in vdsm[master]: init: configure multipath on upgrade from ovirt-3.5
by ykaplan＠redhat.com
Yeela Kaplan has uploaded a new change for review.
Change subject: init: configure multipath on upgrade from ovirt-3.5
......................................................................
init: configure multipath on upgrade from ovirt-3.5
multipath configurator was added to vdsm-tool
in ovirt-3.6.
In case we upgrade from ovirt-3.5 we now run configure of
module multipath in init script.
Change-Id: Ifbebdebb6bda25606b20d91804977fa8eb7f13e8
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1276736
Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com>
---
M init/vdsmd_init_common.sh.in
1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/41/47941/1
diff --git a/init/vdsmd_init_common.sh.in b/init/vdsmd_init_common.sh.in
index 00e4054..62e81bd 100644
--- a/init/vdsmd_init_common.sh.in
+++ b/init/vdsmd_init_common.sh.in
@@ -259,6 +259,11 @@
# by running manual configure command
"$VDSM_TOOL" configure --force
ret=$?
+ elif grep -q '^vdsm-4\.16\.' "${upgraded_ver_file}"; then
+ # We need to treat the upgrade from 4.16.x specifically
+ # by running manual multipath configure command
+ "$VDSM_TOOL" configure --module multipath
+ ret=$?
fi
[ "${ret}" -eq 0 ] && rm -f "${upgraded_ver_file}"
fi
--
To view, visit https://gerrit.ovirt.org/47941
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifbebdebb6bda25606b20d91804977fa8eb7f13e8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
8 years, 6 months
Change in vdsm[master]: lib: grub2 module
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: lib: grub2 module
......................................................................
Patch Set 1: Code-Review-1
(3 comments)
-1 for visibility
I started reviewing, and gave initial comments, than realized that here we are partially replicating a facility that should be system wide.
So, +1 for the feature, but I feel that we need to draw a line, construct the parameters we want (or need), but then the actual job of handling the grub2 config should _enterely_ shed on some other system wide tool (grub2-mkconfig is not enough, too low level), on which we should depend on.
https://gerrit.ovirt.org/#/c/47946/1/lib/vdsm/grub2.py
File lib/vdsm/grub2.py:
Line 22: import collections
Line 23:
Line 24: from . import utils
Line 25:
Line 26: _GRUB_CONFIG = os.path.abspath('/etc/default/grub')
but this isn't already an abs path? :)
Line 27:
Line 28: _CFG_NAME_PCI_STUB = 'VDSM_PCI_STUB_DEVICES'
Line 29: _CFG_NAME_WORKAROUNDS = 'VDSM_WORKAROUNDS'
Line 30: _CFG_NAME_CMDLINE = 'GRUB_CMDLINE_LINUX'
Line 29: _CFG_NAME_WORKAROUNDS = 'VDSM_WORKAROUNDS'
Line 30: _CFG_NAME_CMDLINE = 'GRUB_CMDLINE_LINUX'
Line 31: _CMDLINE_STUB_IDS = 'pci-stub.ids='
Line 32:
Line 33: _GRUB2_MKCONFIG = utils.CommandPath('/usr/sbin/grub2-mkconfig')
maybe just grub2-mkconfig and let CommandPath figure out the path?
Line 34:
Line 35:
Line 36: class Error(Exception):
Line 37:
Line 32:
Line 33: _GRUB2_MKCONFIG = utils.CommandPath('/usr/sbin/grub2-mkconfig')
Line 34:
Line 35:
Line 36: class Error(Exception):
For another patch, not necessarily from yours: let's factor this Error copied everywhere once for all.
Line 37:
Line 38: def __init__(self, rc, out, err):
Line 39: self.rc = rc
Line 40: self.out = out
--
To view, visit https://gerrit.ovirt.org/47946
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e7e0b327882f64196a71fe20dcbd99017b80800
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 6 months
Change in vdsm[master]: migration: added support for convergance schedule
by tjelinek＠redhat.com
Tomas Jelinek has posted comments on this change.
Change subject: migration: added support for convergance schedule
......................................................................
Patch Set 4:
(10 comments)
https://gerrit.ovirt.org/#/c/46940/4/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 135: with utils.running(self._monitorThread):
Line 136: self._perform_migration(duri, muri)
Line 137:
Line 138: self._do_perform_migration = _perform_with_downtime_thread
Line 139: if kwargs.has_key('convergenceSchedule'):
> let's just use
Done
Line 140: # example convergence schedule
Line 141: # "[(1, ('setDowntime', (10))), (4, ('abort', ()))]"
Line 142: schedule = literal_eval(kwargs.get('convergenceSchedule'))
Line 143: self._do_perform_migration = \
Line 138: self._do_perform_migration = _perform_with_downtime_thread
Line 139: if kwargs.has_key('convergenceSchedule'):
Line 140: # example convergence schedule
Line 141: # "[(1, ('setDowntime', (10))), (4, ('abort', ()))]"
Line 142: schedule = literal_eval(kwargs.get('convergenceSchedule'))
> Proposal:
right, also better to send json from the engine
Line 143: self._do_perform_migration = \
Line 144: _perform_with_conv_schedule_monitor_thread
Line 145:
Line 146: self._convergenceSchedule = map(ConvergenceItem._make,
Line 145:
Line 146: self._convergenceSchedule = map(ConvergenceItem._make,
Line 147: [(s[0],
Line 148: map(ConvergenceAction._make,
Line 149: s[1:])[0]) for s in schedule])
> whatever format we choose, we can also write:
MUCH better! Thank you, done.
Line 150:
Line 151: self.log.debug('convergence schedule set to: ' + str(self._convergenceSchedule))
Line 152:
Line 153:
Line 399: self._vm.log.info('starting migration to %s '
Line 400: 'with miguri %s', duri, muri)
Line 401:
Line 402: self._monitorThread = MonitorThread(self._vm, startTime, self._convergenceSchedule)
Line 403: self._do_perform_migration(self, duri, muri)
> no need to have self twice :)
actually... :) the _do_perform_migration is assigned to self in the constructor and the actual function is a function nested inside the constructor and expects the self to be the one in which it can find the _vm in.
But based on your comment I suspect I did not find the most pythonic solution ;)
Line 404:
Line 405: self.log.info("migration took %d seconds to complete",
Line 406: (time.time() - startTime) + destCreationTime)
Line 407:
Line 519: self._vm = vm
Line 520: self._startTime = startTime
Line 521: self.daemon = True
Line 522: self.progress = 0
Line 523: self._convSchedule = convSchedule
> conv_schedule, let's use pep8 (https://www.python.org/dev/peps/pep-0008/) n
Done
Line 524:
Line 525: @property
Line 526: def enabled(self):
Line 527: return MonitorThread._MIGRATION_MONITOR_INTERVAL > 0
Line 557: fileTotal, fileProcessed, _) = self._vm._dom.jobInfo()
Line 558: # from libvirt sources: data* = file* + mem*.
Line 559: # docs can be misleading due to misaligned lines.
Line 560: now = time.time()
Line 561:
> spurious extra line, please drop
Done
Line 562: if 0 < migrationMaxTime < now - self._startTime:
Line 563: self._vm.log.warn('The migration took %d seconds which is '
Line 564: 'exceeding the configured maximum time '
Line 565: 'for migrations of %d seconds. The '
Line 577: 'Migration stalling: remaining (%sMiB)'
Line 578: ' > lowmark (%sMiB).'
Line 579: ' Refer to RHBZ#919201.',
Line 580: dataRemaining / Mbytes, lowmark / Mbytes)
Line 581: self._next_action(now - lastProgressTime)
> not sure this needs to be in at this nesting level.
right
Line 582:
Line 583: if self._stop.isSet():
Line 584: break
Line 585:
Line 579: ' Refer to RHBZ#919201.',
Line 580: dataRemaining / Mbytes, lowmark / Mbytes)
Line 581: self._next_action(now - lastProgressTime)
Line 582:
Line 583: if self._stop.isSet():
> ditto
Done
Line 584: break
Line 585:
Line 586: if jobType != libvirt.VIR_DOMAIN_JOB_NONE:
Line 587: self.progress = update_progress(dataRemaining, dataTotal)
Line 594: self._vm.log.debug('stopping migration monitor thread')
Line 595: self._stop.set()
Line 596:
Line 597: def _next_action(self, stalling):
Line 598: head, tail = self._convSchedule[0], self._convSchedule[1:]
> head = self._convSchedule.pop(0)
I need to assign the tail to the conv_schedule only if head.stallingLimit < stalling while the pop would remove the head always.
Line 599: self._vm.log.debug('Stalling for %d seconds, '
Line 600: 'trying to make next action: '
Line 601: '%s, tail: %s', stalling, head, tail)
Line 602: if head.stallingLimit < stalling:
Line 605: self._convSchedule = tail
Line 606:
Line 607: def _execute_next_step(self, stalling, actionWithParams):
Line 608: action = str(actionWithParams.action)
Line 609: if action == CONVERGENCE_SCHEDULE_SET_DOWNTIME:
> you can pass around function and/or closures if you like/feel
hm, do you mean to provide some map like convergance_actions which would map the action name to action so it will look here like this?
convergance_actions[action]()
In this case not sure where to define this functions so it will not reduce the readability...
The functions are quite short and simple and there is just a few of them and it is not expected that they will grow.
What you think?
Line 610: downtime = actionWithParams.params[0]
Line 611: self.log.warn('Stalling for %d, setting downtime to %d',
Line 612: stalling, downtime)
Line 613: self._vm._dom.migrateSetMaxDowntime(int(downtime), 0)
--
To view, visit https://gerrit.ovirt.org/46940
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I989cff12d08ef1cab36bd10df7daaa999a8dac14
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjelinek(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjelinek(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 6 months
Change in vdsm[master]: utils: replace order of import for persist/unpersist
by Douglas Schilling Landgraf
Douglas Schilling Landgraf has uploaded a new change for review.
Change subject: utils: replace order of import for persist/unpersist
......................................................................
utils: replace order of import for persist/unpersist
Preferable use the new ovirt-node Config class for persist/unpersist
commands. The previous persist/unpersist provider uses /tmp/ovirt.log
and might affect the vdsm start as missing ownership of file.
Change-Id: I51571693d2ff145c1b72b47e28c5c2f09b15acb7
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1274063
Signed-off-by: Douglas Schilling Landgraf <dougsland(a)redhat.com>
---
M lib/vdsm/utils.py
1 file changed, 7 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/47829/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index 31d8529..bcd7cff 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -59,15 +59,15 @@
from . import constants
try:
- # If failing to import old code, then try importing the legacy code
- from ovirtnode import ovirtfunctions
- persist = ovirtfunctions.ovirt_store_config
- unpersist = ovirtfunctions.remove_config
+ from ovirt.node.utils.fs import Config
+ persist = Config().persist
+ unpersist = Config().unpersist
except ImportError:
try:
- from ovirt.node.utils.fs import Config
- persist = Config().persist
- unpersist = Config().unpersist
+ # If failing to import old code, then try importing the legacy code
+ from ovirtnode import ovirtfunctions
+ persist = ovirtfunctions.ovirt_store_config
+ unpersist = ovirtfunctions.remove_config
except ImportError:
persist = lambda name: None
unpersist = lambda name: None
--
To view, visit https://gerrit.ovirt.org/47829
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I51571693d2ff145c1b72b47e28c5c2f09b15acb7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
8 years, 6 months
Change in vdsm[master]: lib: grub2 module
by automation＠ovirt.org
automation(a)ovirt.org has posted comments on this change.
Change subject: lib: grub2 module
......................................................................
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/47946
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e7e0b327882f64196a71fe20dcbd99017b80800
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 6 months
Change in vdsm[master]: WIP: rename stats cache to vm stats cache
by fromani＠redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: WIP: rename stats cache to vm stats cache
......................................................................
WIP: rename stats cache to vm stats cache
Future patch want to modernize HostStats* and
port it to periodic Operations infrastructure.
To make room for this change and reduce
ambiguity, re-introduced the ubiquitous
vm* prefix.
No changes besides naming.
Change-Id: If38c49f686dfc2bc0994d444ff24c7736f2e951b
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M tests/samplingTests.py
M tests/vmfakelib.py
M vdsm/virt/periodic.py
M vdsm/virt/sampling.py
M vdsm/virt/vm.py
5 files changed, 16 insertions(+), 16 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/40371/1
diff --git a/tests/samplingTests.py b/tests/samplingTests.py
index 2624dd7..1f2a65c 100644
--- a/tests/samplingTests.py
+++ b/tests/samplingTests.py
@@ -250,13 +250,13 @@
self.assertTrue(self._sampleCount >= self.STOP_SAMPLE)
-class StatsCacheTests(TestCaseBase):
+class VmStatsCacheTests(TestCaseBase):
FAKE_CLOCK_STEP = 1
def setUp(self):
self.clock = 0
- self.cache = sampling.StatsCache(clock=self.fake_monotonic_time)
+ self.cache = sampling.VmStatsCache(clock=self.fake_monotonic_time)
def fake_monotonic_time(self):
self.clock += self.FAKE_CLOCK_STEP
diff --git a/tests/vmfakelib.py b/tests/vmfakelib.py
index d8ce6ce..544eb5a 100644
--- a/tests/vmfakelib.py
+++ b/tests/vmfakelib.py
@@ -201,7 +201,7 @@
fake._guestCpuRunning = runCpu
if status is not None:
fake._lastStatus = status
- sampling.stats_cache.add(fake.id)
+ sampling.vm_stats_cache.add(fake.id)
yield fake
diff --git a/vdsm/virt/periodic.py b/vdsm/virt/periodic.py
index a3f8cc6..00615c3 100644
--- a/vdsm/virt/periodic.py
+++ b/vdsm/virt/periodic.py
@@ -94,7 +94,7 @@
sampling.VMBulkSampler(
libvirtconnection.get(cif),
cif.getVMs,
- sampling.stats_cache),
+ sampling.vm_stats_cache),
config.getint('vars', 'vm_sample_interval')),
# we do this only until we get high water mark notifications
diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py
index 6804406..89f109b 100644
--- a/vdsm/virt/sampling.py
+++ b/vdsm/virt/sampling.py
@@ -375,7 +375,7 @@
EMPTY_SAMPLE = StatsSample(None, None, None, None)
-class StatsCache(object):
+class VmStatsCache(object):
"""
Cache for bulk stats samples.
Provide facilities to retrieve per-vm samples, and the glue code to deal
@@ -391,7 +391,7 @@
VDSM has countermeasures for those cases. Stuck threads are replaced,
thanks to Executor. But still, before to be destroyed, a replaced
- thread can mistakenly try to add a sample to a StatsCache.
+ thread can mistakenly try to add a sample to a VmStatsCache.
Because of worker thread replacement, that sample from stuck thread
can be stale.
@@ -402,7 +402,7 @@
between a well behaving call and an unblocked stuck call.
"""
- _log = logging.getLogger("sampling.StatsCache")
+ _log = logging.getLogger("sampling.VmStatsCache")
def __init__(self, clock=utils.monotonic_time):
self._clock = clock
@@ -478,7 +478,7 @@
self._vm_last_timestamp[vmid] = monotonic_ts
-stats_cache = StatsCache()
+vm_stats_cache = VmStatsCache()
# this value can be tricky to tune.
@@ -492,18 +492,18 @@
class VMBulkSampler(object):
- def __init__(self, conn, get_vms, stats_cache,
+ def __init__(self, conn, get_vms, vm_stats_cache,
stats_flags=0, timeout=_TIMEOUT):
self._conn = conn
self._get_vms = get_vms
- self._stats_cache = stats_cache
+ self._vm_stats_cache = vm_stats_cache
self._stats_flags = stats_flags
self._skip_doms = ExpiringCache(timeout)
self._sampling = Stage()
self._log = logging.getLogger("sampling.VMBulkSampler")
def __call__(self):
- timestamp = self._stats_cache.clock()
+ timestamp = self._vm_stats_cache.clock()
# we are deep in the hot path. bool(ExpiringCache)
# *is* costly so we should avoid it if we can.
fast_path = (self._sampling.empty and not self._skip_doms)
@@ -514,7 +514,7 @@
# If everything's ok, we can skip all the costly checks.
bulk_stats = self._conn.getAllDomainStats(
self._stats_flags)
- self._stats_cache.put(_translate(bulk_stats), timestamp)
+ self._vm_stats_cache.put(_translate(bulk_stats), timestamp)
else:
# A previous call got stuck, or not every domain
# has properly recovered. Thus we must whitelist domains.
@@ -523,7 +523,7 @@
if doms:
bulk_stats = self._conn.domainListGetStats(
doms, self._stats_flags)
- self._stats_cache.put(_translate(bulk_stats), timestamp)
+ self._vm_stats_cache.put(_translate(bulk_stats), timestamp)
def _get_responsive_doms(self):
vms = self._get_vms()
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index dcc215b..2b7e3eb 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1308,7 +1308,7 @@
decStats = {}
try:
- vm_sample = sampling.stats_cache.get(self.id)
+ vm_sample = sampling.vm_stats_cache.get(self.id)
decStats = vmstats.produce(self,
vm_sample.first_value,
vm_sample.last_value,
@@ -1675,7 +1675,7 @@
nic.name)
self._guestEventTime = self._startTime
- sampling.stats_cache.add(self.id)
+ sampling.vm_stats_cache.add(self.id)
try:
self.guestAgent.connect()
except Exception:
@@ -3454,7 +3454,7 @@
self.lastStatus = vmstatus.POWERING_DOWN
# Terminate the VM's creation thread.
self._incomingMigrationFinished.set()
- sampling.stats_cache.remove(self.id)
+ sampling.vm_stats_cache.remove(self.id)
self.guestAgent.stop()
if self._dom:
result = self._destroyVmGraceful()
--
To view, visit https://gerrit.ovirt.org/40371
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If38c49f686dfc2bc0994d444ff24c7736f2e951b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
8 years, 6 months
Change in vdsm[master]: sampling: introduce expensive checks
by fromani＠redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: sampling: introduce expensive checks
......................................................................
sampling: introduce expensive checks
To improve troubleshooting, introduce more
expensive sanity checks, with a default-off
tunable to enable them.
These sanity checks will make no attempt to recover,
will only add logs to report possible issues.
Change-Id: I7b3bb707dd60de194eedfc2e3de1efbf05574ff7
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M lib/vdsm/config.py.in
M vdsm/virt/sampling.py
2 files changed, 19 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/91/40391/1
diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index e213839..b60e836 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -325,6 +325,10 @@
('periodic_task_per_worker', '100',
'Max number of tasks which can be queued on workers.'
' This is for internal usage and may change without warning'),
+
+ ('expensive_checks', 'false',
+ 'Perform additional sanity checks and does additional debug logs'
+ 'which are expensive performance-wise'),
]),
# Section: [devel]
diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py
index b0628a4..3c1510f 100644
--- a/vdsm/virt/sampling.py
+++ b/vdsm/virt/sampling.py
@@ -504,6 +504,7 @@
self._skip_doms = ExpiringCache(timeout)
self._sampling = Stage()
self._log = logging.getLogger("sampling.VMBulkSampler")
+ self._extra_check = config.getboolean('sampling', 'expensive_checks')
def __call__(self):
timestamp = self._vm_stats_cache.clock()
@@ -517,7 +518,6 @@
# If everything's ok, we can skip all the costly checks.
bulk_stats = self._conn.getAllDomainStats(
self._stats_flags)
- self._vm_stats_cache.put(_translate(bulk_stats), timestamp)
else:
# A previous call got stuck, or not every domain
# has properly recovered. Thus we must whitelist domains.
@@ -526,7 +526,12 @@
if doms:
bulk_stats = self._conn.domainListGetStats(
doms, self._stats_flags)
- self._vm_stats_cache.put(_translate(bulk_stats), timestamp)
+ else:
+ bulk_stats = []
+
+ stats = _translate(bulk_stats)
+ self._vm_stats_cache.put(stats, timestamp)
+ self._log_missing_vms(self._get_vms(), stats, timestamp)
def _get_responsive_doms(self):
vms = self._get_vms()
@@ -541,6 +546,14 @@
doms.append(vm_obj._dom._dom)
return doms
+ def _log_missing_vms(self, expected_vms, retrieved_stats, timestamp):
+ # costly check. add another layer of check before to embark on it.
+ if self._extra_check:
+ for vm_id in expected_vms:
+ if vm_id not in retrieved_stats:
+ self._log.debug('VM %s not updated in bulk at %f',
+ vm_id, timestamp)
+
class HostStatsThread(threading.Thread):
"""
--
To view, visit https://gerrit.ovirt.org/40391
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7b3bb707dd60de194eedfc2e3de1efbf05574ff7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
8 years, 6 months
Change in vdsm[master]: sampling: simplify flows
by fromani＠redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: sampling: simplify flows
......................................................................
sampling: simplify flows
Executor already has a fallback net for uncaught exceptions.
Failures in SampleVMs.__call__ are expected to be sporadic,
and in these cases we actually want to be as much noisy as we
can.
This patch removes redundant code in SampleVMs.__call__,
to make the flow less convoluted.
Change-Id: Ide103d0ed9a694cc9ddd9b0b382e2d81a1bd48c0
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M vdsm/virt/sampling.py
1 file changed, 17 insertions(+), 21 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/26/40326/1
diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py
index 09ce2b3..5a205f7 100644
--- a/vdsm/virt/sampling.py
+++ b/vdsm/virt/sampling.py
@@ -507,27 +507,23 @@
# we are deep in the hot path. bool(ExpiringCache)
# *is* costly so we should avoid it if we can.
fast_path = (self._sampling.empty and not self._skip_doms)
- try:
- with self._sampling.performer():
- if fast_path:
- # This is expected to be the common case.
- # If everything's ok, we can skip all the costly checks.
- bulk_stats = self._conn.getAllDomainStats(
- self._stats_flags)
- else:
- # A previous call got stuck, or not every domain
- # has properly recovered. Thus we must whitelist domains.
- doms = self._get_responsive_doms()
- self._log.debug('sampling %d domains', len(doms))
- if doms:
- bulk_stats = self._conn.domainListGetStats(
- doms, self._stats_flags)
- else:
- bulk_stats = []
- except Exception:
- self._log.exception("vm sampling failed")
- else:
- self._stats_cache.put(_translate(bulk_stats), timestamp)
+
+ with self._sampling.performer():
+ if fast_path:
+ # This is expected to be the common case.
+ # If everything's ok, we can skip all the costly checks.
+ bulk_stats = self._conn.getAllDomainStats(
+ self._stats_flags)
+ self._stats_cache.put(_translate(bulk_stats), timestamp)
+ else:
+ # A previous call got stuck, or not every domain
+ # has properly recovered. Thus we must whitelist domains.
+ doms = self._get_responsive_doms()
+ self._log.debug('sampling %d domains', len(doms))
+ if doms:
+ bulk_stats = self._conn.domainListGetStats(
+ doms, self._stats_flags)
+ self._stats_cache.put(_translate(bulk_stats), timestamp)
def _get_responsive_doms(self):
vms = self._get_vms()
--
To view, visit https://gerrit.ovirt.org/40326
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ide103d0ed9a694cc9ddd9b0b382e2d81a1bd48c0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
8 years, 6 months
Change in vdsm[master]: api: Adding dns as additional field for setupNetworks ifcfg:...
by gcheresh＠redhat.com
Genadi Chereshnya has uploaded a new change for review.
Change subject: api: Adding dns as additional field for setupNetworks ifcfg: Updating ifcfg file with DNSs
......................................................................
api: Adding dns as additional field for setupNetworks
ifcfg: Updating ifcfg file with DNSs
Adding DNSs to the setupNetwork command
With multiple paragraphs if necessary.
Change-Id: Ib9ea16d033cc07ef10c05868da3e6b4bcee56f74
Bug-Url: https://bugzilla.redhat.com/??????
Signed-off-by: Genadi Chereshnya <gcheresh(a)redhat.com>
---
M vdsm/network/api.py
M vdsm/network/configurators/ifcfg.py
2 files changed, 12 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/43/47843/1
diff --git a/vdsm/network/api.py b/vdsm/network/api.py
index 4ed64f3..e3f8289 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -89,7 +89,7 @@
ipv6addr=None, ipv6gateway=None, ipv6autoconf=None,
dhcpv6=None, defaultRoute=None, _netinfo=None,
configurator=None, blockingdhcp=None,
- implicitBonding=None, opts=None):
+ implicitBonding=None, opts=None, dnss=None):
"""
Constructs an object hierarchy that describes the network configuration
that is passed in the parameters.
@@ -118,6 +118,7 @@
routing table?
:param opts: misc options received by the callee, e.g., {'stp': True}. this
function can modify the dictionary.
+ :param dnss: list of DNSs
:returns: the top object of the hierarchy.
"""
@@ -249,7 +250,7 @@
ipv6addr=None, ipv6gateway=None, ipv6autoconf=None,
force=False, configurator=None, bondingOptions=None,
bridged=True, _netinfo=None, hostQos=None,
- defaultRoute=None, blockingdhcp=False, **options):
+ defaultRoute=None, blockingdhcp=False, dnss=None, **options):
nics = nics or ()
if _netinfo is None:
_netinfo = netinfo.NetInfo()
@@ -311,7 +312,8 @@
netmask=netmask, gateway=gateway, bootproto=bootproto, dhcpv6=dhcpv6,
blockingdhcp=blockingdhcp, ipv6addr=ipv6addr, ipv6gateway=ipv6gateway,
ipv6autoconf=ipv6autoconf, defaultRoute=defaultRoute,
- _netinfo=_netinfo, configurator=configurator, opts=options)
+ _netinfo=_netinfo, configurator=configurator, opts=options, dnss=dnss
+ )
if bridged and network in _netinfo.bridges:
net_ent_to_configure = net_ent.port
diff --git a/vdsm/network/configurators/ifcfg.py b/vdsm/network/configurators/ifcfg.py
index c2b0345..c342d90 100644
--- a/vdsm/network/configurators/ifcfg.py
+++ b/vdsm/network/configurators/ifcfg.py
@@ -503,7 +503,9 @@
if self.unifiedPersistence and utils.isOvirtNode():
node_fs.Config().persist(fileName)
- def _createConfFile(self, conf, name, ipv4, ipv6, mtu=None, **kwargs):
+ def _createConfFile(
+ self, conf, name, ipv4, ipv6, mtu=None, dnss=None, **kwargs
+ ):
""" Create ifcfg-* file with proper fields per device """
cfg = 'DEVICE=%s\n' % pipes.quote(name)
cfg += conf
@@ -536,6 +538,10 @@
elif ipv6.dhcpv6:
cfg += 'DHCPV6C=yes\n'
cfg += 'IPV6_AUTOCONF=%s\n' % _to_ifcfg_bool(ipv6.ipv6autoconf)
+ if dnss:
+ for i, dns in enumerate(dnss):
+ cfg += 'DNS' + str(i+1) + '=%s\n' % dns
+
BLACKLIST = ['TYPE', 'NAME', 'DEVICE', 'VLAN', 'bondingOptions',
'force', 'blockingdhcp', 'custom',
'connectivityCheck', 'connectivityTimeout',
--
To view, visit https://gerrit.ovirt.org/47843
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib9ea16d033cc07ef10c05868da3e6b4bcee56f74
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Genadi Chereshnya <gcheresh(a)redhat.com>
8 years, 6 months
