Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAS-123123 / 22.12.4 / Fix raw receive with different indirect block size. #145

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

ixhamza
Copy link

@ixhamza ixhamza commented Jul 20, 2023

Ticket: https://ixsystems.atlassian.net/browse/NAS-122583

Unlike regular receive, raw receive require destination to have the same block structure as the source. In case of dnode reclaim this triggers two special cases, requiring special handling:

  • If dn_nlevels == 1, we can change the ibs, but dnode_set_blksz() should not dirty the data buffer if block size does not change, or durign receive dbuf_dirty_lightweight() will trigger assertion.
  • If dn_nlevels > 1, we just can't change the ibs, dnode_set_blksz() would fail and receive_object would trigger assertion, so we should destroy and recreate the dnode from scratch.

Reviewed-by: Paul Dagnelie pcd@delphix.com
Signed-off-by: Alexander Motin mav@FreeBSD.org
Sponsored by: iXsystems, Inc.
Closes openzfs#15039

Motivation and Context

Description

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

Unlike regular receive, raw receive require destination to have the
same block structure as the source.  In case of dnode reclaim this
triggers two special cases, requiring special handling:
 - If dn_nlevels == 1, we can change the ibs, but dnode_set_blksz()
should not dirty the data buffer if block size does not change, or
durign receive dbuf_dirty_lightweight() will trigger assertion.
 - If dn_nlevels > 1, we just can't change the ibs, dnode_set_blksz()
would fail and receive_object would trigger assertion, so we should
destroy and recreate the dnode from scratch.

Reviewed-by: Paul Dagnelie <pcd@delphix.com>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes openzfs#15039
@ixhamza ixhamza requested a review from amotin July 20, 2023 16:47
@bugclerk bugclerk changed the title Fix raw receive with different indirect block size. NAS-123123 / 22.12.4 / Fix raw receive with different indirect block size. Jul 20, 2023
@bugclerk
Copy link

@ixhamza
Copy link
Author

ixhamza commented Jul 20, 2023

@amotin I am not sure if we should really add 7468f5d if we just want to cherry-pick it to release/22.12.3 branch.

@amotin
Copy link
Collaborator

amotin commented Jul 20, 2023

@ixhamza It seems purely cosmetic, but I don't see why not merge. I just wonder what this version would mean for different releases (22.12.3.3 vs 22.12.4).

@ixhamza
Copy link
Author

ixhamza commented Jul 20, 2023

@amotin zfs --version result would differ. If you want, I can merge it.

@amotin
Copy link
Collaborator

amotin commented Jul 20, 2023

@ixhamza I'd like versions to be consistent between all branches. I am OK either way. If 2.1.12-1 in other branch includes more than this change, then here would have to be something like If 2.1.12-0.1, or whatever sub-versioning is possible there. Or we may give up on this, relying only on TrueNAS versioning.

@ixhamza
Copy link
Author

ixhamza commented Jul 20, 2023

Thanks, @amotin. I agree with you. To avoid more confusion, I think it's better to revert 7468f5d on truenas/zfs-2.1-release and rely on Truenas versioning if you agree.

@amotin
Copy link
Collaborator

amotin commented Jul 20, 2023

better to revert 7468f5d on truenas/zfs-2.1-release and rely on Truenas versioning

Yea. That version description, but including other changes there there is confusing.

@amotin amotin merged commit 4b88096 into stable/bluefin Jul 20, 2023
16 of 18 checks passed
@amotin amotin deleted the NAS-123123-bluefin branch July 20, 2023 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants