Martin Polednik has uploaded a new change for review.
Change subject: caps: report realtime kernel status ......................................................................
caps: report realtime kernel status
Determining whether system is suitable for RT operation is complex task consisting of multiple variables. One of the necessary but insufficient indications is RT_PREEMPT patch within running kernel, as shown by "RT" string within kernel's release string.
We now report realtimeKernel in caps, using os.uname() to gather information about runtime kernel; allowing engine to act based on this information.
Change-Id: I016ae8a66963a973a1a2f78a9c2706af84f804d1 Signed-off-by: Martin Polednik mpolednik@redhat.com --- M lib/api/vdsm-api.yml M lib/vdsm/osinfo.py M vdsm/caps.py 3 files changed, 19 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/65715/1
diff --git a/lib/api/vdsm-api.yml b/lib/api/vdsm-api.yml index f311b17..e44e06a 100644 --- a/lib/api/vdsm-api.yml +++ b/lib/api/vdsm-api.yml @@ -6489,6 +6489,13 @@ name: kernelArgs type: string added: '4.0' + + - defaultvalue: no-default + description: Indication whether the running kernel is compiled + with RT_PREEMPT enabled + name: realtimeKernel + type: string + added: '4.1' type: object
VmShortStatus: &VmShortStatus diff --git a/lib/vdsm/osinfo.py b/lib/vdsm/osinfo.py index 23bb2c2..f5342a4 100644 --- a/lib/vdsm/osinfo.py +++ b/lib/vdsm/osinfo.py @@ -25,6 +25,8 @@ import logging import os
+from collections import namedtuple + from vdsm import utils
# For debian systems we can use python-apt if available @@ -46,6 +48,9 @@ glusterEnabled = True except ImportError: glusterEnabled = False + + +KernelFlags = namedtuple('KernelFlags', 'version, realtime')
class OSName: @@ -178,7 +183,7 @@
def package_versions(): - pkgs = {'kernel': _runtime_kernel_version()} + pkgs = {'kernel': runtime_kernel_flags().version}
if _release_name() in (OSName.RHEVH, OSName.OVIRT, OSName.FEDORA, OSName.RHEL, OSName.POWERKVM): @@ -243,7 +248,7 @@ return pkgs
-def _runtime_kernel_version(): +def runtime_kernel_flags(): ret = os.uname() try: ver, rel = ret[2].split('-', 1) @@ -251,4 +256,6 @@ logging.error('kernel release not found', exc_info=True) ver, rel = '0', '0'
- return dict(version=ver, release=rel) + realtime = 'RT' in ret[3] + + return KernelFlags(dict(version=ver, release=rel), realtime) diff --git a/vdsm/caps.py b/vdsm/caps.py index 703b500..57ea7e9 100644 --- a/vdsm/caps.py +++ b/vdsm/caps.py @@ -168,7 +168,8 @@ caps['operatingSystem'] = osinfo.version() caps['uuid'] = host.uuid() caps['packages2'] = osinfo.package_versions() - caps['kernelArgs'] = osinfo.kernel_args() + caps['realtimeKernel'] = str( + osinfo.runtime_kernel_flags().realtime).lower() caps['emulatedMachines'] = machinetype.emulated_machines( cpuarch.effective()) caps['ISCSIInitiatorName'] = _getIscsiIniName()
gerrit-hooks has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 1:
* Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Francesco Romani has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 1: Code-Review+1
(3 comments)
https://gerrit.ovirt.org/#/c/65715/1/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
PS1, Line 6495: with RT_PREEMPT enabled I wonder if you need to document those details here. I mean, do the consumers of the API care about how Vdsm computes this field? If so, why?
PS1, Line 6497: type I think you may want to use:
type: string datatype: boolean
https://gerrit.ovirt.org/#/c/65715/1/lib/vdsm/osinfo.py File lib/vdsm/osinfo.py:
PS1, Line 261: dict why dict() and not literal syntax? Just curious, I don't think this has any real impact
Martin Polednik has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 1:
(3 comments)
https://gerrit.ovirt.org/#/c/65715/1/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
PS1, Line 6495: with RT_PREEMPT enabled
I wonder if you need to document those details here.
This is not really about computation of the field, but slightly-more-than-required explanation what the value is. The reason for the amount of care is to avoid any API consumer to consider the value as final statement on RT operation status -- it's only a little piece of the puzzle.
Why mentioning the kernel config option of just saying "realtime kernel blah blah"? Mostly due to RT wiki, where RT_PREEMPT google query will most likely return https://rt.wiki.kernel.org/index.php/RT_PREEMPT_HOWTO which is one of the best sources of information regarding RT up to date.
PS1, Line 6497: type
I think you may want to use:
"True!"
https://gerrit.ovirt.org/#/c/65715/1/lib/vdsm/osinfo.py File lib/vdsm/osinfo.py:
PS1, Line 261: dict
why dict() and not literal syntax? Just curious, I don't think this has any
I'm afraid the only reason is that it was this way since I remember the code. If you feel that it's improvement I can change the syntax in the patch.
gerrit-hooks has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 2:
* Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, no bug url/s found * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Francesco Romani has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/65715/1/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
PS1, Line 6495: with RT_PREEMPT enabled
This is not really about computation of the field, but slightly-more-than-r
Fair enough, since it is description we can cheaply adjust later on.
Francesco Romani has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 2: Code-Review+1
(1 comment)
Looks good to me. Let's have someone from infra approving the schema change.
https://gerrit.ovirt.org/#/c/65715/2/lib/vdsm/osinfo.py File lib/vdsm/osinfo.py:
PS2, Line 261: dict I'm fine with this, I don't think using literal syntax has any significant benefit.
Piotr Kliczewski has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/65715/2/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
PS2, Line 6497: string Why not to use boolean directly here. I see that we make it a string ourselves. Is there any reason?
Martin Polednik has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 2:
(1 comment)
https://gerrit.ovirt.org/#/c/65715/2/lib/api/vdsm-api.yml File lib/api/vdsm-api.yml:
PS2, Line 6497: string
Why not to use boolean directly here. I see that we make it a string oursel
For some reason (that I think is XML RPC) we've always went with boolean (lowercase) encapsulated within string. Is it safe to use raw boolean now? Should we?
gerrit-hooks has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 3:
* Update Tracker::IGNORE, no bug url/s found * Check Bug-Url::IGNORE, no bug url/s found * Check Public Bug::WARN, no public bug url found * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Martin Polednik has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 3: Verified+1
Tested on both RT and non-RT kernels. The check itself is not anywhere near "robust", but it seems good enough for present, past and future kernels (and I have no idea how to make it even better without parsing kconfig).
Piotr Kliczewski has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 3: Code-Review+1
api change looks good
Francesco Romani has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 3: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: caps: report realtime kernel status ......................................................................
caps: report realtime kernel status
Determining whether system is suitable for RT operation is complex task consisting of multiple variables. One of the necessary but insufficient indications is RT_PREEMPT patch within running kernel, as shown by "RT" string within kernel's release string.
We now report realtimeKernel in caps, using os.uname() to gather information about runtime kernel; allowing engine to act based on this information.
Change-Id: I016ae8a66963a973a1a2f78a9c2706af84f804d1 Signed-off-by: Martin Polednik mpolednik@redhat.com Reviewed-on: https://gerrit.ovirt.org/65715 Continuous-Integration: Jenkins CI Reviewed-by: Piotr Kliczewski piotr.kliczewski@gmail.com Reviewed-by: Francesco Romani fromani@redhat.com --- M lib/api/vdsm-api.yml M lib/vdsm/osinfo.py M vdsm/caps.py 3 files changed, 18 insertions(+), 4 deletions(-)
Approvals: Piotr Kliczewski: Looks good to me, but someone else must approve Jenkins CI: Passed CI tests Francesco Romani: Looks good to me, approved Martin Polednik: Verified
gerrit-hooks has posted comments on this change.
Change subject: caps: report realtime kernel status ......................................................................
Patch Set 4:
* update_tracker: OK * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org