You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "harikrishna-patnala (via GitHub)" <gi...@apache.org> on 2024/01/30 13:18:04 UTC
[PR] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
harikrishna-patnala opened a new pull request, #8577:
URL: https://github.com/apache/cloudstack/pull/8577
### Description
This PR introduces a new API "checkVolume" that allows users or admins to check and repair if any leaks observed. Currently this is supported only for KVM
There are few cases when VMs shutdown uncleanly, particularly those using qcow2, they can leak clusters. This may sometimes lead to volumes taking up much more space than they are supposed to. When we use qcow2 format to thin provision, and the volume size is pretty close to the actual formatted size, leaked clusters can run us out of space, so we need a way to check/repair.
To address this, we have introduced a new API "checkVolume" API which takes parameters volume id and repair (possible values are leaks/all)
API name: checkVolume
Parameters:
- id : volume ID
- repair : parameter to repair the volume, leaks or all are the possible values
There is also option to repair the volume during VM start or while attaching the volume to VM. Introduced a new boolean global setting
volume.check.and.repair.leaks.before.use with a default false.
STEPS TO REPRODUCE
1. Create a VM on local storage, or NFS storage.
2. attach a data disk
3. run a write benchmark on data disk in guest. e.g.:
` fio --filename=/dev/vdb --direct=1 --rw=randwrite --bsrange=512-4k --ioengine=libaio --iodepth=32 --runtime=120 --numjobs=8 --time_based --group_reporting --name=iops-test-job --norandommap`
4. immediately kill the VM (from host try virsh shutdown or "kill -9" of qemu process
5. run a check on the underlying qcow2 file, observe "leaks" count
```
# qemu-img check /var/lib/libvirt/images/26be20c7-b9d0-43f6-a76e-16c70737a0e0 --output=json 2>/dev/null
{
"image-end-offset": 6442582016,
"total-clusters": 163840,
"check-errors": 0,
"leaks": 124,
"allocated-clusters": 98154,
"filename": "/var/lib/libvirt/images/26be20c7-b9d0-43f6-a76e-16c70737a0e0",
"format": "qcow2",
"fragmented-clusters": 96135
}
```
6. repair leaks
```
# qemu-img check /var/lib/libvirt/images/26be20c7-b9d0-43f6-a76e-16c70737a0e0 --output=json -r leaks 2>/dev/null
{
"image-end-offset": 6442582016,
"total-clusters": 163840,
"check-errors": 0,
"leaks-fixed": 124,
"allocated-clusters": 98154,
"filename": "/var/lib/libvirt/images/26be20c7-b9d0-43f6-a76e-16c70737a0e0",
"format": "qcow2",
"fragmented-clusters": 96135
}
```
<!--- 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)
- [ ] 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
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [ ] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
(localcloud) 🐱 > check volume id=55937826-2f08-414a-9eef-4c6b7d6fd3b1
{
.
.
"volumecheckresult": {
"allocated-clusters": "110",
"check-errors": "0",
"leaks": 73,
"filename": "/mnt/e72364b6-eab0-369f-af0b-2ec8bed9d8ac/55937826-2f08-414a-9eef-4c6b7d6fd3b1",
"format": "qcow2",
"fragmented-clusters": "32",
"image-end-offset": "7995392",
"total-clusters": "131072"
},
(localcloud) 🐱 > check volume id=55937826-2f08-414a-9eef-4c6b7d6fd3b1 repair=leaks
{
"volumecheckresult": {
"allocated-clusters": "110",
"check-errors": "0",
"leaks": 73,
"filename": "/mnt/e72364b6-eab0-369f-af0b-2ec8bed9d8ac/55937826-2f08-414a-9eef-4c6b7d6fd3b1",
"format": "qcow2",
"fragmented-clusters": "32",
"image-end-offset": "7995392",
"total-clusters": "131072"
},
"volumerepairresult": {
"allocated-clusters": "110",
"check-errors": "0",
"leaks-fixed": 73,
"filename": "/mnt/e72364b6-eab0-369f-af0b-2ec8bed9d8ac/55937826-2f08-414a-9eef-4c6b7d6fd3b1",
"format": "qcow2",
"fragmented-clusters": "32",
"image-end-offset": "7995392",
"total-clusters": "131072"
},
}
#### 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1965824847
@rohityadavcloud 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1970720951
> I just realised after merging the base branch of the PR is 4.19 and not main, is that an issue - should we revert @harikrishna-patnala @DaanHoogland @kiranchavala ?
It is a new API, but no backwards incompatibility and it is also an improvement. Not ideal, but I think it is fine. cc @sureshanaparti @JoaoJandre
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1921265842
> @harikrishna-patnala I might be missing something, but how will this new API be handled when called in a xen or vmware env? as you state "Currently this is supported only for KVM" I am sure you implemented this somewhere, but all I can find is an implicit error and no graceful message
>
> see https://github.com/apache/cloudstack/pull/8577/files#diff-63d1a7ba0fc6bbd393feffaaf961d192431e8d343280b7cdc8d3817c2d6b7f1cR2801
Kept a check in the validate method @DaanHoogland
https://github.com/shapeblue/cloudstack/blob/353b2f4f0d938f9fab91c42358859a430dcc1c14/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java#L1909-L1911
is this what you are trying to refer ?
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1953575621
Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 8719
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1933929140
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1965824305
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1495502213
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/CheckAndRepairVolumeCmd.java:
##########
@@ -0,0 +1,136 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject.ResponseView;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.storage.Volume;
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
+
+import java.util.Arrays;
+
+@APICommand(name = "checkVolume", description = "Check the volume for any errors or leaks and also repairs when repair parameter is passed, this is currently supported for KVM only", responseObject = VolumeResponse.class, entityType = {Volume.class},
+ since = "4.19.1",
Review Comment:
check version
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1920633245
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1933931405
@harikrishna-patnala 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1916846653
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1956037228
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1955887424
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1495331535
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/CheckAndRepairVolumeCmd.java:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject.ResponseView;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.storage.Volume;
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
+
+import java.util.Arrays;
+
+@APICommand(name = "checkVolume", description = "Check the volume for any errors or leaks and also repairs when repair parameter is passed, this is currently supported for KVM only", responseObject = VolumeResponse.class, entityType = {Volume.class},
+ since = "4.19.1",
+ authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
+public class CheckAndRepairVolumeCmd extends BaseAsyncCmd {
+ public static final Logger s_logger = Logger.getLogger(CheckAndRepairVolumeCmd.class.getName());
Review Comment:
addressed the logger and the messages.
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1960952770
@JoaoJandre is this ok for you now?
@kiranchavala do you have any qa work for this planned?
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1931648089
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1953566418
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1497062189
##########
api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java:
##########
@@ -288,6 +289,14 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co
@Param(description = "volume uuid that is given by virtualisation provider (only for VMware)")
private String externalUuid;
+ @SerializedName(ApiConstants.VOLUME_CHECK_RESULT)
+ @Param(description = "details for the volume check result, they may vary for different hypervisors, since = 4.19.1")
Review Comment:
ok, thanks @harikrishna-patnala
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1921206966
@harikrishna-patnala I might be missing something, but how will this new API be handled when called in a xen or vmware env? as you state "Currently this is supported only for KVM" I am sure you implemented this somewhere, but all I can find is an implicit error and no graceful message
see https://github.com/apache/cloudstack/pull/8577/files#diff-63d1a7ba0fc6bbd393feffaaf961d192431e8d343280b7cdc8d3817c2d6b7f1cR2801
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1916849377
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8577?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`fedcf66`)](https://app.codecov.io/gh/apache/cloudstack/commit/fedcf66de06911c1866353efa8b3a0bad998655a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 30.81% compared to head [(`21378f8`)](https://app.codecov.io/gh/apache/cloudstack/pull/8577?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 4.39%.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8577 +/- ##
============================================
- Coverage 30.81% 4.39% -26.42%
============================================
Files 5341 361 -4980
Lines 375033 28626 -346407
Branches 54554 4993 -49561
============================================
- Hits 115564 1258 -114306
+ Misses 244185 27229 -216956
+ Partials 15284 139 -15145
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8577/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/8577/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/8577/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.39% <ø> (ø)` | |
| [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8577/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.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8577?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1955886541
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1956039282
@harikrishna-patnala 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1926613227
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8517
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1970713730
I just realised after merging the base branch of the PR is 4.19 and not main, is that an issue - should we revert @harikrishna-patnala @DaanHoogland @kiranchavala ?
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud merged PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1487389381
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/CheckAndRepairVolumeCmd.java:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject.ResponseView;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.storage.Volume;
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
+
+import java.util.Arrays;
+
+@APICommand(name = "checkVolume", description = "Check the volume for any errors or leaks and also repairs when repair parameter is passed, this is currently supported for KVM only", responseObject = VolumeResponse.class, entityType = {Volume.class},
+ since = "4.19.1",
+ authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
+public class CheckAndRepairVolumeCmd extends BaseAsyncCmd {
+ public static final Logger s_logger = Logger.getLogger(CheckAndRepairVolumeCmd.class.getName());
Review Comment:
@harikrishna-patnala while you are at it, can you also replace the `String.format()` in logging calls for proper logging formatting expressions?
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1967317556
<b>[SF] Trillian test result (tid-9332)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43396 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8577-t9332-kvm-centos7.zip
Smoke tests completed. 129 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1495500590
##########
api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java:
##########
@@ -288,6 +289,14 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co
@Param(description = "volume uuid that is given by virtualisation provider (only for VMware)")
private String externalUuid;
+ @SerializedName(ApiConstants.VOLUME_CHECK_RESULT)
+ @Param(description = "details for the volume check result, they may vary for different hypervisors, since = 4.19.1")
Review Comment:
@harikrishna-patnala can you confirm the release version, if it's targeted for 4.19.1, rebase with 4.19 branch & update target 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1926462807
@harikrishna-patnala 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1483041689
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/CheckAndRepairVolumeCmd.java:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject.ResponseView;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.storage.Volume;
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
+
+import java.util.Arrays;
+
+@APICommand(name = "checkVolume", description = "Check the volume for any errors or leaks and also repairs when repair parameter is passed, this is currently supported for KVM only", responseObject = VolumeResponse.class, entityType = {Volume.class},
+ since = "4.19.1",
+ authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
+public class CheckAndRepairVolumeCmd extends BaseAsyncCmd {
+ public static final Logger s_logger = Logger.getLogger(CheckAndRepairVolumeCmd.class.getName());
Review Comment:
You don't need to create loggers for each new command anymore, they will inherit it from the BaseCmd because of #7131,
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1495500590
##########
api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java:
##########
@@ -288,6 +289,14 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co
@Param(description = "volume uuid that is given by virtualisation provider (only for VMware)")
private String externalUuid;
+ @SerializedName(ApiConstants.VOLUME_CHECK_RESULT)
+ @Param(description = "details for the volume check result, they may vary for different hypervisors, since = 4.19.1")
Review Comment:
@harikrishna-patnala can you confirm the release version, if it's targeted for 4.19.1, rebase with 4.19 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1496907328
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/CheckAndRepairVolumeCmd.java:
##########
@@ -0,0 +1,136 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject.ResponseView;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.storage.Volume;
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
+
+import java.util.Arrays;
+
+@APICommand(name = "checkVolume", description = "Check the volume for any errors or leaks and also repairs when repair parameter is passed, this is currently supported for KVM only", responseObject = VolumeResponse.class, entityType = {Volume.class},
+ since = "4.19.1",
Review Comment:
rebased the branch to 4.19
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1955887720
@harikrishna-patnala 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1953600643
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1961269766
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8753
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1958839731
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8738
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1958778280
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1953567207
@rohityadavcloud 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1926450450
@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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1471672209
##########
engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java:
##########
@@ -2760,6 +2765,47 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) {
return snapshot;
}
+ @Override
+ public void checkAndRepairVolumeBasedOnConfig(DataObject dataObject, Host host) {
+ if (HypervisorType.KVM.equals(host.getHypervisorType()) && DataObjectType.VOLUME.equals(dataObject.getType())) {
+ if (com.cloud.storage.VolumeApiServiceImpl.AllowCheckAndRepairVolume.value()) {
Review Comment:
Why not import this?
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/CheckAndRepairVolumeCmd.java:
##########
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.exception.InvalidParameterValueException;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject.ResponseView;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.storage.Volume;
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
+
+import java.util.Arrays;
+
+@APICommand(name = "checkVolume", description = "Check the volume for any errors or leaks and also repairs when repair parameter is passed, this is currently supported for KVM only", responseObject = VolumeResponse.class, entityType = {Volume.class},
+ since = "4.19.1",
+ authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
+public class CheckAndRepairVolumeCmd extends BaseCmd {
+ public static final Logger s_logger = Logger.getLogger(CheckAndRepairVolumeCmd.class.getName());
+
+ private static final String s_name = "checkandrepairvolumeresponse";
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VolumeResponse.class, required = true, description = "The ID of the volume")
+ private Long id;
+
+ @Parameter(name = ApiConstants.REPAIR, type = CommandType.STRING, required = false, description = "parameter to repair the volume, leaks or all are the possible values")
+ private String repair;
+
+ /////////////////////////////////////////////////////
+ /////////////////// Accessors ///////////////////////
+ /////////////////////////////////////////////////////
+
+ public enum RepairValues {
+ LEAKS, ALL
+ }
+
+ public Long getId() {
+ return id;
+ }
+
+ public String getRepair() {
+ if (org.apache.commons.lang3.StringUtils.isNotEmpty(repair)) {
Review Comment:
Why not import StringUtils?
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1817,7 +1822,147 @@ public void publishVolumeCreationUsageEvent(Volume volume) {
s_logger.debug(String.format("Volume [%s] has been successfully recovered, thus a new usage event %s has been published.", volume.getUuid(), EventTypes.EVENT_VOLUME_CREATE));
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_VOLUME_CHECK, eventDescription = "checking volume and repair if needed", async = true)
+ public Pair<String, String> checkAndRepairVolume(CheckAndRepairVolumeCmd cmd) throws ResourceAllocationException {
+ long volumeId = cmd.getId();
+ String repair = cmd.getRepair();
+
+ final VolumeVO volume = _volsDao.findById(volumeId);
+ validationsForCheckVolumeOperation(volume);
+
+ Long vmId = volume.getInstanceId();
+ if (vmId != null) {
+ // serialize VM operation
+ AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
+ if (jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {
+ // avoid re-entrance
+
+ VmWorkJobVO placeHolder = null;
+ placeHolder = createPlaceHolderWork(vmId);
+ try {
+ Pair<String, String> result = orchestrateCheckAndRepairVolume(volumeId, repair);
+ return result;
+ } finally {
+ _workJobDao.expunge(placeHolder.getId());
+ }
+ } else {
+ Outcome<Pair> outcome = checkAndRepairVolumeThroughJobQueue(vmId, volumeId, repair);
+
+ try {
+ outcome.get();
+ } catch (InterruptedException e) {
+ throw new RuntimeException("Operation is interrupted", e);
+ } catch (java.util.concurrent.ExecutionException e) {
Review Comment:
Why not import this?
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1817,7 +1822,147 @@ public void publishVolumeCreationUsageEvent(Volume volume) {
s_logger.debug(String.format("Volume [%s] has been successfully recovered, thus a new usage event %s has been published.", volume.getUuid(), EventTypes.EVENT_VOLUME_CREATE));
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_VOLUME_CHECK, eventDescription = "checking volume and repair if needed", async = true)
+ public Pair<String, String> checkAndRepairVolume(CheckAndRepairVolumeCmd cmd) throws ResourceAllocationException {
+ long volumeId = cmd.getId();
+ String repair = cmd.getRepair();
+
+ final VolumeVO volume = _volsDao.findById(volumeId);
+ validationsForCheckVolumeOperation(volume);
+
+ Long vmId = volume.getInstanceId();
+ if (vmId != null) {
+ // serialize VM operation
+ AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
+ if (jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {
+ // avoid re-entrance
+
+ VmWorkJobVO placeHolder = null;
+ placeHolder = createPlaceHolderWork(vmId);
+ try {
+ Pair<String, String> result = orchestrateCheckAndRepairVolume(volumeId, repair);
+ return result;
+ } finally {
+ _workJobDao.expunge(placeHolder.getId());
+ }
+ } else {
+ Outcome<Pair> outcome = checkAndRepairVolumeThroughJobQueue(vmId, volumeId, repair);
+
+ try {
+ outcome.get();
+ } catch (InterruptedException e) {
+ throw new RuntimeException("Operation is interrupted", e);
+ } catch (java.util.concurrent.ExecutionException e) {
+ throw new RuntimeException("Execution exception--", e);
+ }
+
+ Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
+ if (jobResult != null) {
+ if (jobResult instanceof ConcurrentOperationException) {
+ throw (ConcurrentOperationException)jobResult;
+ } else if (jobResult instanceof ResourceAllocationException) {
+ throw (ResourceAllocationException)jobResult;
+ } else if (jobResult instanceof Throwable) {
+ throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
+ }
+ }
+
+ // retrieve the entity url from job result
+ if (jobResult != null && jobResult instanceof Pair) {
+ return (Pair<String, String>) jobResult;
+ }
+
+ return null;
+ }
+ } else {
+ CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(repair);
+ VolumeInfo volumeInfo = volFactory.getVolume(volumeId);
+ volumeInfo.addPayload(payload);
+
+ Pair<String, String> result = volService.checkAndRepairVolume(volumeInfo);
+ return result;
+ }
+ }
+
+ protected void validationsForCheckVolumeOperation(VolumeVO volume) {
+ Account caller = CallContext.current().getCallingAccount();
+ _accountMgr.checkAccess(caller, null, true, volume);
+
+ Long volumeId = volume.getId();
+ Long vmId = volume.getInstanceId();
+ if (vmId != null) {
+ UserVmVO vm = _userVmDao.findById(vmId);
+ if (vm == null) {
+ throw new InvalidParameterValueException(String.format("VM not found, please check the VM to which this volume %d is attached", volumeId));
Review Comment:
Why should the volume be attached to a VM?
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1817,7 +1822,147 @@ public void publishVolumeCreationUsageEvent(Volume volume) {
s_logger.debug(String.format("Volume [%s] has been successfully recovered, thus a new usage event %s has been published.", volume.getUuid(), EventTypes.EVENT_VOLUME_CREATE));
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_VOLUME_CHECK, eventDescription = "checking volume and repair if needed", async = true)
+ public Pair<String, String> checkAndRepairVolume(CheckAndRepairVolumeCmd cmd) throws ResourceAllocationException {
+ long volumeId = cmd.getId();
+ String repair = cmd.getRepair();
+
+ final VolumeVO volume = _volsDao.findById(volumeId);
+ validationsForCheckVolumeOperation(volume);
+
+ Long vmId = volume.getInstanceId();
+ if (vmId != null) {
+ // serialize VM operation
+ AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
+ if (jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {
+ // avoid re-entrance
+
+ VmWorkJobVO placeHolder = null;
+ placeHolder = createPlaceHolderWork(vmId);
+ try {
+ Pair<String, String> result = orchestrateCheckAndRepairVolume(volumeId, repair);
+ return result;
+ } finally {
+ _workJobDao.expunge(placeHolder.getId());
+ }
+ } else {
+ Outcome<Pair> outcome = checkAndRepairVolumeThroughJobQueue(vmId, volumeId, repair);
+
+ try {
+ outcome.get();
+ } catch (InterruptedException e) {
+ throw new RuntimeException("Operation is interrupted", e);
+ } catch (java.util.concurrent.ExecutionException e) {
+ throw new RuntimeException("Execution exception--", e);
+ }
+
+ Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
+ if (jobResult != null) {
+ if (jobResult instanceof ConcurrentOperationException) {
+ throw (ConcurrentOperationException)jobResult;
+ } else if (jobResult instanceof ResourceAllocationException) {
+ throw (ResourceAllocationException)jobResult;
+ } else if (jobResult instanceof Throwable) {
+ throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
+ }
+ }
+
+ // retrieve the entity url from job result
+ if (jobResult != null && jobResult instanceof Pair) {
+ return (Pair<String, String>) jobResult;
+ }
+
+ return null;
+ }
+ } else {
+ CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(repair);
+ VolumeInfo volumeInfo = volFactory.getVolume(volumeId);
+ volumeInfo.addPayload(payload);
+
+ Pair<String, String> result = volService.checkAndRepairVolume(volumeInfo);
+ return result;
+ }
+ }
+
+ protected void validationsForCheckVolumeOperation(VolumeVO volume) {
+ Account caller = CallContext.current().getCallingAccount();
+ _accountMgr.checkAccess(caller, null, true, volume);
+
+ Long volumeId = volume.getId();
+ Long vmId = volume.getInstanceId();
+ if (vmId != null) {
+ UserVmVO vm = _userVmDao.findById(vmId);
+ if (vm == null) {
+ throw new InvalidParameterValueException(String.format("VM not found, please check the VM to which this volume %d is attached", volumeId));
+ }
+
+ _accountMgr.checkAccess(caller, null, true, vm);
+
+ if (vm.getState() != State.Stopped) {
+ throw new InvalidParameterValueException(String.format("VM to which the volume %d is attached should be in stopped state", volumeId));
+ }
+ }
+
+ if (volume.getState() != Volume.State.Ready) {
+ throw new InvalidParameterValueException(String.format("VolumeId: %d is not in Ready state", volumeId));
+ }
+
+ HypervisorType hypervisorType = _volsDao.getHypervisorType(volume.getId());
+ if (!HypervisorType.KVM.equals(hypervisorType)) {
+ throw new InvalidParameterValueException(String.format("Check and Repair volumes is supported only for KVM hypervisor"));
+ }
+ }
+
+ private Pair<String, String> orchestrateCheckAndRepairVolume(Long volumeId, String repair) {
+
+ VolumeInfo volume = volFactory.getVolume(volumeId);
+
+ if (volume == null) {
+ throw new InvalidParameterValueException("Checking volume and repairing failed due to volume:" + volumeId + " doesn't exist");
+ }
+
+ if (volume.getState() != Volume.State.Ready) {
+ throw new InvalidParameterValueException("VolumeId: " + volumeId + " is not in " + Volume.State.Ready + " state but " + volume.getState() + ". Cannot check and repair the volume.");
+ }
Review Comment:
This is already checked at line 1907 in the validationsForCheckVolumeOperation method
##########
engine/components-api/src/main/java/com/cloud/vm/VmWorkCheckAndRepairVolume.java:
##########
@@ -0,0 +1,42 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.vm;
+
+public class VmWorkCheckAndRepairVolume extends VmWork {
+
+ private static final long serialVersionUID = 341816293003023824L;
Review Comment:
What is this used for?
##########
plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java:
##########
@@ -812,4 +812,45 @@ protected static boolean helpSupportsImageFormat(String text, QemuImg.PhysicalDi
Pattern pattern = Pattern.compile("Supported\\sformats:[a-zA-Z0-9-_\\s]*?\\b" + format + "\\b", CASE_INSENSITIVE);
return pattern.matcher(text).find();
}
+
+ /**
+ * check for any leaks for an image and repair.
+ *
+ * @param imageOptions
+ * Qemu style image options to be used in the checking process.
+ * @param qemuObjects
+ * Qemu style options (e.g. for passing secrets).
+ * @param repair
+ * Boolean option whether to repair any leaks
+ */
+ public String checkAndRepair(final QemuImgFile file, final QemuImageOptions imageOptions, final List<QemuObject> qemuObjects, final String repair) throws QemuImgException {
+ final Script s = new Script(_qemuImgPath);
Review Comment:
We should avoid single letter variables
```suggestion
final Script script = new Script(_qemuImgPath);
```
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckAndRepairVolumeCommandWrapper.java:
##########
@@ -0,0 +1,137 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.storage.CheckAndRepairVolumeCommand;
+import com.cloud.agent.api.storage.CheckAndRepairVolumeAnswer;
+import com.cloud.agent.api.to.StorageFilerTO;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk;
+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.utils.exception.CloudRuntimeException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.cloudstack.utils.cryptsetup.KeyFile;
+import org.apache.cloudstack.utils.qemu.QemuImageOptions;
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImgException;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.apache.cloudstack.utils.qemu.QemuObject;
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.log4j.Logger;
+import org.libvirt.LibvirtException;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+@ResourceWrapper(handles = CheckAndRepairVolumeCommand.class)
+public class LibvirtCheckAndRepairVolumeCommandWrapper extends CommandWrapper<CheckAndRepairVolumeCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtCheckAndRepairVolumeCommandWrapper.class);
+
+ @Override
+ public Answer execute(CheckAndRepairVolumeCommand command, LibvirtComputingResource serverResource) {
+ final String volumeId = command.getPath();
+ final String repair = command.getRepair();
+ final StorageFilerTO spool = command.getPool();
+
+ final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
+ KVMStoragePool pool = storagePoolMgr.getStoragePool(spool.getType(), spool.getUuid());
+
+ final KVMPhysicalDisk vol = pool.getPhysicalDisk(volumeId);
+ QemuObject.EncryptFormat encryptFormat = QemuObject.EncryptFormat.enumValue(command.getEncryptFormat());
+ byte[] passphrase = command.getPassphrase();
+ try {
+ String checkVolumeResult = checkAndRepairVolume(vol, repair, encryptFormat, passphrase, serverResource);
+ s_logger.info(String.format("Check Volume result for the volume %s is %s", vol.getName(), checkVolumeResult));
+ CheckAndRepairVolumeAnswer answer = new CheckAndRepairVolumeAnswer(command, true, checkVolumeResult);
+ answer.setVolumeCheckExecutionResult(checkVolumeResult);
+
+ int leaks = 0;
+ if (StringUtils.isNotEmpty(checkVolumeResult) && StringUtils.isNotEmpty(repair) && repair.equals("leaks")) {
+ ObjectMapper objectMapper = new ObjectMapper();
+ JsonNode jsonNode = objectMapper.readTree(checkVolumeResult);
+ JsonNode leaksNode = jsonNode.get("leaks");
+ if (leaksNode != null) {
+ leaks = jsonNode.asInt();
Review Comment:
Shouldn't this be `leaks = leaksNode.asInt();` ?
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckAndRepairVolumeCommandWrapper.java:
##########
@@ -0,0 +1,137 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.storage.CheckAndRepairVolumeCommand;
+import com.cloud.agent.api.storage.CheckAndRepairVolumeAnswer;
+import com.cloud.agent.api.to.StorageFilerTO;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk;
+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.utils.exception.CloudRuntimeException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.cloudstack.utils.cryptsetup.KeyFile;
+import org.apache.cloudstack.utils.qemu.QemuImageOptions;
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImgException;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.apache.cloudstack.utils.qemu.QemuObject;
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.log4j.Logger;
+import org.libvirt.LibvirtException;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+@ResourceWrapper(handles = CheckAndRepairVolumeCommand.class)
+public class LibvirtCheckAndRepairVolumeCommandWrapper extends CommandWrapper<CheckAndRepairVolumeCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtCheckAndRepairVolumeCommandWrapper.class);
+
+ @Override
+ public Answer execute(CheckAndRepairVolumeCommand command, LibvirtComputingResource serverResource) {
+ final String volumeId = command.getPath();
+ final String repair = command.getRepair();
+ final StorageFilerTO spool = command.getPool();
+
+ final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
+ KVMStoragePool pool = storagePoolMgr.getStoragePool(spool.getType(), spool.getUuid());
+
+ final KVMPhysicalDisk vol = pool.getPhysicalDisk(volumeId);
+ QemuObject.EncryptFormat encryptFormat = QemuObject.EncryptFormat.enumValue(command.getEncryptFormat());
+ byte[] passphrase = command.getPassphrase();
+ try {
+ String checkVolumeResult = checkAndRepairVolume(vol, repair, encryptFormat, passphrase, serverResource);
+ s_logger.info(String.format("Check Volume result for the volume %s is %s", vol.getName(), checkVolumeResult));
+ CheckAndRepairVolumeAnswer answer = new CheckAndRepairVolumeAnswer(command, true, checkVolumeResult);
+ answer.setVolumeCheckExecutionResult(checkVolumeResult);
+
+ int leaks = 0;
+ if (StringUtils.isNotEmpty(checkVolumeResult) && StringUtils.isNotEmpty(repair) && repair.equals("leaks")) {
+ ObjectMapper objectMapper = new ObjectMapper();
+ JsonNode jsonNode = objectMapper.readTree(checkVolumeResult);
+ JsonNode leaksNode = jsonNode.get("leaks");
+ if (leaksNode != null) {
+ leaks = jsonNode.asInt();
+ }
+
+ if (leaks == 0) {
+ String msg = String.format("no leaks found while checking for the volume %s, so skipping repair", vol.getName());
+ s_logger.info(msg);
+ String jsonStringFormat = String.format("{ \"message\": \"%s\" }", msg);
+ String finalResult = (checkVolumeResult != null ? checkVolumeResult.concat(",") : "") + jsonStringFormat;
+ answer = new CheckAndRepairVolumeAnswer(command, true, finalResult);
+ answer.setVolumeRepairExecutionResult(jsonStringFormat);
+ answer.setVolumeCheckExecutionResult(checkVolumeResult);
+
+ return answer;
+ }
+ }
+
+ if (StringUtils.isNotEmpty(repair)) {
+ String repairVolumeResult = checkAndRepairVolume(vol, repair, encryptFormat, passphrase, serverResource);
+ String finalResult = (checkVolumeResult != null ? checkVolumeResult.concat(",") : "") + repairVolumeResult;
+ s_logger.info(String.format("Repair Volume result for the volume %s is %s", vol.getName(), repairVolumeResult));
+
+ answer = new CheckAndRepairVolumeAnswer(command, true, finalResult);
+ answer.setVolumeRepairExecutionResult(repairVolumeResult);
+ answer.setVolumeCheckExecutionResult(checkVolumeResult);
+ }
Review Comment:
I don't understand the need for this block of code, you already called the checkAndRepairVolume method back in line 69, the volume should be repaired already... am I missing something?
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1923438750
> Kept a check in the validate method @DaanHoogland https://github.com/shapeblue/cloudstack/blob/353b2f4f0d938f9fab91c42358859a430dcc1c14/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java#L1909-L1911
>
> is this what you are trying to refer ?
yes, thanks @harikrishna-patnala , totaly read over 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1485927024
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/CheckAndRepairVolumeCmd.java:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject.ResponseView;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.storage.Volume;
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
+
+import java.util.Arrays;
+
+@APICommand(name = "checkVolume", description = "Check the volume for any errors or leaks and also repairs when repair parameter is passed, this is currently supported for KVM only", responseObject = VolumeResponse.class, entityType = {Volume.class},
+ since = "4.19.1",
+ authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
+public class CheckAndRepairVolumeCmd extends BaseAsyncCmd {
+ public static final Logger s_logger = Logger.getLogger(CheckAndRepairVolumeCmd.class.getName());
Review Comment:
thanks @JoaoJandre will update the PR accordingly
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1934047369
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8586
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1931649290
@harikrishna-patnala 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "JoaoJandre (via GitHub)" <gi...@apache.org>.
JoaoJandre commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1483041689
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/CheckAndRepairVolumeCmd.java:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.volume;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.InvalidParameterValueException;
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiCommandResourceType;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject.ResponseView;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.storage.Volume;
+import com.cloud.user.Account;
+import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
+
+import java.util.Arrays;
+
+@APICommand(name = "checkVolume", description = "Check the volume for any errors or leaks and also repairs when repair parameter is passed, this is currently supported for KVM only", responseObject = VolumeResponse.class, entityType = {Volume.class},
+ since = "4.19.1",
+ authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
+public class CheckAndRepairVolumeCmd extends BaseAsyncCmd {
+ public static final Logger s_logger = Logger.getLogger(CheckAndRepairVolumeCmd.class.getName());
Review Comment:
You don't need to create loggers for each new command anymore, they will inherit it from the BaseCmd because of #7131
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1957893976
<b>[SF] Trillian test result (tid-9314)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46931 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8577-t9314-kvm-centos7.zip
Smoke tests completed. 127 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_condensed_drs_algorithm | `Failure` | 164.54 | test_cluster_drs.py
test_02_trigger_shutdown | `Failure` | 336.65 | test_safe_shutdown.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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1955957791
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8729
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1496906410
##########
api/src/main/java/org/apache/cloudstack/api/response/VolumeResponse.java:
##########
@@ -288,6 +289,14 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co
@Param(description = "volume uuid that is given by virtualisation provider (only for VMware)")
private String externalUuid;
+ @SerializedName(ApiConstants.VOLUME_CHECK_RESULT)
+ @Param(description = "details for the volume check result, they may vary for different hypervisors, since = 4.19.1")
Review Comment:
Thanks @sureshanaparti I've rebased the branch to 4.19.
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1920633909
@harikrishna-patnala 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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1920723061
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8487
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on code in PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#discussion_r1472480827
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java:
##########
@@ -1915,6 +1915,13 @@ public void prepare(VirtualMachineProfile vm, DeployDestination dest) throws Sto
}
}
}
+ } else {
+ Host host = _hostDao.findById(vm.getVirtualMachine().getHostId());
+ try {
+ volService.checkAndRepairVolumeBasedOnConfig(volFactory.getVolume(vol.getId()), host);
+ } catch (Exception e) {
+ s_logger.debug(String.format("Unable to check and repair volume [%s] on host [%s], due to %s.", volToString, host, e.getMessage()));
+ }
Review Comment:
new method
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1817,7 +1822,143 @@ public void publishVolumeCreationUsageEvent(Volume volume) {
s_logger.debug(String.format("Volume [%s] has been successfully recovered, thus a new usage event %s has been published.", volume.getUuid(), EventTypes.EVENT_VOLUME_CREATE));
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_VOLUME_CHECK, eventDescription = "checking volume and repair if needed", async = true)
+ public Pair<String, String> checkAndRepairVolume(CheckAndRepairVolumeCmd cmd) throws ResourceAllocationException {
+ long volumeId = cmd.getId();
+ String repair = cmd.getRepair();
+
+ final VolumeVO volume = _volsDao.findById(volumeId);
+ validationsForCheckVolumeOperation(volume);
+
+ Long vmId = volume.getInstanceId();
+ if (vmId != null) {
+ // serialize VM operation
+ AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
+ if (jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {
+ // avoid re-entrance
+
+ VmWorkJobVO placeHolder = null;
+ placeHolder = createPlaceHolderWork(vmId);
+ try {
+ Pair<String, String> result = orchestrateCheckAndRepairVolume(volumeId, repair);
+ return result;
+ } finally {
+ _workJobDao.expunge(placeHolder.getId());
+ }
+ } else {
+ Outcome<Pair> outcome = checkAndRepairVolumeThroughJobQueue(vmId, volumeId, repair);
+
+ try {
+ outcome.get();
+ } catch (InterruptedException e) {
+ throw new RuntimeException("Operation is interrupted", e);
+ } catch (ExecutionException e) {
+ throw new RuntimeException("Execution exception--", e);
+ }
+
+ Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
+ if (jobResult != null) {
+ if (jobResult instanceof ConcurrentOperationException) {
+ throw (ConcurrentOperationException)jobResult;
+ } else if (jobResult instanceof ResourceAllocationException) {
+ throw (ResourceAllocationException)jobResult;
+ } else if (jobResult instanceof Throwable) {
+ throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
+ }
+ }
+
+ // retrieve the entity url from job result
+ if (jobResult != null && jobResult instanceof Pair) {
+ return (Pair<String, String>) jobResult;
+ }
+
+ return null;
+ }
+ } else {
+ CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(repair);
+ VolumeInfo volumeInfo = volFactory.getVolume(volumeId);
+ volumeInfo.addPayload(payload);
+
+ Pair<String, String> result = volService.checkAndRepairVolume(volumeInfo);
+ return result;
Review Comment:
`checkVolumeWithoutVM()`?
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1817,7 +1822,143 @@ public void publishVolumeCreationUsageEvent(Volume volume) {
s_logger.debug(String.format("Volume [%s] has been successfully recovered, thus a new usage event %s has been published.", volume.getUuid(), EventTypes.EVENT_VOLUME_CREATE));
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_VOLUME_CHECK, eventDescription = "checking volume and repair if needed", async = true)
+ public Pair<String, String> checkAndRepairVolume(CheckAndRepairVolumeCmd cmd) throws ResourceAllocationException {
+ long volumeId = cmd.getId();
+ String repair = cmd.getRepair();
+
+ final VolumeVO volume = _volsDao.findById(volumeId);
+ validationsForCheckVolumeOperation(volume);
+
+ Long vmId = volume.getInstanceId();
+ if (vmId != null) {
+ // serialize VM operation
+ AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
+ if (jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {
+ // avoid re-entrance
+
+ VmWorkJobVO placeHolder = null;
+ placeHolder = createPlaceHolderWork(vmId);
+ try {
+ Pair<String, String> result = orchestrateCheckAndRepairVolume(volumeId, repair);
+ return result;
+ } finally {
+ _workJobDao.expunge(placeHolder.getId());
+ }
+ } else {
+ Outcome<Pair> outcome = checkAndRepairVolumeThroughJobQueue(vmId, volumeId, repair);
+
+ try {
+ outcome.get();
+ } catch (InterruptedException e) {
+ throw new RuntimeException("Operation is interrupted", e);
+ } catch (ExecutionException e) {
+ throw new RuntimeException("Execution exception--", e);
+ }
+
+ Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
+ if (jobResult != null) {
+ if (jobResult instanceof ConcurrentOperationException) {
+ throw (ConcurrentOperationException)jobResult;
+ } else if (jobResult instanceof ResourceAllocationException) {
+ throw (ResourceAllocationException)jobResult;
+ } else if (jobResult instanceof Throwable) {
+ throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
+ }
+ }
+
+ // retrieve the entity url from job result
+ if (jobResult != null && jobResult instanceof Pair) {
+ return (Pair<String, String>) jobResult;
+ }
+
+ return null;
Review Comment:
can this go to a `handleJobOutcome()`?
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1817,7 +1822,147 @@ public void publishVolumeCreationUsageEvent(Volume volume) {
s_logger.debug(String.format("Volume [%s] has been successfully recovered, thus a new usage event %s has been published.", volume.getUuid(), EventTypes.EVENT_VOLUME_CREATE));
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_VOLUME_CHECK, eventDescription = "checking volume and repair if needed", async = true)
+ public Pair<String, String> checkAndRepairVolume(CheckAndRepairVolumeCmd cmd) throws ResourceAllocationException {
+ long volumeId = cmd.getId();
+ String repair = cmd.getRepair();
+
+ final VolumeVO volume = _volsDao.findById(volumeId);
+ validationsForCheckVolumeOperation(volume);
+
+ Long vmId = volume.getInstanceId();
+ if (vmId != null) {
+ // serialize VM operation
+ AsyncJobExecutionContext jobContext = AsyncJobExecutionContext.getCurrentExecutionContext();
+ if (jobContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) {
+ // avoid re-entrance
+
+ VmWorkJobVO placeHolder = null;
+ placeHolder = createPlaceHolderWork(vmId);
+ try {
+ Pair<String, String> result = orchestrateCheckAndRepairVolume(volumeId, repair);
+ return result;
+ } finally {
+ _workJobDao.expunge(placeHolder.getId());
+ }
+ } else {
+ Outcome<Pair> outcome = checkAndRepairVolumeThroughJobQueue(vmId, volumeId, repair);
+
+ try {
+ outcome.get();
+ } catch (InterruptedException e) {
+ throw new RuntimeException("Operation is interrupted", e);
+ } catch (java.util.concurrent.ExecutionException e) {
+ throw new RuntimeException("Execution exception--", e);
+ }
+
+ Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob());
+ if (jobResult != null) {
+ if (jobResult instanceof ConcurrentOperationException) {
+ throw (ConcurrentOperationException)jobResult;
+ } else if (jobResult instanceof ResourceAllocationException) {
+ throw (ResourceAllocationException)jobResult;
+ } else if (jobResult instanceof Throwable) {
+ throw new RuntimeException("Unexpected exception", (Throwable)jobResult);
+ }
+ }
+
+ // retrieve the entity url from job result
+ if (jobResult != null && jobResult instanceof Pair) {
+ return (Pair<String, String>) jobResult;
+ }
+
+ return null;
+ }
+ } else {
+ CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(repair);
+ VolumeInfo volumeInfo = volFactory.getVolume(volumeId);
+ volumeInfo.addPayload(payload);
+
+ Pair<String, String> result = volService.checkAndRepairVolume(volumeInfo);
+ return result;
+ }
+ }
+
+ protected void validationsForCheckVolumeOperation(VolumeVO volume) {
+ Account caller = CallContext.current().getCallingAccount();
+ _accountMgr.checkAccess(caller, null, true, volume);
+
+ Long volumeId = volume.getId();
+ Long vmId = volume.getInstanceId();
+ if (vmId != null) {
+ UserVmVO vm = _userVmDao.findById(vmId);
+ if (vm == null) {
+ throw new InvalidParameterValueException(String.format("VM not found, please check the VM to which this volume %d is attached", volumeId));
Review Comment:
basically, if there is a VmId there must be a Vm. (so this block is the inline method `handleVmIdPresentOnVolume()` ;) )
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckAndRepairVolumeCommandWrapper.java:
##########
@@ -0,0 +1,137 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.storage.CheckAndRepairVolumeCommand;
+import com.cloud.agent.api.storage.CheckAndRepairVolumeAnswer;
+import com.cloud.agent.api.to.StorageFilerTO;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk;
+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.utils.exception.CloudRuntimeException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.cloudstack.utils.cryptsetup.KeyFile;
+import org.apache.cloudstack.utils.qemu.QemuImageOptions;
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImgException;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.apache.cloudstack.utils.qemu.QemuObject;
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.log4j.Logger;
+import org.libvirt.LibvirtException;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+@ResourceWrapper(handles = CheckAndRepairVolumeCommand.class)
+public class LibvirtCheckAndRepairVolumeCommandWrapper extends CommandWrapper<CheckAndRepairVolumeCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtCheckAndRepairVolumeCommandWrapper.class);
+
+ @Override
+ public Answer execute(CheckAndRepairVolumeCommand command, LibvirtComputingResource serverResource) {
+ final String volumeId = command.getPath();
+ final String repair = command.getRepair();
+ final StorageFilerTO spool = command.getPool();
+
+ final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
+ KVMStoragePool pool = storagePoolMgr.getStoragePool(spool.getType(), spool.getUuid());
+
+ final KVMPhysicalDisk vol = pool.getPhysicalDisk(volumeId);
+ QemuObject.EncryptFormat encryptFormat = QemuObject.EncryptFormat.enumValue(command.getEncryptFormat());
+ byte[] passphrase = command.getPassphrase();
+ try {
+ String checkVolumeResult = checkAndRepairVolume(vol, repair, encryptFormat, passphrase, serverResource);
+ s_logger.info(String.format("Check Volume result for the volume %s is %s", vol.getName(), checkVolumeResult));
+ CheckAndRepairVolumeAnswer answer = new CheckAndRepairVolumeAnswer(command, true, checkVolumeResult);
+ answer.setVolumeCheckExecutionResult(checkVolumeResult);
+
+ int leaks = 0;
+ if (StringUtils.isNotEmpty(checkVolumeResult) && StringUtils.isNotEmpty(repair) && repair.equals("leaks")) {
+ ObjectMapper objectMapper = new ObjectMapper();
+ JsonNode jsonNode = objectMapper.readTree(checkVolumeResult);
+ JsonNode leaksNode = jsonNode.get("leaks");
+ if (leaksNode != null) {
+ leaks = jsonNode.asInt();
+ }
+
+ if (leaks == 0) {
+ String msg = String.format("no leaks found while checking for the volume %s, so skipping repair", vol.getName());
+ s_logger.info(msg);
+ String jsonStringFormat = String.format("{ \"message\": \"%s\" }", msg);
+ String finalResult = (checkVolumeResult != null ? checkVolumeResult.concat(",") : "") + jsonStringFormat;
+ answer = new CheckAndRepairVolumeAnswer(command, true, finalResult);
+ answer.setVolumeRepairExecutionResult(jsonStringFormat);
+ answer.setVolumeCheckExecutionResult(checkVolumeResult);
+
+ return answer;
+ }
+ }
+
+ if (StringUtils.isNotEmpty(repair)) {
+ String repairVolumeResult = checkAndRepairVolume(vol, repair, encryptFormat, passphrase, serverResource);
+ String finalResult = (checkVolumeResult != null ? checkVolumeResult.concat(",") : "") + repairVolumeResult;
+ s_logger.info(String.format("Repair Volume result for the volume %s is %s", vol.getName(), repairVolumeResult));
+
+ answer = new CheckAndRepairVolumeAnswer(command, true, finalResult);
+ answer.setVolumeRepairExecutionResult(repairVolumeResult);
+ answer.setVolumeCheckExecutionResult(checkVolumeResult);
+ }
Review Comment:
maybe you can disect this method to make it more readable @harikrishna-patnala ?
--
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] New API "checkVolume" to check and repair any leaks or issues reported by qemu-img check [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8577:
URL: https://github.com/apache/cloudstack/pull/8577#issuecomment-1958778927
@harikrishna-patnala 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