Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ignore_vdis to VM.snapshot method #4563

Merged
merged 1 commit into from Dec 7, 2021

Conversation

benjamreis
Copy link
Contributor

This allow to snapshot a VM and ignore some VDIs during the snapshot
This can lead to a gain of time & space ignoring non essential data

See: #4551

Signed-off-by: BenjiReis benjamin.reis@vates.fr

@benjamreis benjamreis changed the title [WIP] Add ignored_vdis to VM.snampshot method [WIP] Add ignored_vdis to VM.snapshot method Oct 28, 2021
@benjamreis benjamreis force-pushed the exclude_vdi_from_snapshot branch 4 times, most recently from e7df3b0 to 2860b07 Compare October 28, 2021 12:45
ocaml/idl/datamodel_vm.ml Outdated Show resolved Hide resolved
ocaml/xapi/xapi_vm.ml Outdated Show resolved Hide resolved
@benjamreis benjamreis force-pushed the exclude_vdi_from_snapshot branch 3 times, most recently from e535ace to d60fd22 Compare November 2, 2021 12:46
@benjamreis benjamreis changed the title [WIP] Add ignored_vdis to VM.snapshot method [WIP] Add ignore_vdis to VM.snapshot method Nov 2, 2021
@lindig
Copy link
Contributor

lindig commented Nov 2, 2021

Could you confirm that the List.filter logic is correct?

@benjamreis
Copy link
Contributor Author

benjamreis commented Nov 3, 2021

Could you confirm that the List.filter logic is correct?

I forgot a not in the condition indeed since I want to exclude all VBDs that are on a VDI present in ignore_vdis

@lindig
Copy link
Contributor

lindig commented Nov 3, 2021

You might want to add tests because we are unlikely to use the new feature and therefore adding tests

@benjamreis
Copy link
Contributor Author

Yes, I'll look into the XAPi tests and some for this. I'll also link a PR here to our own repo of tests on real XCP-ng hosts.

ocaml/xapi/xapi_vm_clone.ml Outdated Show resolved Hide resolved
@benjamreis
Copy link
Contributor Author

Hi!

Any update on this?
It seems impossible to have optional params in XAPI datamodel object's methods. :/

I'd like to write tests but as long as my PR can't build I can"t.

Thanks.

@benjamreis benjamreis force-pushed the exclude_vdi_from_snapshot branch 2 times, most recently from 4b68aa8 to d161ab5 Compare November 24, 2021 14:58
@benjamreis
Copy link
Contributor Author

I don't understand the error about pool secret. Can it be mocked?

@benjamreis
Copy link
Contributor Author

benjamreis commented Nov 30, 2021

Hi,

Can someone help me initiate a pool secret in my tests? IDK how to do it in a mocked context.

[failure] the pool secrets either do not exist or have not been loaded yet
          Raised at file "stdlib.ml", line 29, characters 17-33
          Called from file "ocaml/xapi/xapi_globs.ml" (inlined), line 46, characters 6-89
          Called from file "ocaml/xapi/helpers.ml", line 448, characters 8-35
          Called from file "ocaml/xapi/xapi_vm_snapshot.ml", line 33, characters 4-98
          Called from file "ocaml/xapi/xapi_vm.ml" (inlined), line 762, characters 2-65
          Called from file "ocaml/tests/test_vm_snapshot.ml", line 25, characters 4-71
          Called from file "src/alcotest-engine/core.ml", line 277, characters 17-23
          Called from file "src/alcotest-engine/monad.ml", line 34, characters 31-35

Thanks

@benjamreis benjamreis force-pushed the exclude_vdi_from_snapshot branch 2 times, most recently from 6cc994a to 6cf4a7d Compare December 2, 2021 09:28
@benjamreis
Copy link
Contributor Author

I'll let the PR in WIP until I'm done with my manual tests & wrote more tests in our test repo.
I don't think the code will change though so it's good to be reviewed :)

benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request Dec 6, 2021
See: xapi-project/xen-api#4563

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
ocaml/idl/datamodel_vm.ml Outdated Show resolved Hide resolved
ocaml/xapi-cli-server/cli_frontend.ml Outdated Show resolved Hide resolved
benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request Dec 6, 2021
See: xapi-project/xen-api#4563

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
@benjamreis benjamreis force-pushed the exclude_vdi_from_snapshot branch 2 times, most recently from ce52822 to 870d3ba Compare December 6, 2021 15:37
benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request Dec 7, 2021
See: xapi-project/xen-api#4563

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
@benjamreis benjamreis force-pushed the exclude_vdi_from_snapshot branch 2 times, most recently from 07bb230 to 60ba28d Compare December 7, 2021 09:18
This allow to snapshot a VM and ignore some VDIs during the snapshot
This can lead to a gain of time & space ignoring non essential data

See: xapi-project#4551

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
@benjamreis
Copy link
Contributor Author

Sorry for the ping, I misscliked ^^'.
Is this good for merge?

I re-ran the tests on a host after latest modifcations, test is successful + no XAPI objects leaked. 👍

@lindig
Copy link
Contributor

lindig commented Dec 7, 2021

I think this is good to go. Thanks for going trough the many iterations for something that looked quite simple conceptually.

@benjamreis
Copy link
Contributor Author

No problem, I learned how XAPI unit tests work which is a good thing for future PR. :)
Thanks for your patience!

@robhoes robhoes merged commit 1121e4b into xapi-project:master Dec 7, 2021
@benjamreis benjamreis deleted the exclude_vdi_from_snapshot branch December 7, 2021 12:10
benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request Dec 7, 2021
See: xapi-project/xen-api#4563

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request Dec 7, 2021
See: xapi-project/xen-api#4563

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request Dec 7, 2021
See: xapi-project/xen-api#4563

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request Dec 7, 2021
See: xapi-project/xen-api#4563

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request Dec 7, 2021
See: xapi-project/xen-api#4563

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
benjamreis added a commit to xcp-ng/xcp-ng-tests that referenced this pull request Dec 9, 2021
See: xapi-project/xen-api#4563

Signed-off-by: BenjiReis <benjamin.reis@vates.fr>
@olivierlambert
Copy link

Hey @robhoes

I can't find the modification in the generated XAPI doc (should be there https://xapi-project.github.io/xen-api/releases/next.html right?)

Am I missing something?

@psafont
Copy link
Member

psafont commented Jan 24, 2023

The API changes weren't properly tracked for some time. Now it's quite better, but not quite enough for this case (parameter versioning is not properly tracked yet)

It should have appeared in https://xapi-project.github.io/xen-api/releases/21.4.0.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants