Change in vdsm[master]: readlines - remove redundant pathExists check

abaron at redhat.com abaron at redhat.com
Wed Apr 10 21:23:24 UTC 2013


Ayal Baron has posted comments on this change.

Change subject: readlines - remove redundant pathExists check
......................................................................


Patch Set 3: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/fileSD.py
Line 123:                                       self._metafile))
Line 124:         except (IOError, OSError) as e:
Line 125:             if e.errno != errno.ENOENT:
Line 126:                 raise
Line 127:             self._log.warn("Could not retrieve metadata file for doamin in "
s/file //
Move this to the error flow (we shouldn't warn here as this method is called from create and it's fine that it's empty in that flow).

Need to fix persistentDict though to fail in case no metadata checksum is found (log and raise an error here instead of saying everything is fine):
251             if declaredChecksum is None:
252                 # No checksum in the metadata, let it through as is
253                 # FIXME : This is ugly but necessary, What we need is a class
254                 # method that creates the initial metadata. Then we can assume
255                 # that empty metadata is always invalid.
256                 self.log.warn("data has no embedded checksum - "
257                               "trust it as it is")
258                 self._isValid = True
259                 self._metadata = newMD
260                 return

Deleting all these lines would be insufficient as currently createStorageDomain flow uses the 'update' method which calls refresh which has the code above instead of flush which just writes without trying to first load the metadata.
So I believe it would be sufficient to just change blockSD.py create:
s/md.update(initialMetadata)/md.flush(initialMetadata)/

and fileSD.py _prepareMetadata:
s/md.update/md.flush/
Line 128:                            "path %s", self._metafile)
Line 129:             return []
Line 130: 
Line 131:     def writelines(self, metadata):


--
To view, visit http://gerrit.ovirt.org/13664
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id73f4bd177465f253eb4f65403b17ff7375e329c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Ohad Basan <obasan at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list