CP-15502: Adding VM Scheduled Snapshot functions, APIs, CLIs and records. #2577
Conversation
| | "snapshot" -> `snapshot | ||
| | "checkpoint" -> `checkpoint | ||
| | "snapshot_with_quiesce" -> `snapshot_with_quiesce | ||
| | _ -> raise (Record_failure ("Expected 'snapshot', 'checkpoint', 'snapshot_with_quiesce'")) |
There was a problem hiding this comment.
Can you include the string which was actually received in this exception, as string_to_network_default_locking mode does?
|
I notice that the design is called 'scheduled snapshots' but in the rest of the design document and in the code here (both external strings and internal definitions) you use 'schedule snapshot'. 'Scheduled snapshot' seems more correct to me. |
| List.map (fun (k,v)-> k,(assert_key ~field ~ks ~key:k ~value:v)) value | ||
| ) | ||
| else None | ||
| ) |
There was a problem hiding this comment.
All the functions from mem above to here seem like generic utilities which should be in a general library module.
They should all have unit tests.
There was a problem hiding this comment.
There are already some functions like this for handling key-value pair lists in map_check.ml - that might be a good place to add them.
|
Does the actual snapshot scheduler apply any jitter to the schedule? There must be some mechanism to spread out the load of taking the snapshots - otherwise in the worst case every 15 minutes Xapi will grind to a halt as it snapshots every VM at the same time. |
Signed-off-by: Sharad Yadav <sharad.yadav@citrix.com>
1) scheduled-snapshot : VM part of scheduled snapshot. 2) is-vmss-snapshot : Snapshot created from VMSS. Signed-off-by: Sharad Yadav <sharad.yadav@citrix.com>
Signed-off-by: Sharad Yadav <sharad.yadav@citrix.com>
Signed-off-by: Sharad Yadav <sharad.yadav@citrix.com>
Signed-off-by: Sharad Yadav <sharad.yadav@citrix.com>
Signed-off-by: Sharad Yadav <sharad.yadav@citrix.com>
| (* Check VMs associated to VMSS supports the new snapshot type *) | ||
| let snapshot_type = Record_util.vmss_type_to_string value in | ||
| let vms = Db.VMSS.get_VMs ~__context ~self in | ||
| if not (vms = []) then |
|
Removing the reviewed label since there are more changes coming according to sharad's comment. |
Signed-off-by: Sharad Yadav <sharad.yadav@citrix.com>
sharady
force-pushed
the
CP-15502-v2
branch
2 times, most recently
from
91763d4
to
a1ad48f
Compare
|
@euanh I have moved the key:value check functions to map_check.ml |
simonjbeaumont
changed the title
|
@sharady you can remove the needs updating label if you think it's ready for (re-)review. |
Signed-off-by: Sharad Yadav <sharad.yadav@citrix.com>
|
I haven't reviewed this PR, but I did notice some terminology problems. |
|
closing this until we expect progress |
No description provided.