Merge branch 'problame/fix-spaces-in-dataset-names-131'

daemon/logging/build_logging.go

@@ -16,6 +16,7 @@ import (
 	"github.com/zrepl/zrepl/endpoint"
 	"github.com/zrepl/zrepl/logger"
 	"github.com/zrepl/zrepl/replication/driver"
+	"github.com/zrepl/zrepl/replication/logic"
 	"github.com/zrepl/zrepl/rpc"
 	"github.com/zrepl/zrepl/rpc/transportmux"
 	"github.com/zrepl/zrepl/tlsconf"
@@ -79,6 +80,7 @@ const (
 )
 
 func WithSubsystemLoggers(ctx context.Context, log logger.Logger) context.Context {
+	ctx = logic.WithLogger(ctx, log.WithField(SubsysField, SubsysReplication))
 	ctx = driver.WithLogger(ctx, log.WithField(SubsysField, SubsysReplication))
 	ctx = endpoint.WithLogger(ctx, log.WithField(SubsysField, SubsyEndpoint))
 	ctx = pruner.WithLogger(ctx, log.WithField(SubsysField, SubsysPruning))

zfs/replication_history.go

@@ -24,35 +24,36 @@ func ZFSGetReplicationCursor(fs *DatasetPath) (*FilesystemVersion, error) {
 
 func ZFSSetReplicationCursor(fs *DatasetPath, snapname string) (guid uint64, err error) {
 	snapPath := fmt.Sprintf("%s@%s", fs.ToString(), snapname)
+	debug("replication cursor: snap path %q", snapPath)
 	propsSnap, err := zfsGet(snapPath, []string{"createtxg", "guid"}, sourceAny)
 	if err != nil {
-		return 0, err
+		return 0, errors.Wrap(err, "zfs: replication cursor: get snapshot createtxg")
 	}
 	snapGuid, err := strconv.ParseUint(propsSnap.Get("guid"), 10, 64)
 	bookmarkPath := fmt.Sprintf("%s#%s", fs.ToString(), ReplicationCursorBookmarkName)
 	propsBookmark, err := zfsGet(bookmarkPath, []string{"createtxg"}, sourceAny)
 	_, bookmarkNotExistErr := err.(*DatasetDoesNotExist)
 	if err != nil && !bookmarkNotExistErr {
-		return 0, err
+		return 0, errors.Wrap(err, "zfs: replication cursor: get bookmark txg")
 	}
 	if err == nil {
 		bookmarkTxg, err := strconv.ParseUint(propsBookmark.Get("createtxg"), 10, 64)
 		if err != nil {
-			return 0, errors.Wrap(err, "cannot parse bookmark createtxg")
+			return 0, errors.Wrap(err, "zfs: replication cursor: parse bookmark createtxg")
 		}
 		snapTxg, err := strconv.ParseUint(propsSnap.Get("createtxg"), 10, 64)
 		if err != nil {
-			return 0, errors.Wrap(err, "cannot parse snapshot createtxg")
+			return 0, errors.Wrap(err, "zfs: replication cursor: parse snapshot createtxg")
 		}
 		if snapTxg < bookmarkTxg {
-			return 0, errors.New("replication cursor can only be advanced, not set back")
+			return 0, errors.New("zfs: replication cursor: can only be advanced, not set back")
 		}
 		if err := ZFSDestroy(bookmarkPath); err != nil { // FIXME make safer by using new temporary bookmark, then rename, possible with channel programs
-			return 0, err
+			return 0, errors.Wrap(err, "zfs: replication cursor: destroy current cursor")
 		}
 	}
 	if err := ZFSBookmark(fs, snapname, ReplicationCursorBookmarkName); err != nil {
-		return 0, err
+		return 0, errors.Wrapf(err, "zfs: replication cursor: create bookmark")
 	}
 	return snapGuid, nil
 }

zfs/zfs.go

@@ -577,10 +577,17 @@ type DrySendInfo struct {
 	SizeEstimate int64 // -1 if size estimate is not possible
 }
 
-var sendDryRunInfoLineRegex = regexp.MustCompile(`^(full|incremental)(\t(\S+))?\t(\S+)\t([0-9]+)$`)
+var (
+	// keep same number of capture groups for unmarshalInfoLine homogenity
+
+	sendDryRunInfoLineRegexFull = regexp.MustCompile(`^(full)\t()([^\t]+@[^\t]+)\t([0-9]+)$`)
+	// cannot enforce '[#@]' in incremental source, see test cases  
+	sendDryRunInfoLineRegexIncremental = regexp.MustCompile(`^(incremental)\t([^\t]+)\t([^\t]+@[^\t]+)\t([0-9]+)$`)
+)
 
 // see test cases for example output
 func (s *DrySendInfo) unmarshalZFSOutput(output []byte) (err error) {
+	debug("DrySendInfo.unmarshalZFSOutput: output=%q", output)
 	lines := strings.Split(string(output), "\n")
 	for _, l := range lines {
 		regexMatched, err := s.unmarshalInfoLine(l)
@@ -592,7 +599,7 @@ func (s *DrySendInfo) unmarshalZFSOutput(output []byte) (err error) {
 		}
 		return nil
 	}
-	return fmt.Errorf("no match for info line (regex %s)", sendDryRunInfoLineRegex)
+	return fmt.Errorf("no match for info line (regex1 %s) (regex2 %s)", sendDryRunInfoLineRegexFull, sendDryRunInfoLineRegexIncremental)
 }
 
 
@@ -602,24 +609,32 @@ func (s *DrySendInfo) unmarshalZFSOutput(output []byte) (err error) {
 // => see test cases
 func (s *DrySendInfo) unmarshalInfoLine(l string) (regexMatched bool, err error) {
 
-	m := sendDryRunInfoLineRegex.FindStringSubmatch(l)
-	if m == nil {
+	mFull := sendDryRunInfoLineRegexFull.FindStringSubmatch(l)
+	mInc := sendDryRunInfoLineRegexIncremental.FindStringSubmatch(l)
+	var m []string
+	if mFull == nil && mInc == nil {
 		return false, nil
+	} else if mFull != nil && mInc != nil {
+		panic(fmt.Sprintf("ambiguous ZFS dry send output: %q", l))
+	} else if mFull != nil {
+		m = mFull
+	} else if mInc != nil {
+		m = mInc
 	}
 	s.Type, err = DrySendTypeFromString(m[1])
 	if err != nil {
 		return true, err
 	}
 
-	s.From = m[3]
-	s.To = m[4]
+	s.From = m[2]
+	s.To = m[3]
 	toFS, _, _, err := DecomposeVersionString(s.To)
 	if err != nil {
 		return true, fmt.Errorf("'to' is not a valid filesystem version: %s", err)
 	}
 	s.Filesystem = toFS
 
-	s.SizeEstimate, err = strconv.ParseInt(m[5], 10, 64)
+	s.SizeEstimate, err = strconv.ParseInt(m[4], 10, 64)
 	if err != nil {
 		return true, fmt.Errorf("cannot not parse size: %s", err)
 	}
@@ -909,7 +924,7 @@ func ZFSGet(fs *DatasetPath, props []string) (*ZFSProperties, error) {
 	return zfsGet(fs.ToString(), props, sourceAny)
 }
 
-var zfsGetDatasetDoesNotExistRegexp = regexp.MustCompile(`^cannot open '(\S+)': (dataset does not exist|no such pool or dataset)`)
+var zfsGetDatasetDoesNotExistRegexp = regexp.MustCompile(`^cannot open '([^)]+)': (dataset does not exist|no such pool or dataset)`)
 
 type DatasetDoesNotExist struct {
 	Path string
@@ -1065,6 +1080,8 @@ func ZFSBookmark(fs *DatasetPath, snapshot, bookmark string) (err error) {
 	snapname := zfsBuildSnapName(fs, snapshot)
 	bookmarkname := zfsBuildBookmarkName(fs, bookmark)
 
+	debug("bookmark: %q %q", snapname, bookmarkname)
+
 	cmd := exec.Command(ZFS_BINARY, "bookmark", snapname, bookmarkname)
 
 	stderr := bytes.NewBuffer(make([]byte, 0, 1024))

zfs/zfs_test.go

@@ -70,6 +70,10 @@ func TestZFSPropertySource(t *testing.T) {
 
 }
 
+func TestDrySendRegexesHaveSameCaptureGroupCount(t *testing.T) {
+	assert.Equal(t, sendDryRunInfoLineRegexFull.NumSubexp(), sendDryRunInfoLineRegexIncremental.NumSubexp())
+}
+
 func TestDrySendInfo(t *testing.T) {
 
 	// # full send
@@ -127,6 +131,11 @@ full	zroot/test/a@3	10518512
 size	10518512
 `
 
+	fullWithSpaces := "\nfull\tpool1/otherjob/ds with spaces@blaffoo\t12912\nsize\t12912\n"
+	fullWithSpacesInIntermediateComponent := "\nfull\tpool1/otherjob/another ds with spaces/childfs@blaffoo\t12912\nsize\t12912\n"
+	incrementalWithSpaces := "\nincremental\tblaffoo\tpool1/otherjob/another ds with spaces@blaffoo2\t624\nsize\t624\n"
+	incrementalWithSpacesInIntermediateComponent := "\nincremental\tblaffoo\tpool1/otherjob/another ds with spaces/childfs@blaffoo2\t624\nsize\t624\n"
+
 	type tc struct {
 		name string
 		in string
@@ -187,6 +196,46 @@ size	10518512
 				SizeEstimate: 10518512,
 			},
 		},
+		{
+			name: "fullWithSpaces", in: fullWithSpaces,
+			exp: &DrySendInfo{
+				Type:         DrySendTypeFull,
+				Filesystem:   "pool1/otherjob/ds with spaces",
+				From:         "",
+				To:           "pool1/otherjob/ds with spaces@blaffoo",
+				SizeEstimate: 12912,
+			},
+		},
+		{
+			name: "fullWithSpacesInIntermediateComponent", in: fullWithSpacesInIntermediateComponent,
+			exp: &DrySendInfo{
+				Type:         DrySendTypeFull,
+				Filesystem:   "pool1/otherjob/another ds with spaces/childfs",
+				From:         "",
+				To:           "pool1/otherjob/another ds with spaces/childfs@blaffoo",
+				SizeEstimate: 12912,
+			},
+		},
+		{
+			name: "incrementalWithSpaces", in: incrementalWithSpaces,
+			exp: &DrySendInfo{
+				Type:         DrySendTypeIncremental,
+				Filesystem:   "pool1/otherjob/another ds with spaces",
+				From:         "blaffoo",
+				To:           "pool1/otherjob/another ds with spaces@blaffoo2",
+				SizeEstimate: 624,
+			},
+		},
+		{
+			name: "incrementalWithSpacesInIntermediateComponent", in: incrementalWithSpacesInIntermediateComponent,
+			exp: &DrySendInfo{
+				Type:         DrySendTypeIncremental,
+				Filesystem:   "pool1/otherjob/another ds with spaces/childfs",
+				From:         "blaffoo",
+				To:           "pool1/otherjob/another ds with spaces/childfs@blaffoo2",
+				SizeEstimate: 624,
+			},
+		},
 	}
 
 	for _, tc := range tcs {