CP-51042: Introduce new SR.scan2 for SMAPI{v1,v2,v3} #5959
Conversation
Vincent-lau
force-pushed
the
private/shul2/sr-scan2
branch
from
115ee83 to
85b222e
Compare
| List.fold | ||
| ~f:(fun set x -> | ||
| match | ||
| List.Assoc.find x.Xapi_storage.Control.keys |
There was a problem hiding this comment.
I assume this list is short? Otherwise this could be costly if response is large.
There was a problem hiding this comment.
I copied this from the previous implementation, but response contains a list of volumes within an SR, and the max length of the list would be defined as the max num of volumes, which can be large potentially https://docs.xenserver.com/en-us/xenserver/8/system-requirements/configuration-limits.html
There was a problem hiding this comment.
and this would depend on the size of keys as well, which I believe should be small. But even if it is large, there is not much we can do about it, creating an set for each x in the list of volumes? That would be a O(mn) operation itself (plus O(mn) space)
| "unreachable" | ||
| | Unavailable -> | ||
| "unavailabe" | ||
| [@@deriving rpcty, show {with_path= false}] |
There was a problem hiding this comment.
how about this: removed the to_string function now since I can't type it right
Vincent-lau
force-pushed
the
private/shul2/sr-scan2
branch
from
e2c44c0 to
9301b85
Compare
ocaml/xapi/xapi_sr.ml
Outdated
| ) | ||
| to_update | ||
|
|
||
| let scan2 ~__context ~sr = |
There was a problem hiding this comment.
This function is added (and unused) in the first commit and removed in the second. I suspect it was for testing, but to avoid confusion it would be better to exclude it from the first commit already.
There was a problem hiding this comment.
Kind of. What I did there in the first commit was to introduce scan2 everywhere and not delete anything related to the original scan. And then in the second commit, I deleted scan from xapi_sr.ml and renamed scan2 to scan as a replacement. Maybe that did not come across in the diff view as it looks like I introduced scan2 and then deleted it again, but it was really introduce scan2 and then delete scan + rename scan2 to scan
There was a problem hiding this comment.
I will add a few more explanations in the commit message.
Add a new API SR.scan2 as a toplevel function which will will be implemented differently on SMAPIv1 and SMAPIv3. But not calling it yet, as that will be done in the following commit. On SMAPIv1, this will be implemented as a scan followed by a stat, which is as implemented before. It is important to keep this ordering because the storage backend relies on the scan call to do the resizing. On SMAPIv3, this is implemented as a stat followed by an ls, and the ls is only called if the SR is healthy, as returned by the stat. This is Ok since on SMAPIv3 SR.stat does the resizing instead. Signed-off-by: Vincent Liu <[email protected]>
Vincent-lau
force-pushed
the
private/shul2/sr-scan2
branch
from
9301b85 to
23c4a0c
Compare
In other words rename scan2 to scan and delete the original scan implementation. This is fine since the function signature remains the same, but internally Xapi_sr.scan will now call SR.scan2 rather than SR.scan. scans in other places, such as storage_mux.ml are left untouched for backwards compatability. Signed-off-by: Vincent Liu <[email protected]>
4 participants
Second attempt of the reverted PR, this time introduce a new SMAPIv2 API and let SMAPIv1 and v3 implementations to decide what to do