Project

General

Profile

Actions

Bug #53897

closed

diff-iterate can report holes when diffing against the beginning of time (fromsnapname == NULL)

Added by Ilya Dryomov over 2 years ago. Updated 2 months ago.

Status:
Resolved
Priority:
High
Assignee:
Target version:
-
% Done:

0%

Source:
Tags:
backport_processed
Backport:
pacific,quincy,reef
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

rbd_diff_iterate/rbd_diff_iterate2() API documentation says:

If the source snapshot name is NULL, we interpret that as the beginning of time and return all allocated regions of the image.

Based on this sentence, the user can rightfully assume that the callback would only be invoked for allocated areas (i.e. with exists=true). However, if there is a snapshot which contains a discarded object, diff-iterate reports holes (exists=false) even when diffing against the beginning of time:

$ rbd create --size 16M foo
$ sudo rbd map foo
/dev/rbd0
$ sudo xfs_io -d -c 'pwrite -b 4M 0 16M' /dev/rbd0
wrote 16777216/16777216 bytes at offset 0
16 MiB, 4 ops; 0.4607 sec (34.726 MiB/sec and 8.6814 ops/sec)
$ rbd snap create foo@snap
Creating snap: 100% complete...done.
$ rbd diff foo
Offset    Length   Type
0         4194304  data
4194304   4194304  data
8388608   4194304  data
12582912  4194304  data

$ blkdiscard -o 4M -l 8M /dev/rbd0
$ rbd diff foo
Offset    Length   Type
0         4194304  data
4194304   4194304  zero
8388608   4194304  zero
12582912  4194304  data

Removing such a snapshot makes these "holes" disappear from the report:

$ rbd snap rm foo@snap
Removing snap: 100% complete...done.
$ rbd diff foo
Offset    Length   Type
0         4194304  data
12582912  4194304  data

If nothing depends on this quirk, it should be fixed. Otherwise, the documentation needs to be updated to mention this.


Related issues 7 (2 open5 closed)

Related to rbd - Bug #63719: [test] scribble()-based DiffIterate tests are too weakNew

Actions
Related to rbd - Bug #63770: [diff-iterate] discards that truncate aren't accounted for by ObjectListSnapsRequestPending BackportIlya Dryomov

Actions
Blocks rbd - Feature #63341: improve rbd_diff_iterate2() performance in fast-diff modeResolvedIlya Dryomov

Actions
Blocked by rbd - Bug #63654: [diff-iterate] ObjectListSnapsRequest's LIST_SNAPS_FLAG_WHOLE_OBJECT behavior is brokenResolvedIlya Dryomov

Actions
Copied to rbd - Backport #63846: pacific: diff-iterate can report holes when diffing against the beginning of time (fromsnapname == NULL)ResolvedIlya DryomovActions
Copied to rbd - Backport #63847: quincy: diff-iterate can report holes when diffing against the beginning of time (fromsnapname == NULL)ResolvedIlya DryomovActions
Copied to rbd - Backport #63848: reef: diff-iterate can report holes when diffing against the beginning of time (fromsnapname == NULL)ResolvedIlya DryomovActions
Actions #1

Updated by Christopher Hoffman about 2 years ago

  • Status changed from New to In Progress
  • Assignee set to Christopher Hoffman
Actions #2

Updated by Christopher Hoffman over 1 year ago

I don't see anything within ceph code that explicitly relies on showing a zeroed entry. I would lean towards keeping this functionality as some user script may rely on this. If so, updating the documentation would be best.

Actions #3

Updated by Ilya Dryomov 6 months ago

  • Blocks Feature #63341: improve rbd_diff_iterate2() performance in fast-diff mode added
Actions #4

Updated by Ilya Dryomov 6 months ago

  • Assignee changed from Christopher Hoffman to Ilya Dryomov
  • Priority changed from Normal to High

After a deep investigation prompted by https://tracker.ceph.com/issues/63341, I'm convinced that this is an implementation bug. The API documentation is correct (i.e. it's how the code was intended to behave).

Keeping the current behavior and instead adjusting documentation would also preclude some of the performance optimizations for fast-diff mode.

Actions #5

Updated by Ilya Dryomov 5 months ago

Christopher Hoffman wrote:

I don't see anything within ceph code that explicitly relies on showing a zeroed entry.

It turns out that deep-copy started relying on this in https://github.com/ceph/ceph/commit/e5a21e904142f8d728ba5de1eecf940b6b8329b5.

Actions #6

Updated by Ilya Dryomov 5 months ago

  • Pull request ID set to 54547
Actions #7

Updated by Ilya Dryomov 5 months ago

  • Blocked by Bug #63654: [diff-iterate] ObjectListSnapsRequest's LIST_SNAPS_FLAG_WHOLE_OBJECT behavior is broken added
Actions #8

Updated by Ilya Dryomov 5 months ago

  • Related to Bug #63719: [test] scribble()-based DiffIterate tests are too weak added
Actions #9

Updated by Ilya Dryomov 5 months ago

  • Related to Bug #63770: [diff-iterate] discards that truncate aren't accounted for by ObjectListSnapsRequest added
Actions #10

Updated by Ilya Dryomov 4 months ago

  • Status changed from In Progress to Pending Backport
  • Backport set to pacific,quincy,reef
Actions #11

Updated by Backport Bot 4 months ago

  • Copied to Backport #63846: pacific: diff-iterate can report holes when diffing against the beginning of time (fromsnapname == NULL) added
Actions #12

Updated by Backport Bot 4 months ago

  • Copied to Backport #63847: quincy: diff-iterate can report holes when diffing against the beginning of time (fromsnapname == NULL) added
Actions #13

Updated by Backport Bot 4 months ago

  • Copied to Backport #63848: reef: diff-iterate can report holes when diffing against the beginning of time (fromsnapname == NULL) added
Actions #14

Updated by Backport Bot 4 months ago

  • Tags set to backport_processed
Actions #15

Updated by Backport Bot 2 months ago

  • Status changed from Pending Backport to Resolved

While running with --resolve-parent, the script "backport-create-issue" noticed that all backports of this issue are in status "Resolved" or "Rejected".

Actions

Also available in: Atom PDF