Skip to content

Commit 5e61f82

Browse files
authored
Merge pull request #729 from mtrmac/quota-options
Fix quota option handling
2 parents cbaa5f4 + 1c07506 commit 5e61f82

2 files changed

Lines changed: 75 additions & 87 deletions

File tree

storage/drivers/btrfs/btrfs.go

Lines changed: 45 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -483,15 +483,20 @@ func (d *Driver) CreateFromTemplate(id, template string, templateIDMappings *idt
483483
// CreateReadWrite creates a layer that is writable for use as a container
484484
// file system.
485485
func (d *Driver) CreateReadWrite(id, parent string, opts *graphdriver.CreateOpts) error {
486-
return d.create(id, parent, opts, true)
486+
return d.create(id, parent, opts, false)
487487
}
488488

489489
// Create the filesystem with given id.
490490
func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error {
491-
return d.create(id, parent, opts, false)
491+
return d.create(id, parent, opts, true)
492492
}
493493

494-
func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, applyDriverDefaultQuota bool) error {
494+
func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnly bool) error {
495+
quota, err := d.parseStorageOpt(opts, readOnly)
496+
if err != nil {
497+
return err
498+
}
499+
495500
quotas := d.quotasDir()
496501
subvolumes := d.subvolumesDir()
497502
if err := os.MkdirAll(subvolumes, 0o700); err != nil {
@@ -518,34 +523,14 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, applyDr
518523
}
519524
}
520525

521-
var storageOpt map[string]string
522-
if opts != nil {
523-
storageOpt = opts.StorageOpt
524-
}
525-
526-
var quotaSize uint64
527-
var needQuota bool
528-
if _, ok := storageOpt["size"]; ok {
529-
driver := &Driver{}
530-
if err := d.parseStorageOpt(storageOpt, driver); err != nil {
531-
return err
532-
}
533-
quotaSize = driver.options.size
534-
needQuota = true
535-
}
536-
if !needQuota && applyDriverDefaultQuota && d.options.size > 0 {
537-
quotaSize = d.options.size
538-
needQuota = true
539-
}
540-
if needQuota {
541-
layerDriver := &Driver{options: btrfsOptions{size: quotaSize}}
542-
if err := d.setStorageSize(path.Join(subvolumes, id), layerDriver); err != nil {
526+
if quota != nil {
527+
if err := d.setStorageSize(path.Join(subvolumes, id), *quota); err != nil {
543528
return err
544529
}
545530
if err := os.MkdirAll(quotas, 0o700); err != nil {
546531
return err
547532
}
548-
if err := os.WriteFile(path.Join(quotas, id), []byte(fmt.Sprint(quotaSize)), 0o644); err != nil {
533+
if err := os.WriteFile(path.Join(quotas, id), []byte(fmt.Sprint(quota.size)), 0o644); err != nil {
549534
return err
550535
}
551536
}
@@ -558,40 +543,63 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, applyDr
558543
return label.Relabel(path.Join(subvolumes, id), mountLabel, false)
559544
}
560545

561-
// Parse btrfs storage options
562-
func (d *Driver) parseStorageOpt(storageOpt map[string]string, driver *Driver) error {
546+
// layerQuota contains per-layer quota settings.
547+
type layerQuota struct {
548+
size uint64
549+
}
550+
551+
// parseStorageOpt parses CreateOpts.StorageOpt.
552+
// Returns a *layerQuota if a quota should be applied, nil otherwise.
553+
func (d *Driver) parseStorageOpt(opts *graphdriver.CreateOpts, readOnly bool) (*layerQuota, error) {
554+
var storageOpt map[string]string = nil // Iterating over a nil map is safe
555+
if opts != nil {
556+
storageOpt = opts.StorageOpt
557+
}
558+
559+
res := layerQuota{}
560+
needQuota := false
561+
562+
if !readOnly && d.options.size > 0 {
563+
res.size = d.options.size
564+
needQuota = true
565+
}
566+
563567
// Read size to change the subvolume disk quota per container
564568
for key, val := range storageOpt {
565569
key := strings.ToLower(key)
566570
switch key {
567571
case "size":
568572
size, err := units.RAMInBytes(val)
569573
if err != nil {
570-
return err
574+
return nil, err
571575
}
572-
driver.options.size = uint64(size)
576+
res.size = uint64(size)
577+
needQuota = true
573578
default:
574-
return fmt.Errorf("unknown option %s (%q)", key, storageOpt)
579+
return nil, fmt.Errorf("unknown option %s (%q)", key, storageOpt)
575580
}
576581
}
577582

578-
return nil
583+
if needQuota {
584+
return &res, nil
585+
}
586+
return nil, nil
579587
}
580588

581589
// Set btrfs storage size
582-
func (d *Driver) setStorageSize(dir string, driver *Driver) error {
583-
if driver.options.size <= 0 {
584-
return fmt.Errorf("btrfs: invalid storage size: %s", units.HumanSize(float64(driver.options.size)))
590+
func (d *Driver) setStorageSize(dir string, quota layerQuota) error {
591+
if quota.size <= 0 {
592+
return fmt.Errorf("btrfs: invalid storage size: %s", units.HumanSize(float64(quota.size)))
585593
}
586-
if d.options.minSpace > 0 && driver.options.size < d.options.minSpace {
594+
if d.options.minSpace > 0 && quota.size < d.options.minSpace {
587595
return fmt.Errorf("btrfs: storage size cannot be less than %s", units.HumanSize(float64(d.options.minSpace)))
588596
}
589597

590598
if err := d.enableQuota(); err != nil {
591599
return err
592600
}
593601

594-
if err := subvolLimitQgroup(dir, driver.options.size); err != nil {
602+
if err := subvolLimitQgroup(dir, quota.size); err != nil {
595603
return err
596604
}
597605

storage/drivers/overlay/overlay.go

Lines changed: 30 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -991,46 +991,16 @@ func (d *Driver) CreateReadWrite(id, parent string, opts *graphdriver.CreateOpts
991991
return fmt.Errorf("--storage-opt is supported only for overlay over xfs with 'pquota' mount option")
992992
}
993993

994-
if opts == nil {
995-
opts = &graphdriver.CreateOpts{
996-
StorageOpt: map[string]string{},
997-
}
998-
}
999-
1000994
if d.options.forceMask != nil && d.options.mountProgram == "" {
1001995
return fmt.Errorf("overlay: force_mask option for writeable layers is only supported with a mount_program")
1002996
}
1003997

1004-
if _, ok := opts.StorageOpt["size"]; !ok {
1005-
if opts.StorageOpt == nil {
1006-
opts.StorageOpt = map[string]string{}
1007-
}
1008-
opts.StorageOpt["size"] = strconv.FormatUint(d.options.quota.Size, 10)
1009-
}
1010-
1011-
if _, ok := opts.StorageOpt["inodes"]; !ok {
1012-
if opts.StorageOpt == nil {
1013-
opts.StorageOpt = map[string]string{}
1014-
}
1015-
opts.StorageOpt["inodes"] = strconv.FormatUint(d.options.quota.Inodes, 10)
1016-
}
1017-
1018998
return d.create(id, parent, opts, false)
1019999
}
10201000

10211001
// Create is used to create the upper, lower, and merge directories required for overlay fs for a given id.
10221002
// The parent filesystem is used to configure these directories for the overlay.
10231003
func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr error) {
1024-
if opts != nil && len(opts.StorageOpt) != 0 {
1025-
if _, ok := opts.StorageOpt["size"]; ok {
1026-
return fmt.Errorf("--storage-opt size is only supported for ReadWrite Layers")
1027-
}
1028-
1029-
if _, ok := opts.StorageOpt["inodes"]; ok {
1030-
return fmt.Errorf("--storage-opt inodes is only supported for ReadWrite Layers")
1031-
}
1032-
}
1033-
10341004
return d.create(id, parent, opts, true)
10351005
}
10361006

@@ -1078,6 +1048,11 @@ func (d *Driver) getLayerPermissions(parent string, uidMaps, gidMaps []idtools.I
10781048
}
10791049

10801050
func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnly bool) (retErr error) {
1051+
quota, err := d.parseStorageOpt(opts, readOnly) // Do this even for read-only layers, to allow rejecting quota options
1052+
if err != nil {
1053+
return err
1054+
}
1055+
10811056
dir, homedir, _ := d.dir2(id, readOnly)
10821057

10831058
disableQuota := readOnly
@@ -1127,19 +1102,6 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl
11271102
}()
11281103

11291104
if d.quotaCtl != nil && !disableQuota {
1130-
quota := quota.Quota{}
1131-
if opts != nil && len(opts.StorageOpt) > 0 {
1132-
driver := &Driver{}
1133-
if err := d.parseStorageOpt(opts.StorageOpt, driver); err != nil {
1134-
return err
1135-
}
1136-
if driver.options.quota.Size > 0 {
1137-
quota.Size = driver.options.quota.Size
1138-
}
1139-
if driver.options.quota.Inodes > 0 {
1140-
quota.Inodes = driver.options.quota.Inodes
1141-
}
1142-
}
11431105
// Set container disk quota limit
11441106
// If it is set to 0, we will track the disk usage, but not enforce a limit
11451107
if err := d.quotaCtl.SetQuota(dir, quota); err != nil {
@@ -1226,29 +1188,47 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, readOnl
12261188
}
12271189

12281190
// Parse overlay storage options
1229-
func (d *Driver) parseStorageOpt(storageOpt map[string]string, driver *Driver) error {
1191+
func (d *Driver) parseStorageOpt(opts *graphdriver.CreateOpts, readOnly bool) (quota.Quota, error) {
1192+
var storageOpt map[string]string = nil // Iterating over a nil map is safe
1193+
if opts != nil {
1194+
storageOpt = opts.StorageOpt
1195+
}
1196+
1197+
res := quota.Quota{}
1198+
1199+
if !readOnly {
1200+
res.Size = d.options.quota.Size
1201+
res.Inodes = d.options.quota.Inodes
1202+
}
1203+
12301204
// Read size to set the disk project quota per container
12311205
for key, val := range storageOpt {
12321206
key := strings.ToLower(key)
12331207
switch key {
12341208
case "size":
1209+
if readOnly {
1210+
return quota.Quota{}, fmt.Errorf("--storage-opt size is only supported for ReadWrite Layers")
1211+
}
12351212
size, err := units.RAMInBytes(val)
12361213
if err != nil {
1237-
return err
1214+
return quota.Quota{}, err
12381215
}
1239-
driver.options.quota.Size = uint64(size)
1216+
res.Size = uint64(size)
12401217
case "inodes":
1218+
if readOnly {
1219+
return quota.Quota{}, fmt.Errorf("--storage-opt inodes is only supported for ReadWrite Layers")
1220+
}
12411221
inodes, err := strconv.ParseUint(val, 10, 64)
12421222
if err != nil {
1243-
return err
1223+
return quota.Quota{}, err
12441224
}
1245-
driver.options.quota.Inodes = inodes
1225+
res.Inodes = inodes
12461226
default:
1247-
return fmt.Errorf("unknown option %s", key)
1227+
return quota.Quota{}, fmt.Errorf("unknown option %s", key)
12481228
}
12491229
}
12501230

1251-
return nil
1231+
return res, nil
12521232
}
12531233

12541234
func (d *Driver) getLower(parent string) (string, error) {

0 commit comments

Comments
 (0)