add --restore-root option to specify target for misc vm files

[richardstephens]

Provide a flag to specify where misc files (NVRAM vars, loader, etc) restored from the backup should be placed.

[abbbi]

hi,
thanks for the proposal, but that would also mean that the virtual machine config must be adjusted
accordingly? At least for some stuff like the nvram etc, pathes are also within the VM config.

[richardstephens]

For our use case, we are re-generating the libvirt XML from scratch. But if you think that makes sense i can implement it :)

[richardstephens]

Just picked up a bug with this implementation in testing - the arg is not present on backup, so the makedirs check caused taking a backup to fail.

While testing I also observed that the current implementation of adjust_config sets relative paths for the qcow2 images. This doesn't matter for our use case but doesn't seem like correct behaviour - would you like me to fix that too?

[abbbi]

Just picked up a bug with this implementation in testing - the arg is not present on backup, so the makedirs check caused taking a backup to fail.

what about remote restore? as far as i had a quick look the implementation wouldnt
handle this currently, right?

While testing I also observed that the current implementation of adjust_config sets relative paths for the qcow2 images. This doesn't matter for our use case but doesn't seem like correct behaviour - would you like me to fix that too?

not sure, i havent had any user complain about this.
It would probably be good to enhance the testcases in t/tests.bats for this new funtionality too. You can run the testsuite locally so ensure the new feature doesnt introduce any issues or breaks old functionality.

[richardstephens]

as far as i had a quick look the implementation wouldnt handle this currently, right?

I've not implemented makedirs for the remote restore, no. I'll take a look

It would probably be good to enhance the testcases in t/tests.bats for this new funtionality too.

Will do

[richardstephens]

not sure, i havent had any user complain about this.

I did some testing and i was unable to get it to work

[abbbi]

not sure, i havent had any user complain about this.

I did some testing and i was unable to get it to work

what exactly? Can you provide a concrete example?

in my tests:

./virtnbdrestore -i /tmp/backup -o /tmp/my/restore -c
[..]

results in the xml file:

<source file="/tmp/my/restore/vm1-sda.qcow2" index="3"/>

and like the documentation states, the UEFI and pflash files (if defined) are currently allways restored to the original pathes (if not existant). Becauase usually these pathes in on the hypervisor (especially for UEFI, which might be updated by OS updates). The UEFI files are usually part of system packages, shared amongst multiple virtual machines.

For nvram or UEFI vars files, it makes sense to provide at least an option to relocate them during restore.

It may make sense to implement it this way:

./virtnbdrestore -i /tmp/backup -o /tmp/my/restore -c

1. restores the virtual disk files to /tmp/my/restore/

2. copies the latest UEIF_VARS.fd to /tmp/my/restore (nvram setting)

3. if option -c given: adjust both path to virtual disk and UEFI_VARS.fd

4. the a actual path to the loader file (the BIOS itself) should be left untouched, as it is usually installed by some third party packages. The user could decide to restore it or not.

[richardstephens]

# /tmp/backup/virtnbdbackup-wip/virtnbdrestore -i b1 -o b2d -R b2r -c -L restorelog
# virsh define b2d//vmconfig.xml 
Domain 'restore_vm-bt5-7BcP6T74' defined from b2d//vmconfig.xml
# virsh start restore_vm-bt5-7BcP6T74
error: Failed to start domain 'restore_vm-bt5-7BcP6T74'
error: Cannot access storage file 'b2d/vda.qcow2' (as uid:0, gid:0): No such file or directory

The XML gets generated with the relative path from where the restore was run. Libvirt does not update the XML on import

    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2'/>
      <source file='b2d/vda.qcow2'/>

(Additionally, this testing also revealed another issue with this PR, that the exported XML may contain firmware='efi', which will cause libvirt to trigger its firmware detection logic. I'll update the PR to remove that if the loader is present.)

[abbbi]

The XML gets generated with the relative path from where the restore was run. Libvirt does not update the XML on import

yes, documentation and all examples work with absolute pathes.
I just fixed this with commit:

5ba4657

[richardstephens]

It may make sense to implement it this way:

./virtnbdrestore -i /tmp/backup -o /tmp/my/restore -c

1. restores the virtual disk files to /tmp/my/restore/
2. copies the latest UEIF_VARS.fd to /tmp/my/restore (nvram setting)
3. if option -c given: adjust both path to virtual disk and UEFI_VARS.fd
4. the a actual path to the loader file (the BIOS itself) should be left untouched, as it is usually installed by some third party packages. The user could decide to restore it or not.

I should clarify our motivation for these changes - in our use case, we do not (typically) use OVMF files distributed by the system package manager - we bundle the loader and template blobs into a VM template and store them alongside the VM. That way we can manage their lifecycle on a per-VM basis.

The proposed approach would almost work for us, except for 4. Of course we could always implement our own process to back these files up seperately but I would ask what the point of virtnbdbackup backing them up in the first place is if it doesn't also provide a mechanism to restore them.

There's also the question of how kernel/initrd files should be treated, if present. It seems weird to treat them too differently. We don't use this method of OS loading currently but I expect we will in the future.
One of my goals with this approach was to avoid changing any existing behaviour - if changing existing default behaviour is acceptable, I would propose this solution:
1. By default, kernel, intird and nvram vars are restored to stable paths in the --ooutput directory. The loader is left untouched. If --adjust-config was provided, the paths would be rewritten.
2. A new boolean flag is provided, --restore-loader that would also restore the loader to underneath the output directory. If used in conjunction with --adjust-config, it would also update the <loader/> xml element

What do you think?

Re-reading the thread I realise i misunderstood your suggestion. I will have a think and get back to you

[abbbi]

The proposed approach would almost work for us, except for 4. Of course we could always implement our own process to back these files up seperately but I would ask what the point of virtnbdbackup backing them up in the first place is if it doesn't also provide a mechanism to restore them.

because usually you only lose a virtual machine, not the complete hypervisor.
If you lose the complete hypervisor, you have at least the possibility to restore the bios from the backup, in case you cant setup the same os version, whatever.

[richardstephens]

Apologies for my radio silence on this.

As an alternative approach, do you think it would make sense to simply add arguments for --restore-loader and --restore-nvram?