You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/09/13 09:12:25 UTC
[GitHub] [cloudstack] slavkap opened a new pull request #3724: Storage-based Snapshots for KVM VMs
slavkap opened a new pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724
## Description
Cloudstack with KVM as a hypervisor provides live VM snapshots only with volumes which image format is QCOW.
This is a limitation for storage providers with disks in RAW format.
link to ML: http://mail-archives.apache.org/mod_mbox/cloudstack-dev/201911.mbox/%3cCAA6FghF7eaY-A3XGN5zSwKVvp7zrcuNzv5nFAQKaF+et3zH-ag@mail.gmail.com%3e#archives
With this feature
- CloudStack users will be able to do a live consistent snapshots of all disks attached to the VM
- Is using respective storage provider plugin implementation to take the snapshot
- For NFS/Local storages we are using qemu command - drive-backup. We recommend to use
this functionality only if you have VMs with mixed volumes (in RAW and QCOW format).
- You can create VM snapshot without it's memory
- Creates in the UI and DB VM snapshot and volume's snapshot related to the VM snapshot
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [X] 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)
## Documentation
https://github.com/apache/cloudstack-documentation/pull/153
## Prerequisites
- Install qemu-guest-agent on guest machine (used to freeze/thaw the virtual machine)
- In global settings "kvm.vmstoragesnapshot.enabled" has to be set to true
- If volumes' image format is QCOW you need - qemu version 1.6+ for drive-backup command. If you’re with the latest stable Ubuntu it will be included, but for CentOS7 you need to install qemu-kvm-ev.
- When using drive-backup command the option - "snapshot.backup.to.secondary" should be enabled.
## How Has This Been Tested?
Environment:
- CloudStack - management and agent version - 4.14.0.0-SNAPSHOT
- OS for management and hypervisor - CentOS7
- Hypervisor - KVM
- Primary storages - NFS, StorPool, Local storage and Ceph.
Take snapshot:
- Take VM snapshot without memory on running virtual machine
- We have tested this feature with these storage providers - NFS, StorPool, Local storage and Ceph.
- There is one smoke test, which creates VM, installs qemu-guest-agent, takes, reverts and deletes VM snapshot.
Revert snapshot
- Stop the virtual machine and revert the VM snapshot
--
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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r487055111
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-698187777
@rhtyd a Jenkins job has been kicked to build packages. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-713423271
@slavkap i think there is a PR out to up the default template. not sure if it is ready or merged. for test purposes look at http://dl.openvm.eu/cloudstack/macchinina/x86_64/
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725952761
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r682468064
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
@slavkap Some questions.
- Does the snapshot operation here triggered through VM snapshot operation (as there are changes in VM snapshot strategies)?
- Are the actual volume snapshots taken (for the volumes of the VM) are maintained at volume level (in snapshots table)?
- Does the snapshots taken here are listed with VM snapshots or volume snapshots?
If the snapshots here are VM level snapshots, i think it is better to move these new states (and associated state transitions) to VM Snapshot object.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-868881465
<b>Trillian test result (tid-1098)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 56316 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t1098-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_routers.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshot_kvm.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 83 look OK, 6 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_add_delete_kubernetes_supported_version | `Error` | 60.64 | test_kubernetes_supported_versions.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 340.26 | test_routers_network_ops.py
test_10_attachAndDetach_iso | `Failure` | 826.88 | test_vm_life_cycle.py
ContextSuite context=TestVmSnapshot>:setup | `Error` | 0.00 | test_vm_snapshot_kvm.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 508.68 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Failure` | 570.63 | test_vpc_redundant.py
test_01_vpc_site2site_vpn_multiple_options | `Failure` | 438.10 | test_vpc_vpn.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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-709532612
@DaanHoogland a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-867492850
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919869653
@blueorangutan test matrix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-864851177
It's allright to test for requirements and execute based on availability, so by no means remove them, @slavkap .
My biggest worry is the big interval of functionality differences we have based on qemu versions and how we can handle those in a operator friendly way.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-878217391
Thanks, @nvazquez, that you will schedule this for testing! Actually, I'm testing it whenever I push something new
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-867530792
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 356
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-864857010
Thanks, @DaanHoogland. I will add these checks depending on OS and qemu 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919849762
@rhtyd a Jenkins job has been kicked to build packages. 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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070819396
@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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1083348976
@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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-864827359
Thank you, @DaanHoogland. Then it's normal mine Marvin's tests to fail. Do I have to remove the tests from this PR or skip them?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r518038097
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedKVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
@sureshanaparti @slavkap changing from _AllocatedKVM_ to _AllocatedVM_ would be better indeed. What do you think @sureshanaparti?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r682468064
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
@slavkap Some questions.
- Does the snapshot operation here triggered through VM snapshot operation (as there are changes in VM snapshot strategies)?
- Are the actual volume snapshots taken (for the volumes of the VM) are maintained at volume level (in snapshots table)?
- Does the snapshots taken here are listed with VM snapshots or volume snapshots?
If the snapshots here are VM level snapshots, i think it is better to move these new states (and associated state transitions) to VM Snapshot object.
--
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
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-914911632
@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
[GitHub] [cloudstack] slavkap edited a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap edited a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-878217391
Thanks, @nvazquez, that you will schedule this for testing! Actually, I'm testing it whenever I push something new. I'll do the tests with the latest main changes
--
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
[GitHub] [cloudstack] rhtyd closed pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-918065991
Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_multiplication_x: debian :heavy_check_mark: suse15. SL-JID 1212
--
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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-882684788
Thanks, @weizhouapache. I think that the problem is fixed. I've tested it only on agents with CentOS7 but did the tests with qemu-monitor-command on my workstation (Ubuntu), and it's working with the solution from the link you shared
--
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
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r694911422
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
> > * Does the snapshot operation here triggered through VM snapshot operation (as there are changes in VM snapshot strategies)?
>
> Yes, it's triggered through the VM snapshot operation.
>
> > * Are the actual volume snapshots taken (for the volumes of the VM) are maintained at volume level (in snapshots table)?
>
> They are maintained mostly at the VM level. You will see a volume snapshot, but you don't have any options to operate separately on it.
>
> > * Does the snapshots taken here are listed with VM snapshots or volume snapshots?
>
> You'll see both a VM snapshot and also a volume snapshot for each volume of the group snapshot.
>
> 
> 
@slavkap As no operations can be performed on the volume snapshots (of the VM) and these are read-only, I feel it is better to keep the snapshot as VM snapshot only and maintain the relation of these volume snapshots to VM snapshot at the backend (in DB). Also, better to move these new states (and associated state transitions) to VM Snapshot object.
--
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-864807418
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-632583762
Hello,
I have proposed to libvirt-java project the virDomainQemuMonitorCommand and virDomainQemuAgentCommand implementation (as @wido requested), which were accepted and merged to the project. I'll be happy if some of you can help me with information, what should I do, those changes to go to maven central repo?
Thanks in advance!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919762470
We can rule out suse failure - likely jenkins build issue.
@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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-928962946
Thank you, @sureshanaparti, for your help and time! I marked the PR as a draft until I remove the new states and do some changes. Also, I will remove the functionality for the NFS/Local storage. It's hard to set it up on CentOS hypervisors. The PR proposed by Daniel [5297](https://github.com/apache/cloudstack/pull/5297) is cleaner, and it's better to use it for NFS/Local storage.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-917996581
@rhtyd a Jenkins job has been kicked to build packages. 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
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919849455
Did it overwrite packaging, why do we've three comments cc @davidjumani ?
@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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-698205448
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2080
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-711754387
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881274350
<b>Trillian Build Failed (tid-1288)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1083349610
@nvazquez a 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-884024450
@weizhouapache a Jenkins job has been kicked to build packages. 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919802396
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1259
--
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
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-698187570
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-914911841
@rhtyd a Jenkins job has been kicked to build packages. 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-914944957
Packaging result: :heavy_multiplication_x: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1160
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-921103934
<b>Trillian test result (tid-2081)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42617 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t2081-xenserver-71.zip
Smoke tests completed. 90 look OK, 0 have errors
Only failed 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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-632600979
Thanks @wido ! I'll work on 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-713357505
@slavkap can you look at the logs for those errors and assess them? @GabrielBrascher can you give a second look (code freeze imminent)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-702181069
@GabrielBrascher a Jenkins job has been kicked to build packages. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r498206421
##########
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##########
@@ -358,9 +359,13 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc
throw new InvalidParameterValueException("Can not snapshot memory when VM is not in Running state");
}
+ boolean isKVMsnapshotsEnabled = SnapshotManager.VMsnapshotKVM.value() == null || SnapshotManager.VMsnapshotKVM.value();
Review comment:
Thanks! I've missed this scenario.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-799591652
Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_check_mark: debian. SL-JID 124
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-702097695
> Thanks for the PR @slavkap.
>
> I raised a few comments and will be taking a deeper look at it this week as well.
Thank you @GabrielBrascher!
I did the changes regarding your comments
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-884023994
@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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-702215793
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2109
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r836715012
##########
File path: plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.utils.qemu;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+public class QemuCommand {
+ //Qemu agent commands
+ public static final String AGENT_FREEZE = "guest-fsfreeze-freeze";
+ public static final String AGENT_THAW = "guest-fsfreeze-thaw";
+ public static final String AGENT_FREEZE_STATUS = "guest-fsfreeze-status";
+ //Qemu monitor commands
+ public static final String QEMU_QUERY_BLOCK_JOBS = "query-block-jobs";
+ public static final String QEMU_BLOCK = "query-block";
+ public static final String QEMU_DRIVE_BACKUP = "drive-backup";
+
+ public static final String QEMU_CMD = "execute";
+
+ public static Map<String, Object> executeQemuCommand(String command, Map<String, String> args ){
Review comment:
Can this be renamed? It is a but misleading since nothing is executed at this time
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,104 @@
+//
+// 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 org.apache.cloudstack.utils.qemu.QemuCommand;
+import org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.google.gson.Gson;
+import com.google.gson.JsonParser;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption(),
+ vmName, domainState));
+ }
+
+ String result = getResultOfQemuCommand(command.getOption(), domain);
+ s_logger.debug(String.format("Result of %s command is %s", command.getOption(), result));
+ if (result == null || (result.startsWith("error"))) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s vm %s due to result status is: %s",
+ command.getOption(), vmName, result));
+ }
+ String status = getResultOfQemuCommand(FreezeThawVMCommand.STATUS, domain);
+ s_logger.debug(String.format("Status of %s command is %s", command.getOption(), status));
+ if (status != null && new JsonParser().parse(status).isJsonObject()) {
+ String statusResult = new JsonParser().parse(status).getAsJsonObject().get("return").getAsString();
+ if (statusResult.equals(command.getOption())) {
+ return new FreezeThawVMAnswer(command, true, String.format("%s of VM - %s is successful", command.getOption(), vmName));
+ }
+ }
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s vm %s due to result status is: %s",
+ command.getOption(), vmName, status));
+ } catch (LibvirtException libvirtException) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s VM - %s due to %s",
+ command.getOption(), vmName, libvirtException.getMessage()));
+ }finally {
+ if (domain != null) {
+ try {
+ domain.free();
+ } catch (LibvirtException e) {
+ s_logger.trace("Ingore error ", e);
+ }
+ }
+ }
+ }
+
+ private String getResultOfQemuCommand(String cmd, Domain domain) throws LibvirtException {
+ String result = null;
+ if (cmd.equals(FreezeThawVMCommand.FREEZE)) {
+ result = domain.qemuAgentCommand(new Gson().toJson(QemuCommand.executeQemuCommand(QemuCommand.AGENT_FREEZE, null)).toString(), 10, 0);
Review comment:
I think readability could be increased on these 3 calls by making the `QemuCommand.executeQemuCommand` (or renamed) method return a string instead of a map.
Also, these 3 calls could be reduced to only 1, by defining a collection that returns the agent "guest-fsfreeze-*" command based on the `cmd`
##########
File path: plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.utils.qemu;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+public class QemuCommand {
+ //Qemu agent commands
+ public static final String AGENT_FREEZE = "guest-fsfreeze-freeze";
+ public static final String AGENT_THAW = "guest-fsfreeze-thaw";
+ public static final String AGENT_FREEZE_STATUS = "guest-fsfreeze-status";
+ //Qemu monitor commands
+ public static final String QEMU_QUERY_BLOCK_JOBS = "query-block-jobs";
Review comment:
These 3 are not used, could be removed?
##########
File path: server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java
##########
@@ -59,6 +59,8 @@
public static final ConfigKey<Boolean> BackupSnapshotAfterTakingSnapshot = new ConfigKey<Boolean>(Boolean.class, "snapshot.backup.to.secondary", "Snapshots", "true",
"Indicates whether to always backup primary storage snapshot to secondary storage. Keeping snapshots only on Primary storage is applicable for KVM + Ceph only.", false, ConfigKey.Scope.Global, null);
+ public static final ConfigKey<Boolean> VMsnapshotKVM = new ConfigKey<>(Boolean.class, "kvm.vmstoragesnapshot.enabled", "Snapshots", "false", "For live snapshot of virtual machine instance on KVM hypervisor without memory. Requieres qemu version 1.6+ (on NFS or Local file system) and qemu-guest-agent installed on guest VM", true, ConfigKey.Scope.Global, null);
Review comment:
Could it be renamed to something like 'VMStorageSnapshotKVM'? Could the scope be reduced to zone?
--
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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r517984491
##########
File path: api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java
##########
@@ -31,7 +31,7 @@
enum State {
Allocated("The VM snapshot is allocated but has not been created yet."), Creating("The VM snapshot is being created."), Ready(
"The VM snapshot is ready to be used."), Reverting("The VM snapshot is being used to revert"), Expunging("The volume is being expunging"), Removed(
- "The volume is destroyed, and can't be recovered."), Error("The volume is in error state, and can't be recovered");
+ "The volume is destroyed, and can't be recovered."), CreatingKVM("Creating KVM VM snapshot"), Error("The volume is in error state, and can't be recovered");
Review comment:
I'll check if I can omit those changes, thanks @sureshanaparti
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-724971773
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725278042
> Only failed tests results shown below:
>
> Test Result Time (s) Test File
> test_01_create_vm_snapshots `Error` 32.64 test_vm_snapshot_kvm.py
> test_02_revert_vm_snapshots `Failure` 30.16 test_vm_snapshot_kvm.py
@DaanHoogland I'll look at the logs, but is there a way that I can see the management log, to check the exception?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-864807418
@slavkap, the installed packages;
```
# yum list | grep qemu
ipxe-roms-qemu.noarch 20180825-3.git133f4c.el7 @base
libvirt-daemon-driver-qemu.x86_64 4.5.0-36.el7_9.5 @updates
qemu-img.x86_64 10:1.5.3-175.el7_9.4 @updates
qemu-kvm.x86_64 10:1.5.3-175.el7_9.4 @updates
qemu-kvm-common.x86_64 10:1.5.3-175.el7_9.4 @updates
```
```
# yum list | grep libvirt
libvirt.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-bash-completion.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-client.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-config-network.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-config-nwfilter.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-driver-interface.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-driver-lxc.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-driver-network.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-driver-nodedev.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-driver-nwfilter.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-driver-qemu.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-driver-secret.x86_64 4.5.0-36.el7_9.5 @updates
libvirt-daemon-driver-storage.x86_64 4.5.0-36.el7_9.5 @updates
...
libvirt-daemon-driver-storage-rbd.x86_64 4.5.0-36.el7_9.5 @updates
...
libvirt-libs.x86_64 4.5.0-36.el7_9.5 @updates
python36-libvirt.x86_64 4.5.0-1.el7 @epel
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725990483
@slavkap the env was destroyed so running the full suit. if you want to run just that test, you are a bit on your own.
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-928962946
Thank you, @sureshanaparti, for your help and time! I marked the PR as a draft until I remove the new states and do some changes. Also, I will remove the functionality for the NFS/Local storage. It's hard to set it up on CentOS hypervisors. The PR proposed by Daniel [5297](https://github.com/apache/cloudstack/pull/5297) is cleaner, and it's better to use it for NFS/Local storage.
--
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-921642402
@slavkap can you add a javadoc to the most important classes using the term freeze-thaw to explain what is meant by that term in this context, please?
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-920396398
<b>Trillian test result (tid-2063)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40418 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t2063-xenserver-71.zip
Smoke tests completed. 88 look OK, 2 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_nic | `Error` | 189.26 | test_nic.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 105.52 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 94.15 | test_kubernetes_clusters.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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-870110026
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-801834746
Packaging result: :heavy_check_mark: centos7 :heavy_multiplication_x: centos8 :heavy_check_mark: debian. SL-JID 170
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-884008887
> Thanks, @weizhouapache. I think that the problem is fixed. I've tested it only on agents with CentOS7 but did the tests with qemu-monitor-command on my workstation (Ubuntu), and it's working with the solution from the link you shared
@slavkap great.
this is tested ok on ubuntu 20.04. @nvazquez
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-884529295
<b>Trillian test result (tid-1338)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38558 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t1338-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 90 look OK, 0 have error(s)
Only failed 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-920319404
<b>Trillian test result (tid-2056)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40595 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t2056-kvm-centos7.zip
Smoke tests completed. 90 look OK, 0 have errors
Only failed 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919868126
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1266
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919744007
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_multiplication_x: suse15. SL-JID 1258
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725987455
Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2375
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-711896788
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-713372711
> @slavkap can you look at the logs for those errors and assess them? @GabrielBrascher can you give a second look (code freeze imminent)
@DaanHoogland I think I've found the problem, which is related to unused import that cannot find module. But I think that there will be another problem - in the tests I'm installing "qemu-guest-agent" on the quest VM, is there a way to point out a template which is higher than CentOS 5.5 (I mean which is not out of support)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-711756474
@DaanHoogland a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-726332812
<b>Trillian test result (tid-3161)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34996 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t3161-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshot_kvm.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 84 look OK, 3 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_04_rvpc_privategw_static_routes | `Failure` | 863.40 | test_privategw_acl.py
ContextSuite context=TestVmSnapshot>:setup | `Error` | 0.00 | test_vm_snapshot_kvm.py
test_01_redundant_vpc_site2site_vpn | `Failure` | 296.55 | test_vpc_vpn.py
test_01_vpc_site2site_vpn | `Failure` | 227.35 | test_vpc_vpn.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-884095431
@weizhouapache a 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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-870131550
@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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-698187777
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-879391226
@weizhouapache can you please test this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1083385620
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3027
--
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
[GitHub] [cloudstack] nvazquez removed a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez removed a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070958046
@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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070864827
@nvazquez a 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
[GitHub] [cloudstack] nvazquez removed a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez removed a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070958046
@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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1071049463
@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
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-985317697
@slavkap just to confirm, is this PR on-hold until your changes or are you saying we should go ahead with reviewing/testing https://github.com/apache/cloudstack/pull/5297 ?
--
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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r518743736
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedKVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
@GabrielBrascher I've renamed it to AllocatedVM, @sureshanaparti is it OK to change only the name or I have to think to implement it in different way?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-726534260
I've checked the error and it couldn't create a VM because of this:
> 2020-11-12 16:44:36,206 DEBUG [c.c.a.m.a.i.FirstFitAllocator] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91 FirstFitRoutingAllocator) (logid:144793be) Host Allocator returning 0 suitab
> le hosts
> 2020-11-12 16:44:36,206 DEBUG [c.c.d.DeploymentPlanningManagerImpl] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) No suitable hosts found
> 2020-11-12 16:44:36,206 DEBUG [c.c.d.DeploymentPlanningManagerImpl] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) No suitable hosts found under this Cluster: 1
> 2020-11-12 16:44:36,207 DEBUG [c.c.d.DeploymentPlanningManagerImpl] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) Could not find suitable Deployment Destination for t
> his VM under any clusters, returning.
> 2020-11-12 16:44:36,208 DEBUG [c.c.d.FirstFitPlanner] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) Searching resources only under specified Cluster: 1
> 2020-11-12 16:44:36,208 DEBUG [c.c.d.FirstFitPlanner] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) The specified cluster is in avoid set, returning.
> 2020-11-12 16:44:36,212 DEBUG [c.c.d.DeploymentPlanningManagerImpl] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) DeploymentPlanner allocation algorithm: null
> 2020-11-12 16:44:36,212 DEBUG [c.c.d.DeploymentPlanningManagerImpl] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) Trying to allocate a host and storage pools from dc:
> 1, pod:1,cluster:null, requested cpu: 500, requested ram: (512.00 MB) 536870912
> 2020-11-12 16:44:36,212 DEBUG [c.c.d.DeploymentPlanningManagerImpl] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) Is ROOT volume READY (pool already allocated)?: No
> 2020-11-12 16:44:36,212 DEBUG [c.c.d.DeploymentPlanningManagerImpl] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) Deploy avoids pods: [], clusters: [1], hosts: [1, 2]
> 2020-11-12 16:44:36,213 DEBUG [c.c.d.FirstFitPlanner] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) Searching resources only under specified Pod: 1
> 2020-11-12 16:44:36,214 DEBUG [c.c.d.FirstFitPlanner] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) Listing clusters in order of aggregate capacity, that have (at lea
> st one host with) enough CPU and RAM capacity under this Pod: 1
> 2020-11-12 16:44:36,216 DEBUG [c.c.d.FirstFitPlanner] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) Removing from the clusterId list these clusters from avoid set: [1
> ]
> 2020-11-12 16:44:36,218 DEBUG [c.c.d.FirstFitPlanner] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) No clusters found after removing disabled clusters and clusters in
> avoid list, returning.
> 2020-11-12 16:44:36,228 DEBUG [c.c.c.CapacityManagerImpl] (Work-Job-Executor-58:ctx-8d676e31 job-2472/job-2475 ctx-e703fb91) (logid:144793be) VM state transitted from :Starting to Stopped with event: Oper
> ationFailedvm's original host id: null new host id: null host id before state transition: 1
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-870109568
--
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725294061
> > Only failed tests results shown below:
> > Test Result Time (s) Test File
> > test_01_create_vm_snapshots `Error` 32.64 test_vm_snapshot_kvm.py
> > test_02_revert_vm_snapshots `Failure` 30.16 test_vm_snapshot_kvm.py
>
> @DaanHoogland I'll look at the logs, but is there a way that I can see the management log, to check the exception?
it should be included in the zip, https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t3143-kvm-centos7.zip
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-920433382
<b>Trillian test result (tid-2065)</b>
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43187 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t2065-vmware-65u2.zip
Smoke tests completed. 90 look OK, 0 have errors
Only failed 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919869794
@rhtyd a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-711756474
@DaanHoogland a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881274350
<b>Trillian Build Failed (tid-1288)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725990747
@DaanHoogland a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r696611744
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
Hi @sureshanaparti, I was thinking a lot about your suggestion (and I did it also before 2 years when I've started implementing this PR). Unfortunately, this isn't possible. With this feature, we are creating something like a group snapshot for primary storages that do not have such an implementation. That's why we have to invoke for each volume the respective storage plugin method `takeSnapshot` which requires a DB record in `snapshots` and `snapshot_store_ref` tables.
I think that there are two options with those states, which are:
- to hide them when listing snapshots.
- or all storage plugins have to be modified for this functionality without the need for a DB record
we discussed offline that the NFS snapshots are backup to secondary storage, which was required for me to check that the job of `drive-backup` command is finished and to thaw the VM as soon as possible. I have one suggestion here:
- to remove the support in this functionality for NFS volumes. In this case, there is no need to backup the snapshots on secondary (for the plugins which support backup)
- and to use this implementation when it gets in - [PR 5297](https://github.com/apache/cloudstack/pull/5297).
--
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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r682509925
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
> * Does the snapshot operation here triggered through VM snapshot operation (as there are changes in VM snapshot strategies)?
Yes, it's triggered through the VM snapshot operation.
>
> * Are the actual volume snapshots taken (for the volumes of the VM) are maintained at volume level (in snapshots table)?
>
They are maintained mostly at the VM level. You will see a volume snapshot, but you don't have any options to operate separately on it.
> * Does the snapshots taken here are listed with VM snapshots or volume snapshots?
You'll see both a VM snapshot and also a volume snapshot for each volume of the group snapshot.
--
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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-884109297
@davidjumani please check if you can create a separate PR for the UI issue
--
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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-878196200
Thanks @slavkap, will schedule some time for testing it. Quick question: I've noticed this PR was tested on 4.14 could you test it on the current main branch as well?
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-870110026
@nvazquez a Jenkins job has been kicked to build packages. 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881256930
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 562
--
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
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r517861928
##########
File path: api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java
##########
@@ -31,7 +31,7 @@
enum State {
Allocated("The VM snapshot is allocated but has not been created yet."), Creating("The VM snapshot is being created."), Ready(
"The VM snapshot is ready to be used."), Reverting("The VM snapshot is being used to revert"), Expunging("The volume is being expunging"), Removed(
- "The volume is destroyed, and can't be recovered."), Error("The volume is in error state, and can't be recovered");
+ "The volume is destroyed, and can't be recovered."), CreatingKVM("Creating KVM VM snapshot"), Error("The volume is in error state, and can't be recovered");
Review comment:
VM snapshot states / event shouldn't be hypervisor specific. Can this be addressed in any other way?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd closed pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r837118183
##########
File path: server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java
##########
@@ -59,6 +59,8 @@
public static final ConfigKey<Boolean> BackupSnapshotAfterTakingSnapshot = new ConfigKey<Boolean>(Boolean.class, "snapshot.backup.to.secondary", "Snapshots", "true",
"Indicates whether to always backup primary storage snapshot to secondary storage. Keeping snapshots only on Primary storage is applicable for KVM + Ceph only.", false, ConfigKey.Scope.Global, null);
+ public static final ConfigKey<Boolean> VMsnapshotKVM = new ConfigKey<>(Boolean.class, "kvm.vmstoragesnapshot.enabled", "Snapshots", "false", "For live snapshot of virtual machine instance on KVM hypervisor without memory. Requieres qemu version 1.6+ (on NFS or Local file system) and qemu-guest-agent installed on guest VM", true, ConfigKey.Scope.Global, null);
Review comment:
@nvazquez, I will rename it, but why limit this to a Zone?
--
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
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-917994934
@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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-921090869
<b>Trillian test result (tid-2082)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41355 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t2082-xenserver-71.zip
Smoke tests completed. 90 look OK, 0 have errors
Only failed 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-920367411
<b>Trillian test result (tid-2064)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36994 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t2064-kvm-centos7.zip
Smoke tests completed. 89 look OK, 1 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_list_snapshots_with_removed_data_store | `Error` | 51.67 | test_snapshots.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
[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-711754387
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r497424235
##########
File path: plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java
##########
@@ -0,0 +1,66 @@
+package org.apache.cloudstack.utils.qemu;
Review comment:
Can you please add the Apache license header?
##########
File path: server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
##########
@@ -358,9 +359,13 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc
throw new InvalidParameterValueException("Can not snapshot memory when VM is not in Running state");
}
+ boolean isKVMsnapshotsEnabled = SnapshotManager.VMsnapshotKVM.value() == null || SnapshotManager.VMsnapshotKVM.value();
Review comment:
What do you think of considering `null` as `false` instead of `true`? The default configuration is `false`, therefore, I think that in the case of `null` it would be "safer" to assume it as the default.
In such a case, the KVM snapshot would be enabled only if SnapshotManager.VMsnapshotKVM is set to `true`.
```
boolean isKVMsnapshotsEnabled = SnapshotManager.VMsnapshotKVM.value() != null && SnapshotManager.VMsnapshotKVM.value();
```
##########
File path: engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java
##########
@@ -0,0 +1,457 @@
+/*
+ * 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.storage.vmsnapshot;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+import java.util.UUID;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProviderManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
+import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.cloudstack.test.utils.SpringUtils;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Matchers;
+import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.ComponentScan;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.FilterType;
+import org.springframework.core.type.classreading.MetadataReader;
+import org.springframework.core.type.classreading.MetadataReaderFactory;
+import org.springframework.core.type.filter.TypeFilter;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.support.AnnotationConfigContextLoader;
+
+import com.cloud.agent.AgentManager;
+import com.cloud.agent.api.DeleteVMSnapshotAnswer;
+import com.cloud.agent.api.RevertToVMSnapshotAnswer;
+import com.cloud.agent.api.VMSnapshotTO;
+import com.cloud.exception.AgentUnavailableException;
+import com.cloud.exception.OperationTimedoutException;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.storage.CreateSnapshotPayload;
+import com.cloud.storage.GuestOSHypervisorVO;
+import com.cloud.storage.GuestOSVO;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.VolumeApiService;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.dao.GuestOSDao;
+import com.cloud.storage.dao.GuestOSHypervisorDao;
+import com.cloud.storage.dao.SnapshotDao;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.storage.snapshot.SnapshotApiService;
+import com.cloud.user.AccountService;
+import com.cloud.utils.component.ComponentContext;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
+import com.cloud.utils.net.NetUtils;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.dao.UserVmDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
+
+import junit.framework.TestCase;
+
+@RunWith(SpringJUnit4ClassRunner.class)
+@ContextConfiguration(loader = AnnotationConfigContextLoader.class)
+public class VMSnapshotStrategyKVMTest extends TestCase{
+ List<StoragePoolVO> storage;
+// @Inject
+// VMSnapshotStrategy vmSnapshotStrategy;
Review comment:
Can you please remove the commented lines above?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-870131925
@nvazquez a 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
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919713360
@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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1075280179
Thanks @rp- can you please approve on the Files changed tab -> Submit review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r517983999
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedKVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
Hello @sureshanaparti , thanks for the review! The design of the current implementation was to change as less as possible in CloudStack, that's why we are reusing a lot of the functionality. The additional snapshot's states are added to distinguish with the existing functionality of VM snapshots. In this PR we are lying to CloudStack that a VM snapshot is created, actually we create a separate snapshot for each disk of the VM (and they are consistent, because of the freeze of VM). And the additional states are needed to forbid the user at the end to delete one of the volume's snapshots (by mistake or on purpose).
I'd be glad if you can give me an advice with another option
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-867493284
@slavkap a Jenkins job has been kicked to build packages. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881556244
@weizhouapache, can you share the error you've got with your change of `device` to `node-name`?
--
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-724924210
@wido all your comments seem addressed. Are you fine with this?
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-887928201
@sureshanaparti can you please review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-692512250
> I have some problem. I am used ceph storage but after create vm then reboot ceph server osd service can't start. How can I solve this. this is ceph issue or cloudstack issue ? I am not sure.
Hello @alamintech I'm not deeply familiar with CEPH, but I guess you should ask this question in users or dev mailing list. This pull request does not affect the creation of VM or Ceph's services
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-870126699
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 399
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725266745
<b>Trillian test result (tid-3143)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36014 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t3143-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshot_kvm.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_create_vm_snapshots | `Error` | 32.64 | test_vm_snapshot_kvm.py
test_02_revert_vm_snapshots | `Failure` | 30.16 | test_vm_snapshot_kvm.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-726010810
> @slavkap the env was destroyed so running the full suit. if you want to run just that test, you are a bit on your own.
Thanks @DaanHoogland I did not have bad intentions with the question whatever you could rerun only this test. I just wanted to check the result without to wait all tests to be executed. And if it fails again with this setup, what is the reason. Anyway I know that I'm on my own, but I've executed this test so many times on my dev, without any failures
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-868282663
@sureshanaparti a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland removed a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland removed a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-709532351
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919803113
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919041464
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_multiplication_x: debian :heavy_check_mark: suse15. SL-JID 1239
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-918065628
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_check_mark: suse15. SL-JID 1211
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-919715466
@rhtyd a Jenkins job has been kicked to build packages. 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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r487055111
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez closed pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez closed pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724
--
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
[GitHub] [cloudstack] weizhouapache commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881668644
> @weizhouapache, can you share the error you've got with your change of `device` to `node-name`?
@slavkap the error is same as posted on http://qemu.11.n7.nabble.com/query-block-command-is-missing-device-name-Deprecated-td714506.html
```
2021-07-16 19:25:02,243 DEBUG [c.c.v.s.VMSnapshotManagerImpl] (Work-Job-Executor-1:ctx-0db99c99 job-684/job-685 ctx-d351d56e) (logid:cee7bf47) Failed to create vm snapshot: 8
com.cloud.utils.exception.CloudRuntimeException: Failed to take snapshot: {"id":"libvirt-2170","error":{"class":"GenericError","desc":"Invalid job ID ''"}}
at org.apache.cloudstack.storage.vmsnapshot.StorageVMSnapshotStrategy.takeVMSnapshot(StorageVMSnapshotStrategy.java:234)
at com.cloud.vm.snapshot.VMSnapshotManagerImpl.orchestrateCreateVMSnapshot(VMSnapshotManagerImpl.java:566)
at com.cloud.vm.snapshot.VMSnapshotManagerImpl.orchestrateCreateVMSnapshot(VMSnapshotManagerImpl.java:1249)
```
--
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
[GitHub] [cloudstack] weizhouapache commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r671030918
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1525,22 +1606,62 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ String queryBlockResult = vm.qemuMonitorCommand(new Gson().toJson(QemuCommand.executeQemuCommand(QemuCommand.QEMU_BLOCK, null)).toString(), 0);
+ String path = disk.getPath();
+ String snapshotDestPath = primaryPool.getLocalPath() + File.separator + "tmp";
+ if (queryBlockResult != null) {
Review comment:
@slavkap it is better to use org.apache.commons.lang.StringUtils.isNotBlank in line 1616 (queryBlockResult) and line 1618 (deviceName)
--
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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-864827359
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881239648
@slavkap @nvazquez
I believe this pr is good.
Unfortunately when I tested on ubuntu 20.04, it fails.
the reason is, ubuntu 20.04 uses -blockdev instead -device when start a vm via libvirt.
```
# on ubuntu 20.04
# virsh qemu-monitor-command i-2-85-VM '{"execute":"query-block"}' --pretty |jq -r ".return[].device"
```
same issue as http://qemu.11.n7.nabble.com/query-block-command-is-missing-device-name-Deprecated-td714506.html
I tried with code change below, still does not work
```
diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java
index 8461d31f08..6766291eaf 100644
--- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java
+++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java
@@ -57,6 +57,9 @@ public class QemuCommand {
if (arr.get(i).getAsJsonObject().has("inserted") && arr.get(i).getAsJsonObject().get("inserted").isJsonObject()) {
JsonObject inserted = arr.get(i).getAsJsonObject().get("inserted").getAsJsonObject();
if (inserted.get("file").getAsString().contains(path)) {
+ if (org.apache.commons.lang.StringUtils.isBlank(deviceName)) {
+ return inserted.get("node-name").getAsString();
+ }
return deviceName;
}
}
```
on centos7 it works.
```
# on centos
# virsh qemu-monitor-command i-2-5-VM '{"execute":"query-block"}' --pretty |jq -r '.return[].device'
drive-virtio-disk0
drive-ide0-1-0
```
--
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
[GitHub] [cloudstack] weizhouapache commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-884094982
@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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070864054
@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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r837117010
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,104 @@
+//
+// 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 org.apache.cloudstack.utils.qemu.QemuCommand;
+import org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.google.gson.Gson;
+import com.google.gson.JsonParser;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption(),
+ vmName, domainState));
+ }
+
+ String result = getResultOfQemuCommand(command.getOption(), domain);
+ s_logger.debug(String.format("Result of %s command is %s", command.getOption(), result));
+ if (result == null || (result.startsWith("error"))) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s vm %s due to result status is: %s",
+ command.getOption(), vmName, result));
+ }
+ String status = getResultOfQemuCommand(FreezeThawVMCommand.STATUS, domain);
+ s_logger.debug(String.format("Status of %s command is %s", command.getOption(), status));
+ if (status != null && new JsonParser().parse(status).isJsonObject()) {
+ String statusResult = new JsonParser().parse(status).getAsJsonObject().get("return").getAsString();
+ if (statusResult.equals(command.getOption())) {
+ return new FreezeThawVMAnswer(command, true, String.format("%s of VM - %s is successful", command.getOption(), vmName));
+ }
+ }
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s vm %s due to result status is: %s",
+ command.getOption(), vmName, status));
+ } catch (LibvirtException libvirtException) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s VM - %s due to %s",
+ command.getOption(), vmName, libvirtException.getMessage()));
+ }finally {
+ if (domain != null) {
+ try {
+ domain.free();
+ } catch (LibvirtException e) {
+ s_logger.trace("Ingore error ", e);
+ }
+ }
+ }
+ }
+
+ private String getResultOfQemuCommand(String cmd, Domain domain) throws LibvirtException {
+ String result = null;
+ if (cmd.equals(FreezeThawVMCommand.FREEZE)) {
+ result = domain.qemuAgentCommand(new Gson().toJson(QemuCommand.executeQemuCommand(QemuCommand.AGENT_FREEZE, null)).toString(), 10, 0);
Review comment:
@nvazquez, I prefer to leave it like this. In the feature, we could reuse this for commands that accept arguments, and for me, it will be more readable than assembling different strings.
--
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725381623
> Unfortunately I could not reproduce the failure. According to the logs the create snapshot has failed on execution of qemu-monitor-command "drive-backup". I could add to log the result, but not sure if you rerun the tests it will fail again. If I add the result in log, is it possible to run only this test - test_vm_snapshot_kvm.py?
not from here, but i started it from our lab. will let you know.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r518743917
##########
File path: api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java
##########
@@ -31,7 +31,7 @@
enum State {
Allocated("The VM snapshot is allocated but has not been created yet."), Creating("The VM snapshot is being created."), Ready(
"The VM snapshot is ready to be used."), Reverting("The VM snapshot is being used to revert"), Expunging("The volume is being expunging"), Removed(
- "The volume is destroyed, and can't be recovered."), Error("The volume is in error state, and can't be recovered");
+ "The volume is destroyed, and can't be recovered."), CreatingKVM("Creating KVM VM snapshot"), Error("The volume is in error state, and can't be recovered");
Review comment:
removed the new state and events related to it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r517861928
##########
File path: api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java
##########
@@ -31,7 +31,7 @@
enum State {
Allocated("The VM snapshot is allocated but has not been created yet."), Creating("The VM snapshot is being created."), Ready(
"The VM snapshot is ready to be used."), Reverting("The VM snapshot is being used to revert"), Expunging("The volume is being expunging"), Removed(
- "The volume is destroyed, and can't be recovered."), Error("The volume is in error state, and can't be recovered");
+ "The volume is destroyed, and can't be recovered."), CreatingKVM("Creating KVM VM snapshot"), Error("The volume is in error state, and can't be recovered");
Review comment:
VM snapshot states should be also generic. Can this be addressed in any other way?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-920590375
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + xenserver-71) 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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-882684788
Thanks, @weizhouapache. I think that the problem is fixed. I've tested it only on agents with CentOS7 but did the tests with qemu-monitor-command on my workstation (Ubuntu), and it's working with the solution from the link you shared
--
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-713357797
and @weizhouapache can you give this a look as well?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881241350
@weizhouapache a Jenkins job has been kicked to build packages. 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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-882684788
Thanks, @weizhouapache. I think that the problem is fixed. I've tested it only on agents with CentOS7 but did the tests with qemu-monitor-command on my workstation (Ubuntu), and it's working with the solution from the link you shared
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881849107
<b>Trillian Build Failed (tid-1296)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-698187570
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881275374
@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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r487055830
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725358536
Unfortunately I could not reproduce the failure. According to the logs the create snapshot has failed on execution of qemu-monitor-command "drive-backup". I could add to log the result, but not sure if you rerun the tests it will fail again. If I add the result in log, is it possible to run only this test - test_vm_snapshot_kvm.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-868282523
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881275535
@weizhouapache a 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
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r517861408
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedKVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
@slavkap Snapshot states should be generic, irrespective of hypervisor. Any other thoughts?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070911919
<b>Trillian Build Failed (tid-3653)<b/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070819844
@nvazquez a 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1071105429
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2921
--
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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r487055111
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-880689460
@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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-863801301
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r487056021
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] alamintech commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
alamintech commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-692517896
ok, Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd closed pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-713114372
<b>Trillian test result (tid-3012)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41009 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t3012-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshot_kvm.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 83 look OK, 3 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
runTest | `Error` | 0.00 | test_vm_snapshot_kvm.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 295.62 | test_vpc_redundant.py
test_hostha_kvm_host_degraded | `Error` | 5.68 | test_hostha_kvm.py
test_hostha_kvm_host_fencing | `Error` | 184.07 | test_hostha_kvm.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-711897866
@DaanHoogland a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] alamintech commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
alamintech commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-692390561
I have some problem. I am used ceph storage but after create vm then reboot ceph server osd service can't start. How can I solve this. this is ceph issue or cloudstack issue ? I am not sure.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r682509925
##########
File path: api/src/main/java/com/cloud/storage/Snapshot.java
##########
@@ -48,7 +48,7 @@ public boolean equals(String snapshotType) {
}
public enum State {
- Allocated, Creating, CreatedOnPrimary, BackingUp, BackedUp, Copying, Destroying, Destroyed,
+ Allocated, AllocatedVM, Creating, CreatingForVM, CreatedOnPrimary, CreatedOnPrimaryForVM, BackingUp, BackingUpForVM, BackedUp, BackedUpForVM, Copying, Destroying, Destroyed,
Review comment:
> * Does the snapshot operation here triggered through VM snapshot operation (as there are changes in VM snapshot strategies)?
Yes, it's triggered through the VM snapshot operation.
>
> * Are the actual volume snapshots taken (for the volumes of the VM) are maintained at volume level (in snapshots table)?
>
They are maintained mostly at the VM level. You will see a volume snapshot, but you don't have any options to operate separately on it.
> * Does the snapshots taken here are listed with VM snapshots or volume snapshots?
You'll see both a VM snapshot and also a volume snapshot for each volume of the group snapshot.
--
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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-884015378
Thank you, @weizhouapache, for your time to test this!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-920588989
@blueorangutan test centos7 xenserver-71
--
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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r671106503
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1525,22 +1606,62 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ String queryBlockResult = vm.qemuMonitorCommand(new Gson().toJson(QemuCommand.executeQemuCommand(QemuCommand.QEMU_BLOCK, null)).toString(), 0);
+ String path = disk.getPath();
+ String snapshotDestPath = primaryPool.getLocalPath() + File.separator + "tmp";
+ if (queryBlockResult != null) {
Review comment:
Thanks, @weizhouapache, for the code review! I'll change this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] weizhouapache closed pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881797478
<b>Trillian test result (tid-1289)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 60855 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t1289-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_iso.py
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vms_with_varied_deploymentplanners.py
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_with_userdata.py
Intermittent failure detected: /marvin/tests/smoke/test_diagnostics.py
Intermittent failure detected: /marvin/tests/smoke/test_direct_download.py
Intermittent failure detected: /marvin/tests/smoke/test_domain_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_domain_vpc_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
Intermittent failure detected: /marvin/tests/smoke/test_migration.py
Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_network.py
Intermittent failure detected: /marvin/tests/smoke/test_nic.py
Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
Intermittent failure detected: /marvin/tests/smoke/test_persistent_network.py
Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_projects.py
Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
Intermittent failure detected: /marvin/tests/smoke/test_resource_accounting.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dns.py
Intermittent failure detected: /marvin/tests/smoke/test_router_dnsservice.py
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Smoke tests completed. 60 look OK, 29 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_internallb_roundrobin_1VPC_3VM_HTTP_port80 | `Failure` | 4.58 | test_internal_lb.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 8.79 | test_internal_lb.py
test_03_vpc_internallb_haproxy_stats_on_all_interfaces | `Failure` | 6.58 | test_internal_lb.py
test_04_rvpc_internallb_haproxy_stats_on_all_interfaces | `Failure` | 7.60 | test_internal_lb.py
ContextSuite context=TestLoadBalance>:setup | `Error` | 0.00 | test_loadbalance.py
test_03_create_vpc_domain_vpc_offering | `Error` | 8.03 | test_domain_vpc_offerings.py
test_09_project_suspend | `Error` | 2.26 | test_projects.py
test_10_project_activation | `Error` | 2.20 | test_projects.py
test_deploy_vm_from_iso | `Error` | 2452.84 | test_deploy_vm_iso.py
test_list_vms_metrics | `Error` | 1.40 | test_metrics_api.py
test_00_deploy_vm_root_resize | `Error` | 1431.02 | test_deploy_vm_root_resize.py
test_deployvm_firstfit | `Error` | 28.68 | test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userconcentrated | `Error` | 1.16 | test_deploy_vms_with_varied_deploymentplanners.py
test_deployvm_userdispersing | `Error` | 1.17 | test_deploy_vms_with_varied_deploymentplanners.py
test_03_deploy_vm_domain_service_offering | `Error` | 8.13 | test_domain_service_offerings.py
test_01_native_to_native_network_migration | `Error` | 9.91 | test_migration.py
test_02_native_to_native_vpc_migration | `Error` | 6.69 | test_migration.py
test_deployvm_userdata | `Error` | 1.28 | test_deploy_vm_with_userdata.py
test_deployvm_userdata_post | `Error` | 1.22 | test_deploy_vm_with_userdata.py
test_network_acl | `Error` | 6.27 | test_network_acl.py
ContextSuite context=TestRemoteDiagnostics>:setup | `Error` | 0.00 | test_diagnostics.py
test_delete_account | `Error` | 16.89 | test_network.py
test_delete_network_while_vm_on_it | `Error` | 2.25 | test_network.py
test_delete_network_while_vm_on_it | `Error` | 2.26 | test_network.py
test_deploy_vm_l2network | `Error` | 2.23 | test_network.py
test_deploy_vm_l2network | `Error` | 2.23 | test_network.py
test_l2network_restart | `Error` | 3.33 | test_network.py
test_l2network_restart | `Error` | 3.33 | test_network.py
ContextSuite context=TestL2Networks>:teardown | `Error` | 4.41 | test_network.py
ContextSuite context=TestPortForwarding>:setup | `Error` | 6.06 | test_network.py
ContextSuite context=TestPublicIP>:setup | `Error` | 2.19 | test_network.py
test_reboot_router | `Error` | 1.65 | test_network.py
test_releaseIP | `Error` | 1.63 | test_network.py
ContextSuite context=TestRouterRules>:setup | `Error` | 3.30 | test_network.py
test_01_deployVMInSharedNetwork | `Failure` | 1.15 | test_network.py
test_02_verifyRouterIpAfterNetworkRestart | `Failure` | 1.06 | test_network.py
test_03_destroySharedNetwork | `Failure` | 1.06 | test_network.py
ContextSuite context=TestSharedNetwork>:teardown | `Error` | 2.14 | test_network.py
ContextSuite context=TestDirectDownloadTemplates>:setup | `Error` | 0.00 | test_direct_download.py
test_01_nic | `Error` | 50.40 | test_nic.py
test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_02_deploy_and_upgrade_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_03_deploy_and_scale_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_04_basic_lifecycle_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_06_deploy_invalid_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster | `Failure` | 0.00 | test_kubernetes_clusters.py
ContextSuite context=TestListIdsParams>:setup | `Error` | 0.00 | test_list_ids_parameter.py
test_nic_secondaryip_add_remove | `Error` | 16.81 | test_multipleips_per_nic.py
ContextSuite context=TestIsolatedNetworksPasswdServer>:setup | `Error` | 0.00 | test_password_server.py
test_01_isolated_persistent_network | `Error` | 4.03 | test_persistent_network.py
test_02_L2_persistent_network | `Error` | 1.17 | test_persistent_network.py
test_03_deploy_and_destroy_VM_and_verify_network_resources_persist | `Failure` | 2.27 | test_persistent_network.py
test_03_deploy_and_destroy_VM_and_verify_network_resources_persist | `Error` | 2.27 | test_persistent_network.py
ContextSuite context=TestL2PersistentNetworks>:teardown | `Error` | 2.31 | test_persistent_network.py
test_01_create_delete_portforwarding_fornonvpc | `Error` | 1.80 | test_portforwardingrules.py
test_01_add_primary_storage_disabled_host | `Error` | 0.14 | test_primary_storage.py
test_01_primary_storage_nfs | `Error` | 0.10 | test_primary_storage.py
ContextSuite context=TestStorageTags>:setup | `Error` | 0.19 | test_primary_storage.py
test_01_vpc_privategw_acl | `Failure` | 4.62 | test_privategw_acl.py
test_02_vpc_privategw_static_routes | `Failure` | 4.59 | test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 5.64 | test_privategw_acl.py
test_04_rvpc_privategw_static_routes | `Failure` | 7.65 | test_privategw_acl.py
ContextSuite context=TestResetVmOnReboot>:setup | `Error` | 0.00 | test_reset_vm_on_reboot.py
ContextSuite context=TestRAMCPUResourceAccounting>:setup | `Error` | 0.00 | test_resource_accounting.py
ContextSuite context=TestRouterDHCPHosts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
ContextSuite context=TestRouterDHCPOpts>:setup | `Error` | 0.00 | test_router_dhcphosts.py
ContextSuite context=TestRouterDns>:setup | `Error` | 0.00 | test_router_dns.py
test_05_stop_ssvm | `Failure` | 51.51 | test_ssvm.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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070958046
@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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1071966610
<b>Trillian test result (tid-3656)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30801 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t3656-kvm-centos7.zip
Smoke tests completed. 93 look OK, 0 have errors
Only failed 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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1071131264
@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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070819396
--
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
[GitHub] [cloudstack] nvazquez edited a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez edited a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1075280179
Thanks @rp- can you please approve on the Files changed tab -> Review changes -> Submit review?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez edited a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez edited a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1083212161
Hi @wido @GabrielBrascher @svenvogel would it be possible for you to test this PR on Ceph or Solidfire?
--
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
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r837374559
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,104 @@
+//
+// 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 org.apache.cloudstack.utils.qemu.QemuCommand;
+import org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.google.gson.Gson;
+import com.google.gson.JsonParser;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption(),
+ vmName, domainState));
+ }
+
+ String result = getResultOfQemuCommand(command.getOption(), domain);
+ s_logger.debug(String.format("Result of %s command is %s", command.getOption(), result));
+ if (result == null || (result.startsWith("error"))) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s vm %s due to result status is: %s",
+ command.getOption(), vmName, result));
+ }
+ String status = getResultOfQemuCommand(FreezeThawVMCommand.STATUS, domain);
+ s_logger.debug(String.format("Status of %s command is %s", command.getOption(), status));
+ if (status != null && new JsonParser().parse(status).isJsonObject()) {
+ String statusResult = new JsonParser().parse(status).getAsJsonObject().get("return").getAsString();
+ if (statusResult.equals(command.getOption())) {
+ return new FreezeThawVMAnswer(command, true, String.format("%s of VM - %s is successful", command.getOption(), vmName));
+ }
+ }
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s vm %s due to result status is: %s",
+ command.getOption(), vmName, status));
+ } catch (LibvirtException libvirtException) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s VM - %s due to %s",
+ command.getOption(), vmName, libvirtException.getMessage()));
+ }finally {
+ if (domain != null) {
+ try {
+ domain.free();
+ } catch (LibvirtException e) {
+ s_logger.trace("Ingore error ", e);
+ }
+ }
+ }
+ }
+
+ private String getResultOfQemuCommand(String cmd, Domain domain) throws LibvirtException {
+ String result = null;
+ if (cmd.equals(FreezeThawVMCommand.FREEZE)) {
+ result = domain.qemuAgentCommand(new Gson().toJson(QemuCommand.executeQemuCommand(QemuCommand.AGENT_FREEZE, null)).toString(), 10, 0);
Review comment:
@slavkap ok, since qemuAgentCommand accepts a string as a first parameter I would be +1 on having a helper class or similar that will retrieve the string in the expected format based on the params, but not a blocker
--
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
[GitHub] [cloudstack] nvazquez commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r837376217
##########
File path: server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java
##########
@@ -59,6 +59,8 @@
public static final ConfigKey<Boolean> BackupSnapshotAfterTakingSnapshot = new ConfigKey<Boolean>(Boolean.class, "snapshot.backup.to.secondary", "Snapshots", "true",
"Indicates whether to always backup primary storage snapshot to secondary storage. Keeping snapshots only on Primary storage is applicable for KVM + Ceph only.", false, ConfigKey.Scope.Global, null);
+ public static final ConfigKey<Boolean> VMsnapshotKVM = new ConfigKey<>(Boolean.class, "kvm.vmstoragesnapshot.enabled", "Snapshots", "false", "For live snapshot of virtual machine instance on KVM hypervisor without memory. Requieres qemu version 1.6+ (on NFS or Local file system) and qemu-guest-agent installed on guest VM", true, ConfigKey.Scope.Global, null);
Review comment:
@slavkap I was thinking it would make sense for admins to enable/disable the feature for a zone and not for all of them at the same time - just a suggestion
--
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
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1083212161
Hi @wido @GabrielBrascher @svenvogel would it be possible for you to test this PR on Ceph?
--
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-709532351
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r487055111
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtFreezeThawVMCommandWrapper.java
##########
@@ -0,0 +1,92 @@
+//
+// 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 org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.DomainInfo.DomainState;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.FreezeThawVMAnswer;
+import com.cloud.agent.api.FreezeThawVMCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.utils.script.Script;
+
+@ResourceWrapper(handles = FreezeThawVMCommand.class)
+public class LibvirtFreezeThawVMCommandWrapper extends CommandWrapper<FreezeThawVMCommand, Answer, LibvirtComputingResource> {
+
+ private static final Logger s_logger = Logger.getLogger(LibvirtFreezeThawVMCommandWrapper.class);
+
+ @Override
+ public Answer execute(FreezeThawVMCommand command, LibvirtComputingResource serverResource) {
+ String vmName = command.getVmName();
+ Domain domain = null;
+
+ try {
+ final LibvirtUtilitiesHelper libvirtUtilitiesHelper = serverResource.getLibvirtUtilitiesHelper();
+ Connect connect = libvirtUtilitiesHelper.getConnection();
+ domain = serverResource.getDomain(connect, vmName);
+ if (domain == null) {
+ return new FreezeThawVMAnswer(command, false, String.format("Failed to %s due to %s was not found",
+ command.getOption().toUpperCase(), vmName));
+ }
+ DomainState domainState = domain.getInfo().state ;
+ if (domainState != DomainState.VIR_DOMAIN_RUNNING) {
+ return new FreezeThawVMAnswer(command, false,
+ String.format("%s of VM failed due to vm %s is in %s state", command.getOption().toUpperCase(),
+ vmName, domainState));
+ }
+ Script cmd = new Script("virsh", s_logger);
Review comment:
replaced with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -947,25 +953,57 @@ public Answer backupSnapshot(final CopyCommand cmd) {
return new CopyCmdAnswer(e.toString());
}
} else {
- final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
- command.add("-b", snapshotDisk.getPath());
- command.add("-n", snapshotName);
- command.add("-p", snapshotDestPath);
- if (isCreatedFromVmSnapshot) {
- descName = UUID.randomUUID().toString();
- }
- command.add("-t", descName);
- final String result = command.execute();
- if (result != null) {
- s_logger.debug("Failed to backup snaptshot: " + result);
- return new CopyCmdAnswer(result);
- }
- final File snapFile = new File(snapshotDestPath + "/" + descName);
- if(snapFile.exists()){
- size = snapFile.length();
+ if (isKVMEnabled) {
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block-jobs\" }'", vmName));
Review comment:
here also is replaced the use of virsh with libvirt's Java API
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1456,22 +1496,65 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
final KVMPhysicalDisk disk = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), volume.getPath());
if (state == DomainInfo.DomainState.VIR_DOMAIN_RUNNING && !primaryPool.isExternalSnapshot()) {
final String vmUuid = vm.getUUIDString();
- final Object[] args = new Object[] {snapshotName, vmUuid};
- final String snapshot = SnapshotXML.format(args);
+ boolean isKVMEnabled = cmd.getContextParam("kvmsnapshot") != null ? Boolean.parseBoolean(cmd.getContextParam("kvmsnapshot")) : false;
+ s_logger.debug(String.format("Snapshots on KVM is enabled %s", isKVMEnabled));
+ if (isKVMEnabled) {
+ long size = 0;
+ OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser();
+ Script sc = new Script("virsh");
+ sc.add(String.format("qemu-monitor-command %s '{ \"execute\" : \"query-block\" }'", vmName));
Review comment:
replaced with libvirt's Java bindings
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-725953323
@DaanHoogland a Jenkins job has been kicked to build packages. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] wido commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
wido commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-632587405
Great news @slavkap ! We should now request a new release of libvirt-java in Maven so we can depend on 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-870109568
@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
[GitHub] [cloudstack] sureshanaparti commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-924862408
@slavkap As discussed offline, Can you try to create the VM Snapshot as a volume group snapshot (with different snapshot type), maintaining the same (current) states of snapshot. This way, the same snapshots state machine, db table can be re-used and try not to list the snapshots which are part of volume group snapshot.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-918852225
@rhtyd a Jenkins job has been kicked to build packages. 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
[GitHub] [cloudstack] weizhouapache closed pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724
--
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
[GitHub] [cloudstack] weizhouapache commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881240834
@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
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-726036845
> > @slavkap the env was destroyed so running the full suit. if you want to run just that test, you are a bit on your own.
>
> Thanks @DaanHoogland I did not have bad intentions with the question whatever you could rerun only this test. I just wanted to check the result without to wait all tests to be executed. And if it fails again with this setup, what is the reason. Anyway I know that I'm on my own, but I've executed this test so many times on my dev, without any failures
We don't have that limiting feature but I am working on getting the testrun smaller, that won't be for 4.15 though
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-724924507
@DaanHoogland a Jenkins job has been kicked to build packages. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-921440315
Tests LGTM. Ping @sureshanaparti can you help review this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-918851942
@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
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-709532612
@DaanHoogland a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-702180412
@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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-724969473
Packaging result: ✔centos7 ✖centos8 ✖debian. JID-2355
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-881313085
Thank you, @weizhouapache, for testing this PR! I will research for a fix of that problem, but unfortunately, all of my dev. environments are with centos7
--
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-864092438
> Hi @rhtyd, @DaanHoogland, sorry to bother you both, but what are the versions of qemu/ libvirt on the agents? I'm pretty sure that creating snapshot was passing before, but now it can't find the command 'drive-backup', which is included in qemu-kvm-ev
I didn't review @slavkap , is that a new command used? If so it means we are putting more limits on what versions can be used. I don't know the version, I'll have a look in some running env.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti closed pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
sureshanaparti closed pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-870578078
<b>Trillian test result (tid-1135)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43891 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t1135-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 87 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_isolate_network_FW_PF_default_routes_egress_true | `Failure` | 103.41 | test_routers_network_ops.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 616.08 | test_vpc_redundant.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Error` | 616.10 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 545.86 | test_vpc_redundant.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
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-871150269
Hi all, this PR is ready for review. I'll appreciate it if somebody has time to test it.
According to the test results, I think they are not related to this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-884040176
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 616
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-863625713
<b>Trillian test result (tid-972)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 52795 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3724-t972-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
Intermittent failure detected: /marvin/tests/smoke/test_network.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshot_kvm.py
Smoke tests completed. 86 look OK, 3 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Error` | 605.67 | test_internal_lb.py
test_01_invalid_upgrade_kubernetes_cluster | `Failure` | 3611.20 | test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 108.49 | test_kubernetes_clusters.py
test_01_create_vm_snapshots | `Error` | 31.83 | test_vm_snapshot_kvm.py
test_02_revert_vm_snapshots | `Failure` | 30.13 | test_vm_snapshot_kvm.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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-713830188
@DaanHoogland this PR looks like a good one to get into 4.15. I will help to review/test it :slightly_smiling_face:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-724972385
@DaanHoogland a 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] slavkap commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-985324189
Hi @rhtyd, we discussed with @sureshanaparti that I should make a few changes to the snapshot states. I'm working on them, and it will be great if #5297 gets in first. With it, I won't limit this PR with the functionality for NFS
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1071050049
@nvazquez a 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070846459
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2917
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1071133287
@nvazquez a 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
[GitHub] [cloudstack] blueorangutan commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1070819844
--
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
[GitHub] [cloudstack] slavkap commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
slavkap commented on a change in pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#discussion_r837113539
##########
File path: plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuCommand.java
##########
@@ -0,0 +1,44 @@
+/*
+ * 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.utils.qemu;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+public class QemuCommand {
+ //Qemu agent commands
+ public static final String AGENT_FREEZE = "guest-fsfreeze-freeze";
+ public static final String AGENT_THAW = "guest-fsfreeze-thaw";
+ public static final String AGENT_FREEZE_STATUS = "guest-fsfreeze-status";
+ //Qemu monitor commands
+ public static final String QEMU_QUERY_BLOCK_JOBS = "query-block-jobs";
+ public static final String QEMU_BLOCK = "query-block";
+ public static final String QEMU_DRIVE_BACKUP = "drive-backup";
+
+ public static final String QEMU_CMD = "execute";
+
+ public static Map<String, Object> executeQemuCommand(String command, Map<String, String> args ){
Review comment:
Thanks, @nvazquez, for the review! I will rename it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rp- commented on pull request #3724: Storage-based Snapshots for KVM VMs
Posted by GitBox <gi...@apache.org>.
rp- commented on pull request #3724:
URL: https://github.com/apache/cloudstack/pull/3724#issuecomment-1075199740
I tested this PR today with the Linstor plugin and creating/reverting and deleting works with Linstor.
--
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