Change in vdsm[ovirt-3.4]: fileSD: enable mailbox on file domains

Federico Simoncelli fsimonce at redhat.com
Tue Apr 22 23:14:16 UTC 2014


Hello Nir Soffer, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/26989

to review the following change.

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

fileSD: enable mailbox on file domains

Since we now support mixed-type storage pools (block and file) we
must enable the mailbox on file domains in order to satisfy the
extension requests needed by block domains.
On file domains the inbox and outbox files were prepared empty so
we also need to extend them to the proper size as soon as possible:

* on SPM start
* on master migration

Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1083476
Change-Id: Iba8036e33ed8718d05e9d83cad1d63949794f067
Signed-off-by: Federico Simoncelli <fsimonce at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/26414
Reviewed-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M vdsm/storage/blockSD.py
M vdsm/storage/fileSD.py
M vdsm/storage/localFsSD.py
M vdsm/storage/sd.py
M vdsm/storage/sp.py
5 files changed, 58 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/26989/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 9951f19..a27965e 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -431,10 +431,6 @@
         self._registerResourceNamespaces()
         self._lastUncachedSelftest = 0
 
-    @property
-    def requiresMailbox(self):
-        return True
-
     def _registerResourceNamespaces(self):
         """
         Register resources namespaces and create
diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py
index dabf73d..805391d 100644
--- a/vdsm/storage/fileSD.py
+++ b/vdsm/storage/fileSD.py
@@ -46,6 +46,14 @@
 
 METADATA_PERMISSIONS = 0o660
 
+# On file domains IDS and LEASES volumes don't need a fixed size (they are
+# allocated as we use them)
+FILE_SPECIAL_VOLUME_SIZES_MIB = sd.SPECIAL_VOLUME_SIZES_MIB.copy()
+FILE_SPECIAL_VOLUME_SIZES_MIB.update({
+    sd.IDS: 0,
+    sd.LEASES: 0,
+})
+
 # Specific stat(2) block size as defined in the man page
 ST_BYTES_PER_BLOCK = 512
 
@@ -176,6 +184,29 @@
                     "Lease permission change file '%s' failed: %s"
                     % (metaFile, e))
 
+    def prepareMailbox(self):
+        for mailboxFile in (sd.INBOX, sd.OUTBOX):
+            mailboxByteSize = (FILE_SPECIAL_VOLUME_SIZES_MIB[mailboxFile] *
+                               constants.MEGAB)
+            mailboxFilePath = os.path.join(self.domaindir,
+                                           sd.DOMAIN_META_DATA, mailboxFile)
+
+            try:
+                mailboxStat = self.oop.os.stat(mailboxFilePath)
+            except OSError as e:
+                if e.errno != os.errno.ENOENT:
+                    raise
+                prevMailboxFileSize = None
+            else:
+                prevMailboxFileSize = mailboxStat.st_size
+
+            if (prevMailboxFileSize is None
+                    or prevMailboxFileSize < mailboxByteSize):
+                self.log.info('preparing storage domain %s mailbox file %s '
+                              '(%s bytes)', mailboxFile, mailboxByteSize)
+                self.oop.truncateFile(
+                    mailboxFilePath, mailboxByteSize, METADATA_PERMISSIONS)
+
     @classmethod
     def _prepareMetadata(cls, domPath, sdUUID, domainName, domClass,
                          remotePath, storageType, version):
@@ -188,10 +219,11 @@
         procPool = oop.getProcessPool(sdUUID)
         procPool.fileUtils.createdir(metadataDir, 0o775)
 
-        for metaFile in sd.SPECIAL_VOLUME_SIZES_MIB.iterkeys():
+        for metaFile, metaSize in FILE_SPECIAL_VOLUME_SIZES_MIB.iteritems():
             try:
-                procPool.truncateFile(os.path.join(metadataDir, metaFile), 0,
-                                      METADATA_PERMISSIONS)
+                procPool.truncateFile(
+                    os.path.join(metadataDir, metaFile),
+                    metaSize * constants.MEGAB, METADATA_PERMISSIONS)
             except Exception as e:
                 raise se.StorageDomainMetadataCreationError(
                     "create meta file '%s' failed: %s" % (metaFile, str(e)))
diff --git a/vdsm/storage/localFsSD.py b/vdsm/storage/localFsSD.py
index 1066fb5..1abf018 100644
--- a/vdsm/storage/localFsSD.py
+++ b/vdsm/storage/localFsSD.py
@@ -37,6 +37,10 @@
         3: (clusterlock.LocalLock, True),
     }
 
+    @property
+    def supportsMailbox(self):
+        return False
+
     @classmethod
     def _preCreateValidation(cls, sdUUID, domPath, typeSpecificArg, version):
         # Some trivial resource validation
diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py
index ead883a..56c84cc 100644
--- a/vdsm/storage/sd.py
+++ b/vdsm/storage/sd.py
@@ -314,9 +314,16 @@
         if self.stat:
             threading.Thread(target=self.stat.stop).start()
 
+    def prepareMailbox(self):
+        """
+        This method has been introduced in order to prepare the mailbox
+        on those domains where the metadata for the inbox and outbox
+        wasn't allocated on creation.
+        """
+
     @property
-    def requiresMailbox(self):
-        return False
+    def supportsMailbox(self):
+        return True
 
     @property
     def supportsSparseness(self):
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index bd4cbbe..4bab1b9 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -299,15 +299,10 @@
                 # commands are allowed to run to prevent a race between the
                 # mailbox and the "self._setSecure() call"
 
-                # Create mailbox if SAN pool (currently not needed on nas)
-                # FIXME: Once pool contains mixed type domains (NFS + Block)
-                #        the mailbox will have to be created if there is an
-                #        active block domain in the pool or once one is
-                #        activated
-
-                #FIXME : Use a system wide grouping mechanism
-                if self.masterDomain.requiresMailbox and \
-                        self.lvExtendPolicy == "ON":
+                # FIXME : Use a system wide grouping mechanism
+                if (self.lvExtendPolicy == "ON"
+                        and self.masterDomain.supportsMailbox):
+                    self.masterDomain.prepareMailbox()
                     self.spmMailer = storage_mailbox.SPM_MailMonitor(self,
                                                                      maxHostID)
                     self.spmMailer.registerMessageType('xtnd', partial(
@@ -454,7 +449,8 @@
         if self.hsmMailer:
             return
 
-        if self.masterDomain.requiresMailbox and self.lvExtendPolicy == "ON":
+        if (self.lvExtendPolicy == "ON" and
+                self.masterDomain.supportsMailbox):
             self.hsmMailer = storage_mailbox.HSM_Mailbox(self.id, self.spUUID)
             self.log.debug("HSM mailbox ready for pool %s on master "
                            "domain %s", self.spUUID, self.masterDomain.sdUUID)
@@ -786,6 +782,10 @@
             self.log.debug('migration to the new master %s begins',
                            newmsd.sdUUID)
 
+            # Preparing the mailbox since the new master domain may be an
+            # old domain where the mailbox wasn't allocated
+            newmsd.prepareMailbox()
+
             # Mount new master file system
             newmsd.mountMaster()
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iba8036e33ed8718d05e9d83cad1d63949794f067
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.4
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list