From 7d9a1b7eaee2e4ca0a5214283c8e601df2f54d95 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 21 Mar 2019 16:08:05 +0100 Subject: [PATCH 1/3] zfs: dry send: handle spaces in dataset names correctly fixes #131 --- zfs/zfs.go | 31 ++++++++++++++++++++++-------- zfs/zfs_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/zfs/zfs.go b/zfs/zfs.go index 652a494..d693f2c 100644 --- a/zfs/zfs.go +++ b/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] - toFS, _, _ , err := DecomposeVersionString(s.To) + 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) } diff --git a/zfs/zfs_test.go b/zfs/zfs_test.go index 9126bb0..217d765 100644 --- a/zfs/zfs_test.go +++ b/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 @@ -165,7 +174,7 @@ size 10518512 SizeEstimate: 5383312, }, }, - { + { name: "incNoToken", in: incNoToken, exp: &DrySendInfo{ Type: DrySendTypeIncremental, @@ -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 { From ab38f24198d7ec9190adb4cdafb77d4e568bfc6a Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 21 Mar 2019 16:59:08 +0100 Subject: [PATCH 2/3] zfs: bookmark / replication cursor: handle spaces in ds names correctly fixes #131 --- zfs/replication_history.go | 15 ++++++++------- zfs/zfs.go | 4 +++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/zfs/replication_history.go b/zfs/replication_history.go index 2ad5abf..d420047 100644 --- a/zfs/replication_history.go +++ b/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 } diff --git a/zfs/zfs.go b/zfs/zfs.go index d693f2c..40b1434 100644 --- a/zfs/zfs.go +++ b/zfs/zfs.go @@ -924,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 @@ -1080,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)) From c0028c1c44beacaf0b012d2274ec8ad16271bbf0 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Thu, 21 Mar 2019 17:00:13 +0100 Subject: [PATCH 3/3] daemon/logging: add replication logic logger --- daemon/logging/build_logging.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/daemon/logging/build_logging.go b/daemon/logging/build_logging.go index 52b7e15..0b39f37 100644 --- a/daemon/logging/build_logging.go +++ b/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))