bugfix: set device limit by the mountpoint #2154
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2154 +/- ##
=========================================
- Coverage 64.46% 64.3% -0.16%
=========================================
Files 209 209
Lines 16680 16713 +33
=========================================
- Hits 10752 10748 -4
- Misses 4599 4637 +38
+ Partials 1329 1328 -1
|
storage/quota/prjquota.go
Outdated
| @@ -321,19 +321,30 @@ func (quota *PrjQuotaDriver) setDevLimit(dir string, devID uint64) (uint64, erro | |||
| return limit, nil | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Do we need to update the function in grpquota, it seems like a common interface function implement.
storage/quota/quota.go
Outdated
|
|
||
| // setDevLimit sets device storage upper limit in quota driver according to inpur dir. | ||
| func setDevLimit(dir string, devID uint64) (uint64, error) { | ||
| if limit, exist := devLimits[devID]; exist { |
There was a problem hiding this comment.
Do we need to add lock here to avoid race section reading?
|
LGTM |
pouchrobot
added
the
LGTM
storage/quota/quota.go
Outdated
| continue | ||
| } | ||
|
|
||
| if strings.HasPrefix(dir, parts[1]) && len(parts[1]) > len(mountPoint) { |
There was a problem hiding this comment.
What will happen if there are two reacords in procMountFile and the input dir name is /home/pouch? @rudyfly
/dev/sdb1 /home/pouch ext4 rw,relatime,prjquota,data=ordered 0 0
/dev/sdb2 /home/pouch/a/b/c ext4 rw,relatime,prjquota,data=ordered 0 0
Or is there any possibility that two similar records exist there?
There was a problem hiding this comment.
I have checked len(parts[1]) > len(mountPoint), It will choose the longest mount tab.
There was a problem hiding this comment.
Roger, How about adding this details in the comment of this function or the code block? @rudyfly
Otherwise, I think there will be confusions there.
| @@ -320,3 +329,90 @@ func loadQuotaIDs(repquotaOpt string) (map[uint32]struct{}, uint32, error) { | |||
| logrus.Infof("Load repquota ids: %d, list: %v", len(quotaIDs), quotaIDs) | |||
| return quotaIDs, minID, nil | |||
| } | |||
|
|
|||
| func getMountpoint(dir string) (string, error) { | |||
| var ( | |||
There was a problem hiding this comment.
codes duplicate with prjquota.CheckMountpoint
|
Could you rebase to the latest master? @rudyfly |
storage/quota/quota.go
Outdated
|
|
||
| // setDevLimit sets device storage upper limit in quota driver according to inpur dir. | ||
| func setDevLimit(dir string, devID uint64) (uint64, error) { | ||
| if limit, exist := devLimits[devID]; exist { |
There was a problem hiding this comment.
devLimits represent the device block size limitation
|
|
||
| // checkDevLimit checks if the device on which the input dir lies has already been recorded in driver. | ||
| func checkDevLimit(dir string, size uint64) error { | ||
| devID, err := system.GetDevID(dir) |
There was a problem hiding this comment.
duplicate code, we just call setDevLimit is enough, WDYT???
rudyfly
force-pushed
the
diskquota
branch
2 times, most recently
from
b3efeaa to
b36cc5b
Compare
|
@allencloud I have done rebase master. |
set device limit by the mountpoint. Signed-off-by: Rudy Zhang <[email protected]>
|
lgtm |
Ⅰ. Describe what this PR did
set device limit by the mountpoint.
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)
No need to add test case.
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews
Signed-off-by: Rudy Zhang [email protected]