You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "rp- (via GitHub)" <gi...@apache.org> on 2023/10/10 13:16:55 UTC
[PR] Linstor backup snaphots [cloudstack]
rp- opened a new pull request, #8067:
URL: https://github.com/apache/cloudstack/pull/8067
### Description
This PR adds an config option for the Linstor primary storage driver, that allows you to automatically backup
volume snapshots to the secondary storage.
Additionally it will not mangle the need `java-linstor` dependency into the client.jar, but instead just copy
the `java-linstor.jar` into `lib`.
Config option is called: `lin.backup.snapshots` and is default `false`
The scope of this change should be limited, as it only touches the Linstor driver and a part of `copyAsync`
was implemented with 2 new Linstor specific commands.
<!-- 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: AUTOMATATION 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)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
- [ ] build/CI
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [x] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### How Has This Been Tested?
Tested on 2 different Linstor clusters, one running LVM the other ZFS.
Additionally send this PR to a PoC tester, who requested this feature.
#### 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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1786858731
@rp- lets run a round of smoke test. If all good and no objections we can go ahead and merge
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1803131638
Tests look good now. One failure is unrelated. GH failures are codecov related. Thanks @rp- for fixing. Merging this.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1763840463
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1788682762
@shwstppr can we merge this?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1772528410
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1755476953
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8067?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#8067](https://app.codecov.io/gh/apache/cloudstack/pull/8067?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (8c28e64) into [main](https://app.codecov.io/gh/apache/cloudstack/commit/7e998f922830ab0516837a133e0ce4aaf736b006?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (7e998f9) will **decrease** coverage by `24.35%`.
> Report is 5 commits behind head on main.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## main #8067 +/- ##
============================================
- Coverage 29.19% 4.84% -24.35%
============================================
Files 5101 343 -4758
Lines 358695 25818 -332877
Branches 52346 4446 -47900
============================================
- Hits 104721 1252 -103469
+ Misses 239602 24431 -215171
+ Partials 14372 135 -14237
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8067/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8067/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8067/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.84% <ø> (ø)` | |
| [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8067/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
[see 4758 files with indirect coverage changes](https://app.codecov.io/gh/apache/cloudstack/pull/8067/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1801508350
@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1768101426
> I just found a bug, with non hyperconverged setups. So I'll have to fix that before we can continue this merge here Sorry
np, I converted the PR to draft
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1763839454
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1798215895
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1798215047
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1771104658
Ok, bug is fixed and tested.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1787468494
@blueorangutan test matrix
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1788423781
<b>[SF] Trillian test result (tid-8185)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 47566 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8067-t8185-vmware-67u3.zip
Smoke tests completed. 114 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_balanced_drs_algorithm | `Failure` | 127.08 | test_cluster_drs.py
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1772538869
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1761551275
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1766234247
@rp- PR is currently showing some extra changes in Files changed. Can you please check and rebase for easier review?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#discussion_r1366807155
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -254,6 +256,7 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu
throw new CloudRuntimeException("Linstor: viewResources didn't return resources or volumes.");
}
} catch (ApiException apiEx) {
+ s_logger.error("Linstor.createPhysicalDisk: ApiException: " + apiEx.getBestMessage());
Review Comment:
String.format?
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -214,6 +215,7 @@ private void makeResourceAvailable(DevelopersApi api, String rscName, boolean di
public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, QemuImg.PhysicalDiskFormat format,
Storage.ProvisioningType provisioningType, long size, byte[] passphrase)
{
+ s_logger.debug("Linstor.createPhysicalDisk: " + name + ";" + format);
Review Comment:
use `String.format()`?
```suggestion
s_logger.debug(String.format("Linstor.createPhysicalDisk: %s;%s", name, format));
```
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1788750214
@DaanHoogland a [SL] Trillian-Jenkins test job (alma9 mgmt + kvm-alma9) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1786860030
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790515076
<b>[LL] Trillian Build Failed (tid-6884)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790517906
Where could I check what is failing? Or is this internal shapeblue infrastructure testing?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1798005651
> if you can't provide packages, could you check if `java-linstor-0.3.0.jar` is included in the `cloudstack-agent/lib` ? or maybe it is possible to get more debug output, why it gets stuck on loading the LinstorStorageAdaptor?
@rp- sorry I could get time to upload packages publicly.
I checked in the env and lib was missing in `cloudstack-agent/lib`. Is it because the dependency is removed from `plugins/hypervisors/kvm/pom.xml`? Would it be better to add the dependency in another module that is present in cloudstack-agent?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790303777
@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790385063
<b>[SF] Trillian Build Failed (tid-8211)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1793366251
@rp- It seems to be failing for all OS we test for KVM - centos7, el8, el9. Which packages would you need? l can't promise but will try
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1801503879
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1759774397
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7339
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1759638437
@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1761550090
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1761638520
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7355
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on code in PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#discussion_r1358108775
##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -690,24 +770,173 @@ public void revertSnapshot(
}
}
+ private static boolean canCopyTemplateCond(DataObject srcData, DataObject dstData) {
+ return srcData.getType() == DataObjectType.TEMPLATE && dstData.getType() == DataObjectType.TEMPLATE
+ && dstData.getDataStore().getRole() == DataStoreRole.Primary
+ && (srcData.getDataStore().getRole() == DataStoreRole.Image
+ || srcData.getDataStore().getRole() == DataStoreRole.ImageCache);
+ }
+
@Override
- public boolean canCopy(DataObject srcData, DataObject destData)
+ public boolean canCopy(DataObject srcData, DataObject dstData)
{
+ s_logger.debug("LinstorPrimaryDataStoreDriverImpl.canCopy: " + srcData.getType() + " -> " + dstData.getType());
+
+ if (srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT
+ && (dstData.getDataStore().getRole() == DataStoreRole.Image
+ || dstData.getDataStore().getRole() == DataStoreRole.ImageCache)) {
+ SnapshotInfo sinfo = (SnapshotInfo) srcData;
+ VolumeInfo volume = sinfo.getBaseVolume();
+ StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId());
+ return storagePool.getStorageProviderName().equals(LinstorUtil.PROVIDER_NAME);
+ } else if (canCopyTemplateCond(srcData, dstData)) {
+ StoragePoolVO storagePoolVO = _storagePoolDao.findById(dstData.getDataStore().getId());
+ return storagePoolVO != null && storagePoolVO.getPoolType() == Storage.StoragePoolType.Linstor;
+ }
return false;
}
@Override
- public void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback<CopyCommandResult> callback)
+ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCallback<CopyCommandResult> callback)
{
- // as long as canCopy is false, this isn't called
- s_logger.debug("Linstor: copyAsync with srcdata: " + srcData.getUuid());
+ s_logger.debug("LinstorPrimaryDataStoreDriverImpl.copyAsync: "
+ + srcData.getType() + " -> " + dstData.getType());
+
+ final CopyCommandResult res;
+ if (srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT
+ && (dstData.getDataStore().getRole() == DataStoreRole.Image
+ || dstData.getDataStore().getRole() == DataStoreRole.ImageCache)) {
Review Comment:
didn't want to touch that part again, as you already reviewed it, but I can extract into a method
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1761539236
@DaanHoogland done, thanks for the review
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1766264869
> @rp- PR is currently showing some extra changes in Files changed. Can you please check and rebase for easier review?
@shwstppr done
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland closed pull request #8067: Linstor backup snaphots
URL: https://github.com/apache/cloudstack/pull/8067
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1793538420
usually I test on ubuntu.
But I would be happy for any packages, to check whats wrong.
because on my end it works of course...
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790302834
@blueorangutan test
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1798076938
@rp- Adding dependency in `cloud-plugin-storage-volume-linstor` makes sense as it is present in cloudstack-agent. cc @DaanHoogland @rohityadavcloud
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1794945766
if you can't provide packages, could you check if `java-linstor-0.3.0.jar` is included in the `cloudstack-agent/lib` ?
or maybe it is possible to get more debug output, why it gets stuck on loading the LinstorStorageAdaptor?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1767939266
I just found a bug, with non hyperconverged setup.
So I'll have to fix that before we can continue this merge here
Sorry
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1802933773
<b>[SF] Trillian test result (tid-8271)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48291 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8067-t8271-kvm-centos7.zip
Smoke tests completed. 116 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_08_migrate_vm | `Error` | 44.85 | test_vm_life_cycle.py
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1772410992
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1789169192
<b>[SF] Trillian Build Failed (tid-8207)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1761034557
@DaanHoogland I have another small feature, that depends on this merged.
Should I add that to this PR? or wait and create another PR?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1761235861
> @DaanHoogland I have another small feature, that depends on this merged. Should I add that to this PR? or wait and create another PR?
your choice @rp- , if it is reasonably risk free, I'd add it. else you could wait.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1761270065
I have added the commit, thanks
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1766360168
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7401
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1798034038
@shwstppr where are the dependencies for `cloudstack-agent` declared?
Seems not in agent/pom.xml
Sorry I'm not really familiar with Maven, we use gradle on our end.
agent should than probably have a dependency to the `cloud-plugin-storage-volume-linstor` because that is where the `LinstorStorageAdapter` is located, or maybe the `kvm-hyporvisor-plugin` should have a dep to it?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1798172001
@rp- like Daan has advised, you could configure mvn/pom.xml rule to exclude the specific dependency you don't it to shade but ensure it copies the dependency in the lib directory (in the debian and rpm/spec build config file).
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1798194371
@shwstppr I think I got it right now
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1766269863
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1766711455
@blueorangutan LLtest
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790788343
@shwstppr What OS? and is there a chance to get your build packages for that?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1772411661
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1772507227
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7448
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#discussion_r1352554473
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/api/storage/LinstorRevertBackupSnapshotCommand.java:
##########
@@ -0,0 +1,12 @@
+package com.cloud.api.storage;
Review Comment:
license
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/api/storage/LinstorBackupSnapshotCommand.java:
##########
@@ -0,0 +1,12 @@
+package com.cloud.api.storage;
Review Comment:
license needed
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java:
##########
@@ -0,0 +1,128 @@
+package com.cloud.hypervisor.kvm.resource.wrapper;
Review Comment:
license
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorRevertBackupSnapshotCommandWrapper.java:
##########
@@ -0,0 +1,73 @@
+package com.cloud.hypervisor.kvm.resource.wrapper;
Review Comment:
license
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorRevertBackupSnapshotCommandWrapper.java:
##########
@@ -0,0 +1,73 @@
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import java.io.File;
+
+import com.cloud.agent.api.to.DataStoreTO;
+import com.cloud.api.storage.LinstorRevertBackupSnapshotCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.storage.Storage;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.apache.log4j.Logger;
+
+@ResourceWrapper(handles = LinstorRevertBackupSnapshotCommand.class)
+public final class LinstorRevertBackupSnapshotCommandWrapper
+ extends CommandWrapper<LinstorRevertBackupSnapshotCommand, CopyCmdAnswer, LibvirtComputingResource>
+{
+ private static final Logger s_logger = Logger.getLogger(LinstorRevertBackupSnapshotCommandWrapper.class);
+
+ @Override
+ public CopyCmdAnswer execute(LinstorRevertBackupSnapshotCommand cmd, LibvirtComputingResource serverResource)
+ {
+ s_logger.debug("LinstorRevertBackupSnapshotCommandWrapper: " + cmd.getSrcTO().getPath() + " -> " + cmd.getDestTO().getPath());
+ final SnapshotObjectTO src = (SnapshotObjectTO) cmd.getSrcTO();
+ final VolumeObjectTO dst = (VolumeObjectTO) cmd.getDestTO();
+ KVMStoragePool secondaryPool = null;
+ final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
+ KVMStoragePool linstorPool = storagePoolMgr.getStoragePool(Storage.StoragePoolType.Linstor, dst.getDataStore().getUuid());
+
+ if (linstorPool == null) {
+ return new CopyCmdAnswer("Unable to get linstor storage pool from destination volume.");
+ }
+
+ try
+ {
+ final DataStoreTO srcDataStore = src.getDataStore();
+ File srcFile = new File(src.getPath());
+ secondaryPool = storagePoolMgr.getStoragePoolByURI(
+ srcDataStore.getUrl() + File.separator + srcFile.getParent());
+
+ final QemuImgFile srcQemuFile = new QemuImgFile(
+ secondaryPool.getLocalPath() + File.separator + srcFile.getName(), QemuImg.PhysicalDiskFormat.QCOW2);
+ final QemuImg qemu = new QemuImg(cmd.getWaitInMillSeconds());
+ final QemuImgFile dstFile = new QemuImgFile(
+ linstorPool.getPhysicalDisk(dst.getPath()).getPath(),
+ QemuImg.PhysicalDiskFormat.RAW);
+ qemu.convert(srcQemuFile, dstFile);
+
+ final VolumeObjectTO dstVolume = new VolumeObjectTO();
+ dstVolume.setPath(dst.getPath());
+ return new CopyCmdAnswer(dstVolume);
+ } catch (final Exception e) {
+ final String error = String.format("Failed to revert snapshot with id [%s] with a pool %s, due to %s",
+ cmd.getSrcTO().getId(), cmd.getSrcTO().getDataStore().getUuid(), e.getMessage());
+ s_logger.error(error);
+ return new CopyCmdAnswer(cmd, e);
+ } finally {
+ if (secondaryPool != null) {
+ try {
+ secondaryPool.delete();
+ } catch (final Exception e) {
+ s_logger.debug("Failed to delete secondary storage", e);
+ }
+ }
Review Comment:
looks like a section from the prior cmd wrapper, mayby unify in a util method?
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java:
##########
@@ -0,0 +1,128 @@
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import java.io.File;
+
+import com.cloud.agent.api.to.DataStoreTO;
+import com.cloud.agent.api.to.NfsTO;
+import com.cloud.api.storage.LinstorBackupSnapshotCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.storage.Storage;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.apache.commons.io.FileUtils;
+import org.apache.log4j.Logger;
+import org.joda.time.Duration;
+
+@ResourceWrapper(handles = LinstorBackupSnapshotCommand.class)
+public final class LinstorBackupSnapshotCommandWrapper
+ extends CommandWrapper<LinstorBackupSnapshotCommand, CopyCmdAnswer, LibvirtComputingResource>
+{
+ private static final Logger s_logger = Logger.getLogger(LinstorBackupSnapshotCommandWrapper.class);
+
+ private String zfsSnapdev(boolean hide, String zfsUrl) {
+ Script script = new Script("/usr/bin/zfs", Duration.millis(5000));
+ script.add("set");
+ script.add("snapdev=" + (hide ? "hidden" : "visible"));
+ script.add(zfsUrl.substring(6)); // cutting zfs://
+ return script.execute();
+ }
+
+ private String qemuShrink(String path, long sizeByte, long timeout) {
+ Script qemuImg = new Script("qemu-img", Duration.millis(timeout));
+ qemuImg.add("resize");
+ qemuImg.add("--shrink");
+ qemuImg.add(path);
+ qemuImg.add("" + sizeByte);
+ return qemuImg.execute();
+ }
+
+ @Override
+ public CopyCmdAnswer execute(LinstorBackupSnapshotCommand cmd, LibvirtComputingResource serverResource)
Review Comment:
can you disect this a bit to make it more readable (for instance extract the init section, try - and finally clauses?
##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -668,15 +688,39 @@ public void revertSnapshot(
return;
}
- String resultMsg;
+ final StoragePool pool = (StoragePool) volumeInfo.getDataStore();
+ final DevelopersApi linstorApi = LinstorUtil.getLinstorAPI(pool.getHostAddress());
+ final String rscName = LinstorUtil.RSC_PREFIX + volumeInfo.getUuid();
+ String resultMsg = null;
try {
- final StoragePool pool = (StoragePool) snapshot.getDataStore();
- final String rscName = LinstorUtil.RSC_PREFIX + volumeInfo.getUuid();
- final String snapName = LinstorUtil.RSC_PREFIX + snapshot.getUuid();
- final DevelopersApi linstorApi = LinstorUtil.getLinstorAPI(pool.getHostAddress());
-
- ApiCallRcList answers = linstorApi.resourceSnapshotRollback(rscName, snapName);
- resultMsg = checkLinstorAnswers(answers);
+ if (snapshot.getDataStore().getRole() == DataStoreRole.Primary) {
+ final String snapName = LinstorUtil.RSC_PREFIX + snapshot.getUuid();
+
+ ApiCallRcList answers = linstorApi.resourceSnapshotRollback(rscName, snapName);
+ resultMsg = checkLinstorAnswers(answers);
+ } else if (snapshot.getDataStore().getRole() == DataStoreRole.Image) {
+ String value = _configDao.getValue(Config.BackupSnapshotWait.toString());
+ int _backupsnapshotwait = NumbersUtil.parseInt(
+ value, Integer.parseInt(Config.BackupSnapshotWait.getDefaultValue()));
+
+ LinstorRevertBackupSnapshotCommand cmd = new LinstorRevertBackupSnapshotCommand(
+ snapshot.getTO(),
+ volumeInfo.getTO(),
+ _backupsnapshotwait,
+ VirtualMachineManager.ExecuteInSequence.value());
+
+ Optional<RemoteHostEndPoint> optEP = getDiskfullEP(linstorApi, rscName);
+ if (optEP.isPresent()) {
+ Answer answer = optEP.get().sendMessage(cmd);
+ if (!answer.getResult()) {
+ resultMsg = answer.getDetails();
+ }
+ } else {
+ resultMsg = "Unable to get matching Linstor endpoint.";
+ }
+ } else {
+ resultMsg = "Linstor: Snapshot revert datastore not supported";
+ }
Review Comment:
can this go in (a) separate method(s)?
##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorConfigurationManager.java:
##########
@@ -0,0 +1,24 @@
+package org.apache.cloudstack.storage.datastore.util;
Review Comment:
license
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1788737280
@blueorangutan test alma9 kvm-alma9
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1786977209
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7572
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1787472643
@DaanHoogland a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1786854543
is anything still holding this back?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#discussion_r1358106840
##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -690,24 +770,173 @@ public void revertSnapshot(
}
}
+ private static boolean canCopyTemplateCond(DataObject srcData, DataObject dstData) {
+ return srcData.getType() == DataObjectType.TEMPLATE && dstData.getType() == DataObjectType.TEMPLATE
+ && dstData.getDataStore().getRole() == DataStoreRole.Primary
+ && (srcData.getDataStore().getRole() == DataStoreRole.Image
+ || srcData.getDataStore().getRole() == DataStoreRole.ImageCache);
+ }
+
@Override
- public boolean canCopy(DataObject srcData, DataObject destData)
+ public boolean canCopy(DataObject srcData, DataObject dstData)
{
+ s_logger.debug("LinstorPrimaryDataStoreDriverImpl.canCopy: " + srcData.getType() + " -> " + dstData.getType());
+
+ if (srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT
+ && (dstData.getDataStore().getRole() == DataStoreRole.Image
+ || dstData.getDataStore().getRole() == DataStoreRole.ImageCache)) {
+ SnapshotInfo sinfo = (SnapshotInfo) srcData;
+ VolumeInfo volume = sinfo.getBaseVolume();
+ StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId());
+ return storagePool.getStorageProviderName().equals(LinstorUtil.PROVIDER_NAME);
+ } else if (canCopyTemplateCond(srcData, dstData)) {
+ StoragePoolVO storagePoolVO = _storagePoolDao.findById(dstData.getDataStore().getId());
+ return storagePoolVO != null && storagePoolVO.getPoolType() == Storage.StoragePoolType.Linstor;
+ }
return false;
}
@Override
- public void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback<CopyCommandResult> callback)
+ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCallback<CopyCommandResult> callback)
{
- // as long as canCopy is false, this isn't called
- s_logger.debug("Linstor: copyAsync with srcdata: " + srcData.getUuid());
+ s_logger.debug("LinstorPrimaryDataStoreDriverImpl.copyAsync: "
+ + srcData.getType() + " -> " + dstData.getType());
+
+ final CopyCommandResult res;
+ if (srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT
+ && (dstData.getDataStore().getRole() == DataStoreRole.Image
+ || dstData.getDataStore().getRole() == DataStoreRole.ImageCache)) {
Review Comment:
this is a complex condition, maybe make it a method? `is<something>Needed()`?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#discussion_r1358108564
##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -690,24 +770,173 @@ public void revertSnapshot(
}
}
+ private static boolean canCopyTemplateCond(DataObject srcData, DataObject dstData) {
+ return srcData.getType() == DataObjectType.TEMPLATE && dstData.getType() == DataObjectType.TEMPLATE
+ && dstData.getDataStore().getRole() == DataStoreRole.Primary
+ && (srcData.getDataStore().getRole() == DataStoreRole.Image
+ || srcData.getDataStore().getRole() == DataStoreRole.ImageCache);
+ }
+
@Override
- public boolean canCopy(DataObject srcData, DataObject destData)
+ public boolean canCopy(DataObject srcData, DataObject dstData)
{
+ s_logger.debug("LinstorPrimaryDataStoreDriverImpl.canCopy: " + srcData.getType() + " -> " + dstData.getType());
+
+ if (srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT
+ && (dstData.getDataStore().getRole() == DataStoreRole.Image
+ || dstData.getDataStore().getRole() == DataStoreRole.ImageCache)) {
Review Comment:
same condition as below? (same method)
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1766713630
@shwstppr a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790443966
@DaanHoogland a [LL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790441075
@blueorangutan LLtest
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790584448
@rp- this is for KVM hypervisor, Advanced zone, trying to add 2xKVM hosts. Trillian is not reaching to the point to run any smoke tests on KVM env
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790549514
@rp- it is an internal one. I tried to check the failures. I'm unable to add the KVM host to the env and on the host agent gets stuck at,
```
2023-11-02 11:20:31,054 DEBUG [utils.net.NetUtils] (main:null) (logid:) Retrieving params of address [/10.0.34.196/20 [/10.0.47.255]] of NIC [name:cloudbr0 (cloudbr0)].
2023-11-02 11:20:31,056 DEBUG [agent.properties.AgentPropertiesFileHandler] (main:null) (logid:) Property [heartbeat.update.timeout] has empty or null value. Using default value [60000].
2023-11-02 11:20:31,057 DEBUG [agent.properties.AgentPropertiesFileHandler] (main:null) (logid:) Property [reboot.host.and.alert.management.on.heartbeat.timeout] has empty or null value. Using default value [true].
2023-11-02 11:20:31,067 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in the classpath
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) System resource: null
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) Classpath resource: null
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) Current binaries reside at /usr/share/cloudstack-agent/lib
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in /usr/share/cloudstack-agent/lib/scripts/storage/qcow2/managesnapshot.sh
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in /usr/share/cloudstack-agent/lib/scripts/storage/managesnapshot.sh
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in /usr/share/cloudstack-agent/lib/scripts/managesnapshot.sh
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in /usr/share/cloudstack-agent/lib/managesnapshot.sh
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in /usr/share/cloudstack-agent/managesnapshot.sh
2023-11-02 11:20:31,068 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in /usr/share/managesnapshot.sh
2023-11-02 11:20:31,069 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in /usr/managesnapshot.sh
2023-11-02 11:20:31,069 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in /managesnapshot.sh
2023-11-02 11:20:31,069 DEBUG [utils.script.Script] (main:null) (logid:) Searching in environment.properties
2023-11-02 11:20:31,069 DEBUG [utils.script.Script] (main:null) (logid:) environment.properties says scripts should be in /usr/share/cloudstack-common
2023-11-02 11:20:31,069 DEBUG [utils.script.Script] (main:null) (logid:) Looking for managesnapshot.sh in /usr/share/cloudstack-common/scripts/storage/qcow2/managesnapshot.sh
2023-11-02 11:20:31,178 INFO [kvm.storage.KVMStoragePoolManager] (main:null) (logid:) adding storage adaptor for com.cloud.hypervisor.kvm.storage.LinstorStorageAdaptor
```
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "rp- (via GitHub)" <gi...@apache.org>.
rp- commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1790558755
@shwstppr is this for all tests or just for one?
anyway :man_shrugging:
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1801368637
@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr merged PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1759636544
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#discussion_r1358140371
##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -690,24 +770,173 @@ public void revertSnapshot(
}
}
+ private static boolean canCopyTemplateCond(DataObject srcData, DataObject dstData) {
+ return srcData.getType() == DataObjectType.TEMPLATE && dstData.getType() == DataObjectType.TEMPLATE
+ && dstData.getDataStore().getRole() == DataStoreRole.Primary
+ && (srcData.getDataStore().getRole() == DataStoreRole.Image
+ || srcData.getDataStore().getRole() == DataStoreRole.ImageCache);
+ }
+
@Override
- public boolean canCopy(DataObject srcData, DataObject destData)
+ public boolean canCopy(DataObject srcData, DataObject dstData)
{
+ s_logger.debug("LinstorPrimaryDataStoreDriverImpl.canCopy: " + srcData.getType() + " -> " + dstData.getType());
+
+ if (srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT
+ && (dstData.getDataStore().getRole() == DataStoreRole.Image
+ || dstData.getDataStore().getRole() == DataStoreRole.ImageCache)) {
+ SnapshotInfo sinfo = (SnapshotInfo) srcData;
+ VolumeInfo volume = sinfo.getBaseVolume();
+ StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId());
+ return storagePool.getStorageProviderName().equals(LinstorUtil.PROVIDER_NAME);
+ } else if (canCopyTemplateCond(srcData, dstData)) {
+ StoragePoolVO storagePoolVO = _storagePoolDao.findById(dstData.getDataStore().getId());
+ return storagePoolVO != null && storagePoolVO.getPoolType() == Storage.StoragePoolType.Linstor;
+ }
return false;
}
@Override
- public void copyAsync(DataObject srcData, DataObject destData, AsyncCompletionCallback<CopyCommandResult> callback)
+ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCallback<CopyCommandResult> callback)
{
- // as long as canCopy is false, this isn't called
- s_logger.debug("Linstor: copyAsync with srcdata: " + srcData.getUuid());
+ s_logger.debug("LinstorPrimaryDataStoreDriverImpl.copyAsync: "
+ + srcData.getType() + " -> " + dstData.getType());
+
+ final CopyCommandResult res;
+ if (srcData.getType() == DataObjectType.SNAPSHOT && dstData.getType() == DataObjectType.SNAPSHOT
+ && (dstData.getDataStore().getRole() == DataStoreRole.Image
+ || dstData.getDataStore().getRole() == DataStoreRole.ImageCache)) {
Review Comment:
sorry, I missed it (and am not perfect) I think it will help readability.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#discussion_r1358109010
##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -821,12 +1050,21 @@ public void takeSnapshot(SnapshotInfo snapshotInfo, AsyncCompletionCallback<Crea
result.setResult(errMsg);
} else
{
- s_logger.info(String.format("Successfully took snapshot from %s", rscName));
+ s_logger.info(String.format("Successfully took snapshot %s from %s", snapshot.getName(), rscName));
+
+ SnapshotObjectTO snapshotTO = (SnapshotObjectTO)snapshotInfo.getTO();
+ com.linbit.linstor.api.model.StoragePool linStoragePool = LinstorUtil.getDiskfulStoragePool(api, rscName);
+ if (linStoragePool == null) {
+ throw new CloudRuntimeException("Linstor: Unable to find storage pool for resource " + rscName);
+ }
- SnapshotObjectTO snapshotObjectTo = (SnapshotObjectTO)snapshotInfo.getTO();
- snapshotObjectTo.setPath(rscName + "-" + snapshotInfo.getName());
+ final String path = LinstorUtil.getSnapshotPath(linStoragePool, rscName, LinstorUtil.RSC_PREFIX + snapshotInfo.getUuid());
+ snapshotTO.setPath(path);
+ SnapshotDetailsVO details = new SnapshotDetailsVO(
+ snapshotInfo.getId(), snapshotInfo.getUuid(), path, false);
+ _snapshotDetailsDao.persist(details);
- result = new CreateCmdResult(null, new CreateObjectAnswer(snapshotObjectTo));
+ result = new CreateCmdResult(null, new CreateObjectAnswer(snapshotTO));
result.setResult(null);
Review Comment:
can you extract this?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1763907838
<b>[SF] Trillian Build Failed (tid-7976)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1801364134
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1801497250
Packaging result [SF]: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: el9 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 7672
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1798077620
> @shwstppr where are the dependencies for `cloudstack-agent` declared? Seems not in agent/pom.xml Sorry I'm not really familiar with Maven, we use gradle on our end.
>
> agent should than probably have a dependency to the `cloud-plugin-storage-volume-linstor` because that is where the `LinstorStorageAdapter` is located, or maybe the `kvm-hyporvisor-plugin` should have a dep to it?
@rp- , sounds like you need to add it in the cloud.spec files. but I already see a line
```
cp plugins/storage/volume/linstor/target/*.jar ${RPM_BUILD_ROOT}%{_datadir}/%{name}-agent/lib
```
there. Maybe something else is needed? for the non acs parts of linstore?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1766272294
@shwstppr a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1766808022
<b>[LL] Trillian Build Failed (tid-6845)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1794256837
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "shwstppr (via GitHub)" <gi...@apache.org>.
shwstppr commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1788697206
> @shwstppr can we merge this?
@DaanHoogland looks good but should we wait/check for KVM smoke test results? I realized they are missing from the last run
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1788420337
<b>[SF] Trillian test result (tid-8184)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47362 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8067-t8184-xenserver-71.zip
Smoke tests completed. 115 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Linstor backup snaphots [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#issuecomment-1789771504
<b>[SF] Trillian Build Failed (tid-8210)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org