Keep ID argument#41
Conversation
get_vmids() is split out of get_free_vmid(), to make it common for new check_vmid_free(). check_vmid_free() checks the availability of a vm id. get_free_vmid() would not use any free IDs before the lowest ID. The length to ID diff was replaced/refactored for readability.
wdoekes
left a comment
wdoekes
left a comment
There was a problem hiding this comment.
Thanks for the PR.
Also refactored the get_vmids() function while splitting it, as it had a bug, not using valid VM IDs (>= 100 < FIRST_USED_ID).
I'm sorry, I don't parse that sentence. But I see there is indeed a bug in that it will never hand out IDs below the first existing ID.
Could you split this up into 2 commits:
- fixing the bug / refactoring / splitting
- adding keep-id
| prefer, (aes128-gcm@openssh.com is supposed to | ||
| be fast if you have aes on your cpu); set to | ||
| "-" to use ssh defaults | ||
| --keep-id preserve the source vm id (useful if you want to keep a backup trail) |
There was a problem hiding this comment.
Limit line lengths to 79 please. (Applies elsewhere ass well.)
| if self.opt_keep_id: | ||
| dst_id = src_vm.id | ||
| if not self.dst_pve.check_vmid_free(dst_id): | ||
| raise PrepareError( |
There was a problem hiding this comment.
Too late to raise PrepareErrors here. This should've been checked in move_vm() (also done in dry-run).
Here we can safely assume the ID is free and let Proxmox handle duplicate VM id errors.
|
What would be a nice extension to this patch would be a way to specify a new base id for migrated VMs. (which for a single move would be a way to specify the new ID for the VM) |
|
@engelant: will you be working on this? |
|
@wdoekes : Sorry, but I don't hava a dev environment of two proxmox instances anymore. This was the patch I used for myself at the given time to transfer a server. |
|
I understand 👍 |
3 participants
Added an argument for preserving the source ID of the VM if possible.
Raises an error if ID is already in use.
Also refactored the get_vmids() function while splitting it, as it had a bug, not using valid VM IDs (>= 100 < FIRST_USED_ID).