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

Illumos #1948 (expandsize property) and #2703 (zfs send progress) #908

Closed
wants to merge 4 commits into from
Closed

Illumos #1948 (expandsize property) and #2703 (zfs send progress) #908

wants to merge 4 commits into from

Conversation

mmatuska
Copy link
Contributor

#2703 seems to work very well (zfs send progress reporting)

I need information about #1948 (zpool_reopen) if this can be utilized in Linux in vdev_disk.c (TODO part)
And also the expandsize is currently reporting 16E on my system.

Here is the illumos implementation:
https://hg.openindiana.org/upstream/illumos/illumos-gate/diff/3411fd5f1589/usr/src/uts/common/fs/zfs/vdev_disk.c
illumos/illumos-gate@4263d13#usr/src/uts/common/fs/zfs/vdev_disk.c

Chris Siden and others added 3 commits August 28, 2012 21:44
Reviewed by: Adam Leventhal <ahl@delphix.com>
Reviewed by: Matt Ahrens <matt@delphix.com>
Reviewed by: Eric Schrock <eric.schrock@delphix.com>
Reviewed by: Richard Lowe <richlowe@richlowe.net>
Reviewed by: Albert Lee <trisk@nexenta.com>
Reviewed by: Dan McDonald <danmcd@nexenta.com>
Reviewed by: Garrett D'Amore <garrett@damore.org>
Approved by: Eric Schrock <eric.schrock@delphix.com>

References:
  https://www.illumos.org/issues/1948

Ported by:	Martin Matuska <martin@matuska.org>
Reviewed by: Matt Ahrens <matt@delphix.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Richard Lowe <richlowe@richlowe.net>
Approved by: Eric Schrock <Eric.Schrock@delphix.com>

Ported by: Martin Matuska <martin@matuska.org>
@behlendorf
Copy link
Contributor

@mmatuska Sorry, I've been busy. Anyway, with regards to max_psize it should be entirely safe to simply do *max_psize = *psize. However, your going to need to do it near the end of the function after *psize = bdev_capacity(bdev); or it's going to be undefined. That could pretty easily explain your 16E values.

With significantly more effort we could have it report the possible expansion size. Right now it simply returns the size of the partition, and when we do expansion we re-read the partition table and use the updated size. We could probably find a way to calculate this available space without actually re-reading the partition table.

However, I don't think we should worry about it for now.

@mmatuska
Copy link
Contributor Author

I try to stay to be close to illumos as much as possible - this means the less intervention the better.

Btw. the zfs_send progress was relatively easy to port 181f94e
The only "major" change was that in Linux I had to compare the group_leader processes instead of the individual threads in zfs_ioctl.c. In FreeBSD our SPL is done quite well so I have to change almost nothing . Today I have imported the newest empty_bpobj zpool feature to FreeBSD -HEAD.

@behlendorf
Copy link
Contributor

I try to stay to be close to illumos as much as possible - this means the less intervention the better.

Yes, I agree. The comment was more for the Illumos guys, I need to remember to bring it up when I see them next. If they are going to act as ZFS upstream they should keep this sort of thing in mind.

The only "major" change was that in Linux...

I haven't looked carefully but was this because there was a missing interface in the SPL. If so, probably the right thing to do is add it so we can stay as close to the Illumos code as possible.

@behlendorf
Copy link
Contributor

Squashed in to the following two commits and merged

37abac6 Illumos #2703: add mechanism to report ZFS send progress
1bd201e Illumos #1948: zpool list should show more detailed pool info

@behlendorf behlendorf closed this Sep 19, 2012
@mmatuska
Copy link
Contributor Author

The code is now ready to import the feature flags commits with minimal conflicts

@FransUrbo FransUrbo mentioned this pull request May 19, 2013
ryao pushed a commit to ryao/zfs that referenced this pull request Oct 6, 2013
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Christopher Siden <christopher.siden@delphix.com>
Ported-by: Richard Yao <ryao@gentoo.org>

Porting notes:

zfs_unmount_snap() significantly differs from Illumos, so the changes to this
function had to be done by hand. In addition, the single line addition in
openzfs#908 (openzfs/zfs@4ca9a43)
added a fairly big merge conflict. It was moved into zfs_unmount_snap(), which
is a more natural place for it.
ryao pushed a commit to ryao/zfs that referenced this pull request Oct 7, 2013
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Christopher Siden <christopher.siden@delphix.com>
Ported-by: Richard Yao <ryao@gentoo.org>

Porting notes:

zfs_unmount_snap() significantly differs from Illumos, so the changes to this
function had to be done by hand. In addition, the single line addition in
openzfs#908 (openzfs/zfs@4ca9a43)
added a fairly big merge conflict. It was moved into zfs_unmount_snap(), which
is a more natural place for it.
ryao pushed a commit to ryao/zfs that referenced this pull request Oct 7, 2013
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Christopher Siden <christopher.siden@delphix.com>
Ported-by: Richard Yao <ryao@gentoo.org>

Porting notes:

zfs_unmount_snap() significantly differs from Illumos, so the changes to this
function had to be done by hand. In addition, the single line addition in
openzfs#908 (openzfs/zfs@4ca9a43)
added a fairly big merge conflict. It was moved into zfs_unmount_snap(), which
is a more natural place for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants