IH-728: Refactor SXM code, Pt1 #6084
Conversation
|
Great stuff! |
| @@ -1383,14 +1376,14 @@ let with_task_and_thread ~dbg f = | |||
|
|
|||
| let copy ~dbg ~sr ~vdi ~url ~dest ~verify_dest = | |||
| with_task_and_thread ~dbg (fun task -> | |||
| MigrateLocal.copy_into_sr ~task ~dbg:dbg.Debug_info.log ~sr ~vdi ~url | |||
| ~dest ~verify_dest | |||
| MigrateLocal.copy_into_sr ~task ~dbg:(Debug_info.to_string dbg) ~sr ~vdi | |||
There was a problem hiding this comment.
This is a different string now. The dbg.Debug_info.log string was used here (and below), because the functions further down do not yet handle dbg strings that have both the "log" and "tracing" data combined, which can then end up in the "task" part of log lines. Or has that changed now?
There was a problem hiding this comment.
That is quite true. But line 733, which I deleted, has let dbg = dbg_and_tracing_of_trask task in start function will mean that anything below that line in the start function would be using a dbg parameter with "log" and "dbg" in them. Was this going to cause the same problem as you described?
There was a problem hiding this comment.
I had a look at the following functions which use dbg, it seems that they are all SMAPIv2 calls, which handles tracing already. If they are what you were referring to as functions further down?
|
Thanks everyone for the quick review. I have messed up the commit messages in this PR a bit, where the middle PR "Restructuring Storage migration code" has a lot of changes in it. I will now force push the changes to make it more explicit in each commit I am changing. |
Vincent-lau
force-pushed
the
private/shul2/sxm-clean2
branch
2 times, most recently
from
7f2bc78 to
d608dd6
Compare
Vincent-lau
force-pushed
the
private/shul2/sxm-clean2
branch
from
d608dd6 to
20fb265
Compare
Signed-off-by: Vincent Liu <[email protected]>
Vincent-lau
changed the title
There is no remote invocation of this function, so should be safe. Signed-off-by: Vincent Liu <[email protected]>
Rather than copy and copy', rename them into `copy_into_sr` and `copy_into_vdi`, which is what they actually do. Signed-off-by: Vincent Liu <[email protected]>
Also make the logging information more verbose. Signed-off-by: Vincent Liu <[email protected]>
- Introduce new `MigrateLocal` and `MigrateRemote` modules which contains the main implementations of the migration logic. - Move the actual exposed SMAPIv2 functions in one place towards the end of the file, rather than spreading across the entire file. These refactoring should all be statically verifiable by the compiler. Signed-off-by: Vincent Liu <[email protected]>
Signed-off-by: Vincent Liu <[email protected]>
Previously a task was constructed based on the log and tracing of a dbg of the type Debug_info.t, and then later on a dbg is constructed based on the previously constructued task. Instead of that, just convert the first dbg into a string and thread it through the call. Signed-off-by: Vincent Liu <[email protected]>
The Storage migration code is too messy to be maintainable, making the developing experience quite bad. This is the first attempt to make it less spaghetti.
I am intending to do this in several stages. This PR is just for those that are mostly "safe" operations that can probably be checked statically by the compiler. More risky changes on the way...
Best reviewed by commit.