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

feat: implement ability to clone Qemu VM to different storage #352

Merged

Conversation

OidaTiftla
Copy link
Contributor

Hi,

I want to create a clone to a different storage as it is possible through the Web UI and I noticed that this option is already documented in the README.md, but not yet implemented.

The changes in this PR will allow cloning a Qemu VM to a different storage location without changing the original configuration.

I also added a unit test, but I could not run and verify the unit test with make test. I did not found out how to setup the unit test so the new storage other-storage will exist when the unit test runs. Should I keep the new unit test, or should I remove it?

Nevertheless I testet the change on my Proxmox 8 and it worked.

If there is anything I can improve in this PR, please let me know 😉.

Allows cloning a Qemu VM to a different storage location without changing
the original configuration.
This is already documented in the README.md but was not implemented yet.
@OidaTiftla OidaTiftla force-pushed the allow-clone-to-user-defined-storage branch from d5be3ac to 0b6d93c Compare August 18, 2024 15:28
@OidaTiftla
Copy link
Contributor Author

Hi @Tinyblargon, I don't know what is the correct workflow and if I need to mention you on this pull request.

Let me known if there's anything I need to modify so this can be merged.

Kind regards,
OidaTiftla

Copy link
Collaborator

@Tinyblargon Tinyblargon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one minor change.

proxmox/config_qemu.go Outdated Show resolved Hide resolved
proxmox/config_qemu.go Show resolved Hide resolved
@@ -35,6 +35,27 @@ func Test_Clone_Qemu_VM(t *testing.T) {

}

func Test_Clone_Qemu_VM_To_Different_Storage(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently none of the API test are really functional but this tells us what situation we should have coverage for.

@OidaTiftla
Copy link
Contributor Author

@Tinyblargon thanks for your feedback. You may have a look at the updated version.

@Tinyblargon Tinyblargon merged commit b6deb23 into Telmate:master Aug 20, 2024
5 checks passed
@OidaTiftla OidaTiftla deleted the allow-clone-to-user-defined-storage branch August 21, 2024 00:05
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.

2 participants