Xavi Francisco has uploaded a new change for review.
Change subject: storage: use info log level to report missing checksum ......................................................................
storage: use info log level to report missing checksum
When logging that the storage metadata is missing the checksum the info level should be used instead of the warning level.
This may happen when, for recovery purposes, the metadata must be edited by hand and the checksum information removed. In that case, as it is done in purpose, a warning should not be used.
Change-Id: Ic6627fef453a62603d93bfb8aee66ebedcbbf190 Bug-Url: https://bugzilla.redhat.com/1073989 Signed-off-by: Xavi Francisco xfrancis@redhat.com --- M vdsm/storage/persistentDict.py 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/27528/1
diff --git a/vdsm/storage/persistentDict.py b/vdsm/storage/persistentDict.py index 3464823..c8a86df 100644 --- a/vdsm/storage/persistentDict.py +++ b/vdsm/storage/persistentDict.py @@ -253,7 +253,7 @@ # FIXME : This is ugly but necessary, What we need is a class # method that creates the initial metadata. Then we can assume # that empty metadata is always invalid. - self.log.warn("data has no embedded checksum - " + self.log.info("data has no embedded checksum - " "trust it as it is") self._isValid = True self._metadata = newMD
Nir Soffer has posted comments on this change.
Change subject: storage: use info log level to report missing checksum ......................................................................
Patch Set 1: Code-Review-1
Metadata should have a checksum - if someone edited the the file manually, I want to know that, so this *should* be a warning.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: storage: use info log level to report missing checksum ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/8665/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/7875/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/8796/ : FAILURE
Allon Mureinik has posted comments on this change.
Change subject: storage: use info log level to report missing checksum ......................................................................
Patch Set 1:
I don't think that a WARNING is appropriate for a missing checksum, which we intentionally created.
Federico - your two cents?
Federico Simoncelli has posted comments on this change.
Change subject: storage: use info log level to report missing checksum ......................................................................
Patch Set 1: Code-Review-1
The warning must remain a warning.
What we should do is to check if we need to display it or not, because when the metadata is empty (e.g. bz1073989) there's also no checksum to check.
The metadata being empty is something expected and it may deserve just a debug message.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: persistentDict: if no metadata do not calculate the checksum ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9162/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9947/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10102/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5029/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3186/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_storage-functional-tests-localfs_ge... : SUCCESS
Allon Mureinik has posted comments on this change.
Change subject: persistentDict: if no metadata do not calculate the checksum ......................................................................
Patch Set 2: Code-Review+1
Nir Soffer has posted comments on this change.
Change subject: persistentDict: if no metadata do not calculate the checksum ......................................................................
Patch Set 2:
(1 comment)
Looks good, but log message can be refined.
http://gerrit.ovirt.org/#/c/27528/2/vdsm/storage/persistentDict.py File vdsm/storage/persistentDict.py:
Line 249: newMD[key] = value Line 250: Line 251: if not newMD: Line 252: self.log.debug("Empty metadata - " Line 253: "No need to compute the checksum") There is no need to talk about the checksum in this case. Just "Empty metadata" Line 254: self._isValid = True Line 255: self._metadata = newMD Line 256: return Line 257:
Nir Soffer has posted comments on this change.
Change subject: persistentDict: if no metadata do not calculate the checksum ......................................................................
Patch Set 2:
(2 comments)
The commit message needs minor fixes.
http://gerrit.ovirt.org/#/c/27528/2//COMMIT_MSG Commit Message:
Line 5: CommitDate: 2014-06-12 11:57:07 +0200 Line 6: Line 7: persistentDict: if no metadata do not calculate the checksum Line 8: Line 9: When parsingºsing the storage metadata, if it's empty, accept the parsingºsing -> parsing Line 10: metadata as valid, and log that there's no metadata to parse. In Line 11: this case it's not necessary to raise a warning informing no checksum Line 12: is found as there is no metadata. Line 13:
Line 7: persistentDict: if no metadata do not calculate the checksum Line 8: Line 9: When parsingºsing the storage metadata, if it's empty, accept the Line 10: metadata as valid, and log that there's no metadata to parse. In Line 11: this case it's not necessary to raise a warning informing no checksum raise a warning -> log a warning Line 12: is found as there is no metadata. Line 13: Line 14: Change-Id: Ic6627fef453a62603d93bfb8aee66ebedcbbf190 Line 15: Bug-Url: https://bugzilla.redhat.com/1073989
Nir Soffer has posted comments on this change.
Change subject: persistentDict: if no metadata do not calculate the checksum ......................................................................
Patch Set 2:
(1 comment)
http://gerrit.ovirt.org/#/c/27528/2//COMMIT_MSG Commit Message:
Line 3: AuthorDate: 2014-05-08 17:26:42 +0200 Line 4: Commit: Xavi Francisco xfrancis@redhat.com Line 5: CommitDate: 2014-06-12 11:57:07 +0200 Line 6: Line 7: persistentDict: if no metadata do not calculate the checksum Should be: Do not warn about missing checksum when metadata is empty Line 8: Line 9: When parsingºsing the storage metadata, if it's empty, accept the Line 10: metadata as valid, and log that there's no metadata to parse. In Line 11: this case it's not necessary to raise a warning informing no checksum
oVirt Jenkins CI Server has posted comments on this change.
Change subject: persistentDict: Do not warn about missing checksum when metadata is empty ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9242/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10026/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10181/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5108/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_storage-functional-tests-localfs_ge... : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3265/ : SUCCESS
Nir Soffer has posted comments on this change.
Change subject: persistentDict: Do not warn about missing checksum when metadata is empty ......................................................................
Patch Set 3: Code-Review+1
Federico Simoncelli has posted comments on this change.
Change subject: persistentDict: Do not warn about missing checksum when metadata is empty ......................................................................
Patch Set 3: Code-Review+2
Xavi Francisco has posted comments on this change.
Change subject: persistentDict: Do not warn about missing checksum when metadata is empty ......................................................................
Patch Set 3: Verified+1
To verify: 1. Pick a NFS storage domain 2. (Optional) Backup the metadata file 3. Clear the contents of the metadata file 4. "Empty metadata" message will appear in logs
Dan Kenigsberg has submitted this change and it was merged.
Change subject: persistentDict: Do not warn about missing checksum when metadata is empty ......................................................................
persistentDict: Do not warn about missing checksum when metadata is empty
When parsing the storage metadata, when none is present, accept the metadata as valid and log that there's no metadata. In this case do not log a warning informing no checksum is found as there is no metadata.
Change-Id: Ic6627fef453a62603d93bfb8aee66ebedcbbf190 Bug-Url: https://bugzilla.redhat.com/1073989 Signed-off-by: Xavi Francisco xfrancis@redhat.com Reviewed-on: http://gerrit.ovirt.org/27528 Reviewed-by: Nir Soffer nsoffer@redhat.com Reviewed-by: Federico Simoncelli fsimonce@redhat.com --- M vdsm/storage/persistentDict.py 1 file changed, 6 insertions(+), 0 deletions(-)
Approvals: Xavi Francisco: Verified Nir Soffer: Looks good to me, but someone else must approve Federico Simoncelli: Looks good to me, approved
Dan Kenigsberg has submitted this change and it was merged.
Change subject: persistentDict: Do not warn about missing checksum when metadata is empty ......................................................................
persistentDict: Do not warn about missing checksum when metadata is empty
When parsing the storage metadata, when none is present, accept the metadata as valid and log that there's no metadata. In this case do not log a warning informing no checksum is found as there is no metadata.
Change-Id: Ic6627fef453a62603d93bfb8aee66ebedcbbf190 Bug-Url: https://bugzilla.redhat.com/1073989 Signed-off-by: Xavi Francisco xfrancis@redhat.com Reviewed-on: http://gerrit.ovirt.org/27528 Reviewed-by: Nir Soffer nsoffer@redhat.com Reviewed-by: Federico Simoncelli fsimonce@redhat.com --- M vdsm/storage/persistentDict.py 1 file changed, 6 insertions(+), 0 deletions(-)
Approvals: Xavi Francisco: Verified Nir Soffer: Looks good to me, but someone else must approve Federico Simoncelli: Looks good to me, approved
oVirt Jenkins CI Server has posted comments on this change.
Change subject: persistentDict: Do not warn about missing checksum when metadata is empty ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1453/ : SUCCESS
vdsm-patches@lists.fedorahosted.org
-
Allon Mureinik -
Dan Kenigsberg -
Federico Simoncelli -
Jenkins CI RO -
Nir Soffer -
xfrancis＠redhat.com