Change in vdsm[master]: Check that underlying filesystem supports direct IO before c...

smizrahi at redhat.com smizrahi at redhat.com
Tue Jan 15 22:01:42 UTC 2013


Saggi Mizrahi has posted comments on this change.

Change subject: Check that underlying filesystem supports direct IO before creating a domain on it
......................................................................


Patch Set 5: (2 inline comments)

....................................................
Commit Message
Line 3: AuthorDate: 2012-12-05 11:11:22 -0500
Line 4: Commit:     Saggi Mizrahi <smizrahi at redhat.com>
Line 5: CommitDate: 2013-01-07 11:57:21 -0500
Line 6: 
Line 7: Check that underlying filesystem supports direct IO before creating a domain on it
Don't take it personally but I honestly don't care.
If I do another iteration I'll be sure to change it.
Line 8: 
Line 9: This is done so that we don't allow posixfs domains on top of targets
Line 10: that don't support direct IO.
Line 11: 


....................................................
File vdsm/storage/fileSD.py
Line 133:         self.remotePath = os.path.basename(self.mountpoint)
Line 134:         self.metafile = os.path.join(domainPath, sd.DOMAIN_META_DATA,
Line 135:                                      sd.METADATA)
Line 136: 
Line 137:         self.validateFileSystemFeatures()
You can't move it to the connect phase because that has nothing to do with connecting to things. We have already been down this path and we know it's bad.

You can move it to create if you don't mind someone copying off a domain and other such nonsense.

In any case, it is the correct place to put it. Moving it anywhere else is just premature optimization.

I would have agreed to move it if I didn't already put a lot of effort into making sure that domain objects are only created when they are not already in the cache. This means that a domain object is only created when we intend to reload the metadata and such nonsense. Adding a 1 block size DIO write to whole shebang is not going to break us.
Line 138: 
Line 139:         metadata = FileSDMetadata(self.metafile)
Line 140:         sdUUID = metadata[sd.DMDK_SDUUID]
Line 141:         domaindir = os.path.join(self.mountpoint, sdUUID)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf14d1c4737a88e693e5bebb896aef382b8b424c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: humble devassy <hchiramm at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list