-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add support for V2 SMB leases so that epoch checking can be done. #118
Conversation
(Related to new tests for PSCALE-128466)
pike/smb2.py
Outdated
self.parent_lease_key = None | ||
self.epoch = None | ||
self.lease_flags = LeaseFlags.SMB2_LEASE_FLAG_NONE | ||
self.parent_lease_key = array.array("B", [0] * 16) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parent_lease_key
and epoch
are only for v2 right but we set isV2
later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe retain the old code and have new code under check if do_v2_lease
, then set isV2
within.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured it was harmless to just set it unconditionally, but I can certainly do it this way if you prefer.
(Meanwhile, Masen had given me some review comments for this on slack, and wants me to get rid of my isV2 field and go back to using just the 'flags' for that. So, will be updating this soon...
pike/smb2.py
Outdated
self.lease_flags = None | ||
self.parent_lease_key = None | ||
self.epoch = None | ||
self.lease_flags = LeaseFlags.SMB2_LEASE_FLAG_NONE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also wondering if we should set SMB2_LEASE_FLAG_PARENT_LEASE_KEY_SET
under condition if do_v2_lease
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
srv currently doesn't support parent lease keys, so we don't want to set the flag. (If we set the flag, we'd also need to actually set a valid parent_lease_key -- which we can't because we don't support. I believe parent_lease_key is related to directory leases.)
if do_v2_lease: | ||
self.isV2 = True | ||
if lease_flags_v2 is not None: | ||
self.lease_flags = lease_flags_v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just pull out this statement and a similar statement in else
outside of if-else
block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.lease_flags = lease_flags_v2
(Related to new tests for PSCALE-128466)