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

zfs destroy on a zvol snapshot doesn't remove /dev/zd* device #903

Closed
mgmartin opened this issue Aug 28, 2012 · 10 comments
Closed

zfs destroy on a zvol snapshot doesn't remove /dev/zd* device #903

mgmartin opened this issue Aug 28, 2012 · 10 comments
Milestone

Comments

@mgmartin
Copy link

Creating a snapshot on a zvol followed by a destroy does not remove the /dev/zd* device file. Over time, the /dev directory fills up with invalid device nodes.

I've confirmed the problem appears within the zfs commit : 330d06f Illumos #1644, #1645, #1646, #1647, #1708 . Reverting this commit from a checked out zfs-0.6.0-rc10 tag fixes the issue.

Steps to reproduce:

  1. Checkout code zfs-0.6.0-rc10 or later
  2. zpool create test /dev/loop0
  3. zfs create test/v1 -V 50M
  4. zfs snapshot test/v1@snap1
  5. confirm two /zd* device nodes in /dev
  6. zfs destroy test/v1@snap1
  7. (bug) device node count stays at 2.

The /dev/zd* node is removed if the pool is destroyed.

@behlendorf
Copy link
Contributor

Thanks for doing the leg work, we'll get that fixed for -rc11

@mgmartin
Copy link
Author

Didn't mean to close this issue. I thought ""Close" meant close the window.

@mgmartin mgmartin reopened this Aug 31, 2012
behlendorf added a commit to behlendorf/zfs that referenced this issue Sep 7, 2012
The 'zfs destroy' changes in 330d06f disrupted how zvol devices
get removed on ZoL.  However, it basically boils down to the
fact that we are no longer reliably calling zvol_remove_minor()
via zfs_ioc_destroy_snaps().

Therefore we add the missing call and handle things similarly
to the existing zfs_unmount_snap() case.  Ideally we would check
if this is of type DMU_OST_ZFS or DMU_OST_ZVOL and just do the
right thing as in zfs_ioc_destroy().  However, it looks like
it would be fairly expensive to get the type, and it's harmless
to simply attempt the umount and minor removal.

It's not exactly clear to me why this wasn't an issue for the
upstream Illumos code.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#903
@behlendorf
Copy link
Contributor

@mmatuska Can you please take a look at pull request #946 , it's not clear to me why Illumos and FreeBSD don't need something similar to this.

@mgmartin The above pull request fixes the issue, can you please verify it resolves the problem on your system.

@mgmartin
Copy link
Author

mgmartin commented Sep 8, 2012

I confirmed pull request #946 applied to zfs-0.6.0-rc10 fixes the issue. The /dev/zd* nodes are removed on zvol snapshot destroy.

@mmatuska
Copy link
Contributor

mmatuska commented Sep 8, 2012

I can confirm this issue on FreeBSD and on OpenIndiana (illumos).
What copyright string should be added to the header to comply with CDDL? You in person or LLNL?

Illumos issue filed:
https://www.illumos.org/issues/3170

@behlendorf
Copy link
Contributor

@mmatuska I'm of the opinion that the attribution in the git/svn/hg commit is enough, but if you need to add it then it's best to go with me in person.

Portions Copyright 2012 Brian Behlendorf behlendorf1@llnl.gov

And thanks for checking this out for me on FreeBSD and OpenIndiana, and for opening the Illumos issue. I was fairly sure they were going to have the same issue since I couldn't identify any other machinery which would do the job.

behlendorf added a commit that referenced this issue Sep 10, 2012
The 'zfs destroy' changes in 330d06f disrupted how zvol devices
get removed on ZoL.  However, it basically boils down to the
fact that we are no longer reliably calling zvol_remove_minor()
via zfs_ioc_destroy_snaps().

Therefore we add the missing call and handle things similarly
to the existing zfs_unmount_snap() case.  Ideally we would check
if this is of type DMU_OST_ZFS or DMU_OST_ZVOL and just do the
right thing as in zfs_ioc_destroy().  However, it looks like
it would be fairly expensive to get the type, and it's harmless
to simply attempt the umount and minor removal.

This is also an issue in the latest FreeBSD and Illumos code.
It was being tracked under the following issue, and we may want
to refresh our code when they settle on what they want to do
about it upstream.

  https://www.illumos.org/issues/3170

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue #903
@ryao
Copy link
Contributor

ryao commented Oct 4, 2013

I am looking into porting 3744 zfs shouldn't ignore errors unmounting snapshots from Illumos. In hindsight, it might have been better to place (void) zvol_remove_minor(name); inside zfs_unmount_snap() rather than zfs_ioc_destroy_snaps(). It seems more to be a more natural place for it and it would have reduced the merge conflicts.

@behlendorf
Copy link
Contributor

@ryao If you can bring us more in sync with Illumos while porting 3744 I'm all for it.

@ryao
Copy link
Contributor

ryao commented Oct 11, 2013

Subsequent testing revealed that the current location is fine. #1775 retained the current location.

@dbourget
Copy link

I'm seeing something that looks like a regression of this issue with 0.6.3-2~trusty. I have a system that has in the order of 100 snapshots, but over 24000 /dev/zd* devices. This machine does a lot of snapshot creation/deletion on zvols.

Incidentally, I noticed this while investigating poor IO performance on this system. I wonder if the clutter of zvol devices could be the reason for the poor performance I'm seeing.

Update: it looks like this happens only if the property snapdev is set to "visible" on the pool. I have other machines that perform similar work, but they don't have a proliferation of zd devices. Only the one with the problem had snapdev=visible (I've turned it off now and I'll report whether that stopped the proliferation).

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

No branches or pull requests

5 participants