Change in vdsm[master]: fileSD: enable mailbox on file domains

nsoffer at redhat.com nsoffer at redhat.com
Tue Apr 22 10:00:07 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: fileSD: enable mailbox on file domains
......................................................................


Patch Set 5: Code-Review-1

(5 comments)

-1 for unneeded class method, some other minor things can be improved.

http://gerrit.ovirt.org/#/c/26414/5/vdsm/storage/fileSD.py
File vdsm/storage/fileSD.py:

Line 51: FILE_SPECIAL_VOLUME_SIZES_MIB = sd.SPECIAL_VOLUME_SIZES_MIB.copy()
Line 52: FILE_SPECIAL_VOLUME_SIZES_MIB.update({
Line 53:     sd.IDS: 0,
Line 54:     sd.LEASES: 0,
Line 55: })
Is this related to this patch? result of a rabase?
Line 56: 
Line 57: # Specific stat(2) block size as defined in the man page
Line 58: ST_BYTES_PER_BLOCK = 512
Line 59: 


Line 187:     def prepareMailbox(self):
Line 188:         self._prepareMailbox(self.domaindir, self.sdUUID)
Line 189: 
Line 190:     @classmethod
Line 191:     def _prepareMailbox(cls, domPath, sdUUID):
Now that we do not invoke this from _prepareMatadata class method, this does not need to be a class method.
Line 192:         procPool = oop.getProcessPool(sdUUID)
Line 193: 
Line 194:         for mailboxFile in (sd.INBOX, sd.OUTBOX):
Line 195:             mailboxByteSize = (FILE_SPECIAL_VOLUME_SIZES_MIB[mailboxFile] *


Line 228:         for metaFile, metaSize in FILE_SPECIAL_VOLUME_SIZES_MIB.iteritems():
Line 229:             try:
Line 230:                 procPool.truncateFile(
Line 231:                     os.path.join(metadataDir, metaFile),
Line 232:                     metaSize * constants.MEGAB, METADATA_PERMISSIONS)
Unrelated changes from a rebase?
Line 233:             except Exception as e:
Line 234:                 raise se.StorageDomainMetadataCreationError(
Line 235:                     "create meta file '%s' failed: %s" % (metaFile, str(e)))
Line 236: 


http://gerrit.ovirt.org/#/c/26414/5/vdsm/storage/sd.py
File vdsm/storage/sd.py:

Line 319:         This method has been introduced in order to prepare the mailbox
Line 320:         on those domains where the metadata for the inbox and outbox
Line 321:         wasn't allocated on creation.
Line 322:         """
Line 323: 
Why not use @property like supportsSparsennes?
Line 324:     def supportsMailbox(self):
Line 325:         return True
Line 326: 
Line 327:     @property


http://gerrit.ovirt.org/#/c/26414/5/vdsm/storage/sp.py
File vdsm/storage/sp.py:

Line 792:         try:
Line 793:             self.log.debug('migration to the new master %s begins',
Line 794:                            newmsd.sdUUID)
Line 795: 
Line 796:             newmsd.prepareMailbox()
This needs a comment explaining that the mailbox may be prepare or not. Alternatively, renaming prepareMailbox() to  prepareMailboxIfNeeded() or maybePrepareMailbox(), can make this more clear.
Line 797: 
Line 798:             # Mount new master file system
Line 799:             newmsd.mountMaster()
Line 800: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba8036e33ed8718d05e9d83cad1d63949794f067
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list