fileSD: Fix remotePath in SD metadata (V2)

Adam Litke agl at us.ibm.com
Wed Sep 28 15:02:46 UTC 2011


On Tue, Sep 27, 2011 at 05:05:08PM -0400, Ayal Baron wrote:
> 
> 
> ----- Original Message -----
> > Changes since V1:
> >  - Derive the remotePath from self.mountpoint instead of using the
> >  metadata
> > 
> > The current method for gathering a LOCALFS Storage Domain's
> > remotePath property
> > does not work because these domains are connected with a symlink, not
> > a mount.
> > Fix up the current code so that it handles links and mountpoints.
> > 
> > In the code I have noticed some sentiments that path information
> > should be
> > removed from the storage domain metadata.  I strongly disagree with
> > this idea.
> > The path is a critical piece of information.  End users will care a
> > lot about
> > the path because it is where their images are located.  This
> > informaton is also
> > useful for calling connectStorageServer() and
> > disconnectStorageServer().
> > 
> > Signed-off-by: Adam Litke <agl at us.ibm.com>
> > 
> > diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py
> > index 35f7ab3..e904bfe 100644
> > --- a/vdsm/storage/fileSD.py
> > +++ b/vdsm/storage/fileSD.py
> > @@ -38,7 +38,9 @@ import time
> >  REMOTE_PATH = "REMOTE_PATH"
> >  
> >  FILE_SD_MD_FIELDS = sd.SD_MD_FIELDS.copy()
> > -# TBD: Do we really need this key?
> > +# Do we really need this key?
> > +# Answer: Yes.  The path is an important part of the SD metadata and
> > is useful
> > +# information for end-users.  It can also be used to manage storage
> > connections.
> 
> Comment is incorrect.  We still do not want this key in the metadata

Ok.

> 
> >  FILE_SD_MD_FIELDS[REMOTE_PATH] = (str, str)
> >  
> >  def getDomUuidFromMetafilePath(metafile):
> > @@ -224,6 +226,19 @@ class FileStorageDomain(sd.StorageDomain):
> >      def getRemotePath(self):
> >          return self.remotePath
> >  
> > +    def mountToRemotePath(self):
> 
> I'm not clear on the name seeing as in a local storage scenario this wouldn't be a mount
Here I am trying to use a function that works both for mounts and links.  I can
rework it to use a specialized function for each sub-class (localFileSD and
NfsSD).

> 
> > +        """
> > +        Get the remote path based on the storage mountpoint.
> 
> Again, not a mountpoint
> 
> > +        Handle symlinks and mounts.
> > +        """
> > +        if os.path.islink(self.mountpoint):
> > +            return os.readlink(self.mountpoint)
> doesn't self.remotePath already contain the info you're looking for in this case?

No.  It contains the transformed path (ie. _var_lib_vdsm_storage).  I want the
real path.

> 
> seeing as we have separate classes for localFsSD and nfsSD, why not implement this function in each file separately? (getInfo would call self.getRemotePath())
> 

I will rework as you suggest, but getRemotePath isn't what I want (see above).  

> > +        elif os.path.isdir(self.mountpoint):
> > +            for mount in fileUtils.getMounts():
> > +                if self.mountpoint == mount[1]:
> > +                    return mount[0]
> > +        return ''
> > +
> >      def getInfo(self):
> >          """
> >          Get storage domain info
> > @@ -232,13 +247,7 @@ class FileStorageDomain(sd.StorageDomain):
> >          # First call parent getInfo() - it fills in all the common
> >          details
> >          info = sd.StorageDomain.getInfo(self)
> >          # Now add fileSD specific data
> > -        info['remotePath'] = ''
> > -        mounts = fileUtils.getMounts()
> > -        for mount in mounts:
> > -            if self.mountpoint == mount[1]:
> > -                info['remotePath'] = mount[0]
> > -                break
> > -
> > +        info['remotePath'] = self.mountToRemotePath()
> >          return info
> >  
> >      def getStats(self):
> > --
> > Adam Litke <agl at us.ibm.com>
> > IBM Linux Technology Center
> > 
> > _______________________________________________
> > vdsm-patches mailing list
> > vdsm-patches at lists.fedorahosted.org
> > https://fedorahosted.org/mailman/listinfo/vdsm-patches
> > 
> _______________________________________________
> vdsm-patches mailing list
> vdsm-patches at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/vdsm-patches

-- 
Adam Litke <agl at us.ibm.com>
IBM Linux Technology Center


More information about the vdsm-patches mailing list