abh1sar opened a new pull request, #14007: URL: https://github.com/apache/cloudstack/pull/14007
### Description Restoring a volume from a backup and attaching it to a VM doesn't work since the restore commands were changed to run without a shell in https://github.com/apache/cloudstack/commit/56ad044865bc539231a27fcb459326674b0d1fa5 1. getDeviceToAttachDisk pipes virsh domblklist through awk, but passes the awk program still wrapped in the single quotes a shell would have stripped. Run directly, awk fails with "invalid char" and returns nothing, so the device name is empty and charAt throws StringIndexOutOfBoundsException before any attach is attempted. This affects every storage type. The exit value was also never checked, and the output not trimmed, so even a working awk would leave the trailing line separator and increment that instead of the device letter. 2. The RBD branch passes the literal string "<<EOF%sEOF" as a virsh argument. The placeholder is never substituted with the disk XML, and a here-document cannot work without a shell, so virsh is handed a bogus argument and fails. The XML is now written to a temporary file that virsh reads. 3. The Linstor branch declares "--subdriver qcow2", inverting the previous behaviour where Linstor got a raw attach and every other pool got qcow2. A Linstor volume is a raw DRBD block device, so libvirt rejects it with "Image is not in qcow2 format". The condition is restored, along with the "--driver qemu" that was dropped. <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [x] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): ### How Has This Been Tested? Before fix: 1. awk quoting — getDeviceToAttachDisk (breaks restore-and-attach on all storage) API ``` Error restoring volume [0965ff84-c3e2-4bbe-acfe-a2ae239f61fb] of VM [7b3f859c-1560-42c7-b2e0-61138e2cb8d1] to host [294e5d7b-3581-46be-b33b-b1df6965a563] using backup provider [nas] due to: []. Note the empty [] — the details are lost, which is part of what makes it hard to diagnose. ``` Agent log: ``` java.lang.StringIndexOutOfBoundsException: String index out of range: -1 at java.base/java.lang.StringLatin1.charAt(StringLatin1.java:48) at com.cloud.hypervisor.kvm.resource.wrapper.LibvirtRestoreBackupCommandWrapper.getDeviceToAttachDisk(LibvirtRestoreBackupCommandWr java:401) at com.cloud.hypervisor.kvm.resource.wrapper.LibvirtRestoreBackupCommandWrapper.attachVolumeToVm(LibvirtRestoreBackupCommandWrapper 368) at com.cloud.hypervisor.kvm.resource.wrapper.LibvirtRestoreBackupCommandWrapper.restoreVolume(LibvirtRestoreBackupCommandWrapper.java:192) Root cause at OS level (observed on nas-h1, output truncated at 70 chars by my harness): awk: cmd. line:1: '{print $1}' awk: cmd. line:1: ^ invalid char ''' in ``` 2. RBD <<EOF%sEOF (Ceph only; only reachable once 1 is fixed) Agent log: ``` Executing command [/usr/bin/virsh attach-device i-2-5-VM /dev/stdin <<EOF%sEOF ]. Execution of process [1818313] for command [...] failed. Exit value of process [...] is [1]. API (observed): Error restoring volume [...] ... due to: [Failed to attach volume to VM: i-2-5-VM]. ``` 3. Linstor --subdriver qcow2 inversion (LINSTOR only; also only reachable after 1) Agent log: ``` Execution of process [7761] for command [/usr/bin/virsh attach-disk i-2-9-VM /dev/drbd/by-res/cs-742678da-c440-4d2b-b63a-5ecbe10ba206/0 vdb --subdriver qcow2 --cache none ] failed. ... encountered the error: [1]. libvirt's own message, when I ran that command by hand (observed): error: Failed to attach disk error: internal error: unable to execute QEMU command 'blockdev-add': Image is not in qcow2 format API: same Failed to attach volume to VM: i-2-9-VM shape as above. ``` After fix: Restore and attach volume works as expected on all 3 NFS, Linstor and Ceph primary storages <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
