wido commented on a change in pull request #3724: Storage-based Snapshots for KVM VMs URL: https://github.com/apache/cloudstack/pull/3724#discussion_r366370846
########## 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: All the calls to libvirt from the agent are done through the Java bindings. If we suddenly start to execute virsh from Java we work around all the existing code which is there to have an open connection with libvirt. In addition you can only check on exit status and not handle exceptions properly coming back from libvirt. If we are lacking features in libvirt-java those need to be added through the libvirt project. ---------------------------------------------------------------- This is an automated message from the 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: us...@infra.apache.org With regards, Apache Git Services