VMware Plugin: Add option restore_allow_disks_mismatch#1876
Conversation
|
This is an alternative to PR #1796 |
arogge
left a comment
There was a problem hiding this comment.
The code itself looks pretty good. I'd rename the option and make one of the debug messages a job message.
| bareosfd.DebugMessage( | ||
| 100, | ||
| "add_disk_devices_to_vm(): Disk %s already exists, retrying with datastore name only\n" | ||
| % (device_spec.device.backing.fileName), | ||
| ) |
There was a problem hiding this comment.
maybe this should be a JobMessage instead?
There was a problem hiding this comment.
The plugin tries to preserve the disk file names on restore if possible. When using the vmname option on restore, the plugin tries to avoid that exception by renaming the disk paths before. But when using the folder option, that's not possible. In that case this exception is normal and not worth a job message. Note that VM names must only be unique within a folder. May be it makes more sense to rethink this, probably trying to preserve the disk names isn't really necessary when the VM is recreated, as it can't be guaranteed anyway.
There was a problem hiding this comment.
So by default we currently try to recreate the disks (that's basically VMDK-Files on the VMFS store, right) with the name they had when they were backed up. As you said, we could just always recreate the disks with new names.
With the current implementation, the user may expect to get the same names, because that is what usually happens. Thus I argued there could be a job message (M_INFO is totally sufficient) when we don't meet that expectation.
But feel free to ignore that, if it doesn't make sense - it is just a suggestion :)
|
In the new commit add_disk_devices_to_vm() changed so that it only passes the datastore name as backing path. Like this it should always succeed and register the new disk file name. This is only called when recreating the VM, and if the disk path/filename differs from the backed up name, it will generate a job message. I think like this it should also work with VSAN, without the need to allow mismatch of the disk check. |
arogge
left a comment
There was a problem hiding this comment.
While I sadly cannot test this, when reading the code it looks good to me!
It was reported that restoring to recreated VM fails due to disk check mismatch when using VSAN datastore. Setting the plugin option restore_ignore_disks_mismatch=yes when restoring will ignore a mismatch in the check, which should allow successful restore.
add_disk_devices_to_vm() now always sets the backing filename to contain the datastore name only, so that won't the exception vim.fault.FileAlreadyExists won't happen. Instead any unexpected exception when adding a disk is handled now with a limited number of retries. Also a job message is emitted, when the backing filename of the created disk does not match the expected one. Renamed the plugin option to restore_allow_disks_mismatch, but with the change described above, it's probably not needed even when using VSAN.
b4a8529 to
fa2fe8d
Compare
Adds the option
restore_ignore_disks_mismatchto the VMware Plugin.This can be helpful when using VSAN and the restore fails due to disk check mismatch
when the VM is recreated by the plugin.
Helpful documentation and best practices
Checklist for the reviewer of the PR (will be processed by the Bareos team)
Make sure you check/merge the PR using
devtools/pr-toolto have some simple automated checks run and a proper changelog record added.General
Source code quality