fix: do not allow total all kind of snapshots to be bigger than 250 (backport #1405)

app/cmd/controller.go

@@ -85,7 +85,7 @@ func ControllerCmd() cli.Command {
 			},
 			cli.IntFlag{
 				Name:  "snapshot-max-count",
-				Value: 250,
+				Value: types.MaximumTotalSnapshotCount,
 				Usage: "Maximum number of snapshots to keep",
 			},
 			cli.StringFlag{

app/cmd/replica.go

@@ -75,7 +75,7 @@ func ReplicaCmd() cli.Command {
 			},
 			cli.IntFlag{
 				Name:  "snapshot-max-count",
-				Value: 250,
+				Value: types.MaximumTotalSnapshotCount,
 				Usage: "Maximum number of snapshots to keep",
 			},
 			cli.StringFlag{

go.mod

@@ -4,6 +4,8 @@ go 1.22.7
 
 toolchain go1.23.6
 
+replace github.com/longhorn/types => ../types
+
 require (
 	github.com/docker/go-units v0.4.0
 	github.com/gofrs/flock v0.8.1

go.sum

@@ -81,12 +81,20 @@ github.com/longhorn/go-iscsi-helper v0.0.0-20250111093313-7e1930499625 h1:d39A30
 github.com/longhorn/go-iscsi-helper v0.0.0-20250111093313-7e1930499625/go.mod h1:yIm3sGRuYOw/Y3XzRhG5+3FlZBOfrU5EZOavzwL2jVs=
 github.com/longhorn/sparse-tools v0.0.0-20241216160947-2b328f0fa59c h1:OFz3haCSPdgiiJvXLBeId/4dPu0dxIEqkQkfNMufLwc=
 github.com/longhorn/sparse-tools v0.0.0-20241216160947-2b328f0fa59c/go.mod h1:dfbJqfI8+T9ZCp5zhTYcBi/64hPBNt5/vFF3gTlfMmc=
+<<<<<<< HEAD
 github.com/longhorn/types v0.0.0-20241225162202-00d3a5fd7502 h1:jgw7nosooLe1NQEdCGzM/nEOFzPcurNO+0PDsicc5+A=
 github.com/longhorn/types v0.0.0-20241225162202-00d3a5fd7502/go.mod h1:3jHuVDtpkXQzpnp4prguDBskVRric2kmF8aSPkRJ4jw=
 github.com/mattn/go-colorable v0.1.4 h1:snbPLB8fVfU9iwbbo30TPtbLRzwWu6aJS6Xh4eaaviA=
 github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
 github.com/mattn/go-isatty v0.0.8 h1:HLtExJ+uU2HOZ+wI0Tt5DtUDrx8yhUqDcp7fYERX4CE=
 github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
+=======
+github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
+github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=
+github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
+github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
+github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
+>>>>>>> 8bf0b1d (fix: do not allow total all kind of snapshots to be bigger than 250)
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo=
 github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=

pkg/backend/file/file.go

@@ -100,8 +100,8 @@ func (f *Wrapper) SectorSize() (int64, error) {
 	return 4096, nil
 }
 
-func (f *Wrapper) GetSnapshotCountAndSizeUsage() (int, int64, error) {
-	return 1, 0, nil
+func (f *Wrapper) GetSnapshotCountAndSizeUsage() (int, int, int64, error) {
+	return 1, 1, 0, nil
 }
 
 func (f *Wrapper) GetRevisionCounter() (int64, error) {

pkg/backend/remote/remote.go

@@ -218,16 +218,16 @@ func (r *Remote) SectorSize() (int64, error) {
 	return replicaInfo.SectorSize, nil
 }
 
-func (r *Remote) GetSnapshotCountAndSizeUsage() (int, int64, error) {
+func (r *Remote) GetSnapshotCountAndSizeUsage() (int, int, int64, error) {
 	replicaInfo, err := r.info()
 	if err != nil {
-		return 0, 0, err
+		return 0, 0, 0, err
 	}
 	switch replicaInfo.State {
 	case "open", "dirty", "rebuilding":
-		return replicaInfo.SnapshotCountUsage, replicaInfo.SnapshotSizeUsage, nil
+		return replicaInfo.SnapshotCountUsage, replicaInfo.SnapshotCountTotal, replicaInfo.SnapshotSizeUsage, nil
 	}
-	return 0, 0, fmt.Errorf("invalid state %v for counting snapshots", replicaInfo.State)
+	return 0, 0, 0, fmt.Errorf("invalid state %v for counting snapshots", replicaInfo.State)
 }
 
 func (r *Remote) GetRevisionCounter() (int64, error) {

pkg/controller/control.go

@@ -218,13 +218,16 @@ func (c *Controller) Snapshot(name string, labels map[string]string) (string, er
 }
 
 func (c *Controller) canDoSnapshot() error {
-	countUsage, sizeUsage, err := c.backend.GetSnapshotCountAndSizeUsage()
+	countUsage, countTotal, sizeUsage, err := c.backend.GetSnapshotCountAndSizeUsage()
 	if err != nil {
 		return err
 	}
 	if countUsage >= c.snapshotMaxCount {
 		return fmt.Errorf("snapshot count usage %d is equal or larger than snapshotMaxCount %d", countUsage, c.snapshotMaxCount)
 	}
+	if countTotal >= types.MaximumTotalSnapshotCount {
+		return fmt.Errorf("snapshot count total is already too big: %v", countTotal)
+	}
 	// if SnapshotMaxSize is 0, it means no limit
 	if c.SnapshotMaxSize == 0 {
 		return nil
@@ -690,7 +693,7 @@ func (c *Controller) SetSnapshotMaxCount(count int) error {
 	c.Lock()
 	defer c.Unlock()
 
-	countUsage, _, err := c.backend.GetSnapshotCountAndSizeUsage()
+	countUsage, _, _, err := c.backend.GetSnapshotCountAndSizeUsage()
 	if err != nil {
 		return err
 	}
@@ -728,7 +731,7 @@ func (c *Controller) SetSnapshotMaxSize(size int64) error {
 	c.Lock()
 	defer c.Unlock()
 
-	_, sizeUsage, err := c.backend.GetSnapshotCountAndSizeUsage()
+	_, _, sizeUsage, err := c.backend.GetSnapshotCountAndSizeUsage()
 	if err != nil {
 		return err
 	}

pkg/controller/replicator.go

@@ -353,35 +353,38 @@ type backendWrapper struct {
 	mode    types.Mode
 }
 
-func (r *replicator) GetSnapshotCountAndSizeUsage() (countUsage int, sizeUsage int64, err error) {
+func (r *replicator) GetSnapshotCountAndSizeUsage() (countUsage, countTotal int, sizeUsage int64, err error) {
 	var (
-		count     int
-		size      int64
-		hasResult bool
+		currentCountUsage, currentCountTotal int
+		size                                 int64
+		hasResult                            bool
 	)
 	for _, backend := range r.backends {
 		if backend.mode == types.ERR {
 			continue
 		}
 		// ignore error and try next one
-		count, size, err = backend.backend.GetSnapshotCountAndSizeUsage()
+		currentCountUsage, currentCountTotal, size, err = backend.backend.GetSnapshotCountAndSizeUsage()
 		if err != nil {
 			logrus.Errorf("Failed to get snapshot count and size usage: %v", err)
 			continue
 		}
 		hasResult = true
-		if countUsage < count {
-			countUsage = count
+		if countUsage < currentCountUsage {
+			countUsage = currentCountUsage
+		}
+		if countTotal < currentCountTotal {
+			countTotal = currentCountTotal
 		}
 		if sizeUsage < size {
 			sizeUsage = size
 		}
 	}
 
 	if !hasResult {
-		return 0, 0, fmt.Errorf("cannot get an valid result for snapshot usage")
+		return 0, 0, 0, fmt.Errorf("cannot get an valid result for snapshot usage")
 	}
-	return countUsage, sizeUsage, nil
+	return countUsage, countTotal, sizeUsage, nil
 }
 
 func (r *replicator) GetHeadFileSize() (int64, error) {

pkg/replica/client/client.go

@@ -167,6 +167,7 @@ func GetReplicaInfo(r *ptypes.Replica) *types.ReplicaInfo {
 		RevisionCounterDisabled:   r.RevisionCounterDisabled,
 		UnmapMarkDiskChainRemoved: r.UnmapMarkDiskChainRemoved,
 		SnapshotCountUsage:        int(r.SnapshotCountUsage),
+		SnapshotCountTotal:        int(r.SnapshotCountTotal),
 		SnapshotSizeUsage:         r.SnapshotSizeUsage,
 	}
 

pkg/replica/replica.go

@@ -28,8 +28,6 @@ import (
 const (
 	volumeMetaData = "volume.meta"
 
-	maximumChainLength = 250
-
 	tmpFileSuffix = ".tmp"
 
 	// Special indexes inside r.volume.files
@@ -149,7 +147,11 @@ func New(size, sectorSize int64, dir string, backingFile *backingfile.BackingFil
 func NewReadOnly(dir, head string, backingFile *backingfile.BackingFile) (*Replica, error) {
 	// size and sectorSize don't matter because they will be read from metadata
 	// snapshotMaxCount and SnapshotMaxSize don't matter because readonly replica can't create a new disk
+<<<<<<< HEAD
 	return construct(true, 0, diskutil.ReplicaSectorSize, dir, head, backingFile, false, false, 250, 0)
+=======
+	return construct(ctx, true, 0, diskutil.ReplicaSectorSize, dir, head, backingFile, false, false, types.MaximumTotalSnapshotCount, 0)
+>>>>>>> 8bf0b1d (fix: do not allow total all kind of snapshots to be bigger than 250)
 }
 
 func construct(readonly bool, size, sectorSize int64, dir, head string, backingFile *backingfile.BackingFile, disableRevCounter, unmapMarkDiskChainRemoved bool, snapshotMaxCount int, snapshotMaxSize int64) (*Replica, error) {
@@ -1045,7 +1047,7 @@ func (r *Replica) openLiveChain() error {
 		return err
 	}
 
-	if len(chain) > maximumChainLength {
+	if len(chain) > types.MaximumTotalSnapshotCount {
 		return fmt.Errorf("live chain is too long: %v", len(chain))
 	}
 
@@ -1382,11 +1384,15 @@ func (r *Replica) ListDisks() map[string]DiskInfo {
 	return result
 }
 
-func (r *Replica) GetSnapshotCountUsage() int {
+func (r *Replica) GetSnapshotCount() (int, int) {
 	r.RLock()
 	defer r.RUnlock()
 
-	return r.getSnapshotCountUsage()
+	return r.getSnapshotCountUsage(), r.getSnapshotCountTotal()
+}
+
+func (r *Replica) getSnapshotCountTotal() int {
+	return len(r.diskData)
 }
 
 func (r *Replica) getSnapshotCountUsage() int {

pkg/replica/rpc/server.go

@@ -83,7 +83,9 @@ func (rs *ReplicaServer) getReplica() (replica *ptypes.Replica) {
 		chain, _ := r.DisplayChain()
 		replica.Chain = chain
 		replica.RemainSnapshots = int32(r.GetRemainSnapshotCounts())
-		replica.SnapshotCountUsage = int32(r.GetSnapshotCountUsage())
+		snapCountUsage, snapCountTotal := r.GetSnapshotCount()
+		replica.SnapshotCountUsage = int32(snapCountUsage)
+		replica.SnapshotCountTotal = int32(snapCountTotal)
 		replica.SnapshotSizeUsage = r.GetSnapshotSizeUsage()
 		if !r.IsRevCounterDisabled() {
 			replica.RevisionCounter = r.GetRevisionCounter()

pkg/types/resource.go

@@ -18,6 +18,7 @@ type ReplicaInfo struct {
 	RevisionCounterDisabled   bool                `json:"revisioncounterdisabled"`
 	UnmapMarkDiskChainRemoved bool                `json:"unmapMarkDiskChainRemoved"`
 	SnapshotCountUsage        int                 `json:"snapshotCountUsage"`
+	SnapshotCountTotal        int                 `json:"snapshotCountTotal"`
 	SnapshotSizeUsage         int64               `json:"snapshotSizeUsage"`
 }
 

pkg/types/types.go

@@ -44,6 +44,8 @@ const (
 	EngineFrontendISCSI    = "tgt-iscsi"
 
 	VolumeHeadName = "volume-head"
+
+	MaximumTotalSnapshotCount = 250
 )
 
 type DataServerProtocol string
@@ -103,7 +105,7 @@ type Backend interface {
 	ResetRebuild() error
 	SetSnapshotMaxCount(count int) error
 	SetSnapshotMaxSize(size int64) error
-	GetSnapshotCountAndSizeUsage() (int, int64, error)
+	GetSnapshotCountAndSizeUsage() (int, int, int64, error)
 }
 
 type BackendFactory interface {