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.

Sign up for GitHub

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

Fix intermittent EINVAL in ZFSPool.sync #194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix intermittent EINVAL in ZFSPool.sync #194

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libzfs.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2892,7 +2892,7 @@ cdef class ZFSPool(object):
IF HAVE_LZC_SYNC:
def sync(self, force=False):
cdef int ret
cdef const char *c_name = self.name
cdef const char *c_name = libzfs.zpool_get_name(self.handle)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow, it's doing the same thing in name property ?

Do you have a reproduction case which I could perhaps try locally ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's doing the exact same thing as in the name property. But no, I don't have a very portable reproduction case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a short script that frequently fails. Though if I change the pool name to the much smaller and less busy zroot, then it passes.

import libzfs
zfs = libzfs.ZFS()
ds = zfs.get_dataset("atl-rb22b-ss.b.24")
pool = ds.pool
pool.sync()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asomers can you please create a ticket at https://ixsystems.atlassian.net and link this script there and we can investigate as to why this could be failing potentially. Do mention your scale version please as well. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is a "scale version"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW I'm not a TrueNAS user. I'm using a custom Python application on FreeBSD stable/13.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asomers interesting. I am not able to reproduce using the same script ( i am executing the same code path in a loop ). I wonder if it's a cython/python version issue ?
Could you share some more details wrt cython/python version you have please ? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Python 3.9.13
Cython version 0.29.30

cdef NVList innvl = NVList()

innvl["force"] = force
Expand Down