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 <[email protected]>
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 <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: humble devassy <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to