2:44 p.m.
Nir Soffer has uploaded a new change for review.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
vm: Avoid log spamming when drive format is undefined
Due to the dynamic way drive attribute are initialized, it is possible
that drive's format attribute is undefined. In this case _highwrite
fails repeatedly with noisy exception spamming vdsm log and syslog.
The purpose of _highwrite is to check if drive is too full and should be
extended before it is too late and the vm is paused. In this context, we
care only about block devices using 'cow' format. We could not care less
about drive without a format attribute, and there is no reason to spam
the log about this.
This patch use getattr to check if drive uses the cow format and make
the code more clear by extracting the check to new isExtendable method.
Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com>
---
M vdsm/vm.py
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/51/22551/1
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 1358b09..37b426a 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -1406,6 +1406,10 @@
def isDiskReplicationInProgress(self):
return hasattr(self, "diskReplicate")
+ def isExtendable(self):
+ # Note: format may not be defined during migration
+ return self.blockDev and getattr(self, 'format', None) == 'cow'
+
@property
def volExtensionChunk(self):
"""
@@ -2312,7 +2316,7 @@
extend = []
for drive in self._devices[DISK_DEVICES]:
- if not drive.blockDev or drive.format != 'cow':
+ if not drive.isExtendable():
continue
capacity, alloc, physical = self._dom.blockInfo(drive.path, 0)
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
9:46 a.m.
Federico Simoncelli has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1:
(1 comment)
....................................................
File vdsm/vm.py
Line 1407: return hasattr(self, "diskReplicate")
Line 1408:
Line 1409: def isExtendable(self):
Line 1410: # Note: format may not be defined during migration
Line 1411: return self.blockDev and getattr(self, 'format', None) ==
'cow'
There are tons of places where an AttributeError can happen, and we cannot fix them all
assuming potentially unsafe defaults.
I proposed to use __slots__, but also in that case the format cannot be assumed None or
"raw". It is mandatory.
The proper fix is to handle broken recovery files: if the parameter is missing we have to
find the value running a getVolumeInfo (or any other relevant command). In the same way we
could handle run/migrate requests that are lacking this value.
Line 1412:
Line 1413: @property
Line 1414: def volExtensionChunk(self):
Line 1415: """
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
5:56 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1:
(1 comment)
....................................................
File vdsm/vm.py
Line 1406: def isDiskReplicationInProgress(self):
Line 1407: return hasattr(self, "diskReplicate")
Line 1408:
Line 1409: def isExtendable(self):
Line 1410: # Note: format may not be defined during migration
on migration destination, I suppose.
Line 1411: return self.blockDev and getattr(self, 'format', None) ==
'cow'
Line 1412:
Line 1413: @property
Line 1414: def volExtensionChunk(self):
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
2:53 p.m.
Nir Soffer has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1:
This patch is a quick easy and safe fix. However for the long term solution I think we
should go with patch http://gerrit.ovirt.org/21036.
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:48 p.m.
Nir Soffer has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1:
This is a port of http://gerrit.ovirt.org/22518 after it was tested on user data center.
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
2:47 p.m.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6171/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5384/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6275/ : SUCCESS
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
12:21 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:31 a.m.
Federico Simoncelli has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1:
(1 comment)
....................................................
File vdsm/vm.py
Line 1407: return hasattr(self, "diskReplicate")
Line 1408:
Line 1409: def isExtendable(self):
Line 1410: # Note: format may not be defined during migration
Line 1411: return self.blockDev and getattr(self, 'format', None) ==
'cow'
It seems to me this is making the "format" attribute optional. I definitely
prefer to explicitly explode (forever) rather than keep running without extensions and
have no feedback whatsoever. If "format" got lost we have to find it back
(meaning that we have to understand in what flow it gets lost and fix it).
This issue is probably related to: https://bugzilla.redhat.com/show_bug.cgi?id=994534
Line 1412:
Line 1413: @property
Line 1414: def volExtensionChunk(self):
Line 1415: """
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9:03 a.m.
Nir Soffer has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1:
(1 comment)
....................................................
File vdsm/vm.py
Line 1407: return hasattr(self, "diskReplicate")
Line 1408:
Line 1409: def isExtendable(self):
Line 1410: # Note: format may not be defined during migration
Line 1411: return self.blockDev and getattr(self, 'format', None) ==
'cow'
This can explain why format is missing. But we cannot continue to explode in _highWrite. I
think that the proper fix is to validate input parameters in vm.Drive.__init__ and in the
code reading recovery files.
Line 1412:
Line 1413: @property
Line 1414: def volExtensionChunk(self):
Line 1415: """
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
9:02 a.m.
Itamar Heim has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1:
ping
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
4:14 a.m.
Allon Mureinik has posted comments on this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
8:28 a.m.
Itamar Heim has abandoned this change.
Change subject: vm: Avoid log spamming when drive format is undefined
......................................................................
Abandoned
abandoning per no reply. please restore if still relevant.
--
To view, visit http://gerrit.ovirt.org/22551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ia50e8af94b9c9b54332066a3f30999ce73e7a56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
3647
days inactive
3799
days old
vdsm-patches@lists.fedorahosted.org
11 comments
6 participants
participants (6)
-
Allon Mureinik -
Dan Kenigsberg -
Federico Simoncelli -
iheim＠redhat.com -
Jenkins CI RO -
Nir Soffer