Implement SMB2 LOCK operation#309
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #309 +/- ##
==========================================
- Coverage 99.06% 99.03% -0.04%
==========================================
Files 24 24
Lines 5146 5184 +38
==========================================
+ Hits 5098 5134 +36
- Misses 48 50 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| self.file_attributes = c_resp["file_attributes"].get_value() | ||
| return c_resp | ||
|
|
||
| def lock(self, locks, lsn=0, lsi=0, wait=True, send=True): |
There was a problem hiding this comment.
Do you have a particular reason for exposing the sequence and index numbers right now? The protocol docs somewhat indicate they are used for some global state that I don't fully understand and am wondering if it is just best to keep it at 0 and expose it when requested by someone.
There was a problem hiding this comment.
I exposed it because it's something that clients may use for dialects > 2.0.2. I don't fully understand it either, but I wanted to be able to reproduce any sequence of operations that a client may send to a server (the reason I implemented the LOCK operation was to reproduce an issue we detected with Samba, though it didn't use lsn/lsi).
I think it doesn't hurt to have them, and they may become useful in some cases. The protocol has them, so I don't see why we should hide them. In normal cases they won't be used, so the default value of 0 will be applied.
There was a problem hiding this comment.
Fair enough, I didn't want to paint us into a corner but it's probably not going to be a problem. We can always deprecate things if we want to take away the ability to set these if we ever find out it should be calculated internally somehow but I doubt that would happen.
| lock["locks"] = locks | ||
|
|
||
| if self.connection.dialect > Dialects.SMB_2_0_2: | ||
| lock["lock_sequence"] = (lsn << 28) + lsi |
There was a problem hiding this comment.
If we do decide to expose this, we should ensure that lsn fits into the 28 bits and lsi isn't more than 4 bits. If they are then we should raise a ValueError.
There was a problem hiding this comment.
Sure. I'll add those checks.
Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
|
Thanks for implementing this feature! |
2 participants
This PR adds support for locking and unlocking byte ranges in files.