CP-48463: add parameter to skip device types on get_export_metadata #5544
Conversation
psafont
force-pushed
the
private/paus/chelydra
branch
from
2228822 to
3ce7acd
Compare
| ) | ||
| vmrefs ; | ||
| (* We only keep VMs which are not snapshot *) | ||
| (* When only snapshots have been imported, return all of them. |
There was a problem hiding this comment.
I don't have the context of this - but is this a behavior that customers expect?
There was a problem hiding this comment.
Customers can export snapshots from a pool, without this change when importing this back in, their uuid/ reference can't be known for sure. This leads to uncomfortable ergonomics.
I think returning the references of snapshots just created back to the user is something they expect.
There was a problem hiding this comment.
I was more confused by the difference between when only snapshots are imported and the case were a mix of snapshots and VMs are imported - as I read the comment, the behaviour is different.
There was a problem hiding this comment.
Before this PR the export of metadata-only was blocked, so I don't think this change of behaviour affects any users :)
I am not sure where this restriction came from. It was introduced in the same commit that introduced the metadata-export feature itself, without explanation. Full exports of snapshots are already allowed. Signed-off-by: Rob Hoes <[email protected]>
This allows clients to know which snapshots they knowingly just created and act on them instead of looking again into the db and try to divine which snapshots the call created. Signed-off-by: Pau Ruiz Safont <[email protected]>
This allows clients to detect that the newer device type exclusions for export are available. Signed-off-by: Rob Hoes <[email protected]>
This allows to orchestrators to copy devices like VBDs from the VM, export the metadata without the disk, import the VM to any pool, and finally add the saved VBDs to the new VM before booting it. Signed-off-by: Pau Ruiz Safont <[email protected]>
This allows to easily test the http endpoint. Also advertises the metadata parameter on these calls Signed-off-by: Pau Ruiz Safont <[email protected]>
Use the helper functions to process parameters, use atomics instead of ad-hoc references for counting in possibly-parallel case Signed-off-by: Pau Ruiz Safont <[email protected]>
Signed-off-by: Pau Ruiz Safont <[email protected]>
Signed-off-by: Pau Ruiz Safont <[email protected]>
Signed-off-by: Pau Ruiz Safont <[email protected]>
Tooling complains about using 1970 as the date otherwise Signed-off-by: Pau Ruiz Safont <[email protected]>
psafont
force-pushed
the
private/paus/chelydra
branch
from
3ce7acd to
0c6805f
Compare
| [op (List.hd srs)] | ||
| | [sr] -> | ||
| [op sr] | ||
| | _ -> |
There was a problem hiding this comment.
can we use when here to make this more pattern matchy? like
| [sr] -> [op sr]
| srs -> when multiple && get_bool_param params ...There was a problem hiding this comment.
This file is in need of a lot of care to use sensible coding standards. I think removing the use of List.hd is enough for now. Maybe as an OIL task...
| (* add VTPMs that belong to this VM *) | ||
| vm.API.vM_VTPMs | ||
| |> List.iter (fun ref -> if Db.is_valid_ref __context ref then add ref) ; | ||
| if not (List.mem Devicetype.VTPM excluded_devices) then |
There was a problem hiding this comment.
Do we allow users to exclude vtpms as well (and then add them back later on)?
There was a problem hiding this comment.
Yes, this code is doing the exclusion. For adding it back to the VM, I don't think there's an easy way to export the TPM contents separately
This comment was marked as off-topic.
This comment was marked as off-topic.
psafont
changed the title
4 participants
This allows orchestrators to copy devices like VBDs from the VM, export the
metadata without the disk, import the VM to any pool, and finally add the
saved VBDs to the new VM before booting it.
Also include cleanups in the cli server, it's full of ad-hoc methods.
This has passed manual testing, the corresponding FQP test by QA, and tested by CVAD people as well