You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "harikrishna-patnala (via GitHub)" <gi...@apache.org> on 2026/04/07 04:43:50 UTC

[PR] Added vddk support in vmware to kvm migrations [cloudstack]

harikrishna-patnala opened a new pull request, #12970:
URL: https://github.com/apache/cloudstack/pull/12970

   ### Description
   
   This PR introduces the VDDK implementation as part of VMware to KVM migration which reduces the migration time by half when compared to the existing implementation with exportOVF
   
   
   <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ******************************************************************************* -->
   <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ******************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [ ] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   - [ ] Build/CI
   - [ ] Test (unit or integration test code)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [ ] Major
   - [ ] Minor
   - [ ] Trivial
   
   ### Screenshots (if appropriate):
   
   ### How Has This Been Tested?
   
   <!-- Please describe in detail how you tested your changes. -->
   <!-- Include details of your testing environment, and the tests you ran to -->
   
   #### How did you try to break this feature and the system with this change?
   
   <!-- see how your change affects other areas of the code, etc. -->
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4213633638

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17421


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049189173


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java:
##########
@@ -255,6 +263,10 @@ public Long getStoragePoolId() {
         return storagePoolId;
     }
 
+    public boolean getUseVddk() {
+        return BooleanUtils.toBooleanDefaultIfNull(useVddk, false);

Review Comment:
   ```suggestion
           return BooleanUtils.toBooleanDefaultIfNull(useVddk, true);
   ```
   
   defaulted to true



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3055451340


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -84,61 +91,76 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve
         logger.info(String.format("(%s) Attempting to convert the instance %s from %s to KVM",
                 originalVMName, sourceInstanceName, sourceHypervisorType));
         final String temporaryConvertPath = temporaryStoragePool.getLocalPath();
+        final String temporaryConvertUuid = UUID.randomUUID().toString();
+        boolean verboseModeEnabled = serverResource.isConvertInstanceVerboseModeEnabled();
 
-        String ovfTemplateDirOnConversionLocation;
-        String sourceOVFDirPath;
+        boolean cleanupSecondaryStorage = false;
         boolean ovfExported = false;
-        if (cmd.getExportOvfToConversionLocation()) {
-            String exportInstanceOVAUrl = getExportInstanceOVAUrl(sourceInstance, originalVMName);
-            if (StringUtils.isBlank(exportInstanceOVAUrl)) {
-                String err = String.format("Couldn't export OVA for the VM %s, due to empty url", sourceInstanceName);
-                logger.error(String.format("(%s) %s", originalVMName, err));
-                return new Answer(cmd, false, err);
-            }
+        String ovfTemplateDirOnConversionLocation = null;
 
-            int noOfThreads = cmd.getThreadsCountToExportOvf();
-            if (noOfThreads > 1 && !serverResource.ovfExportToolSupportsParallelThreads()) {
-                noOfThreads = 0;
-            }
-            ovfTemplateDirOnConversionLocation = UUID.randomUUID().toString();
-            temporaryStoragePool.createFolder(ovfTemplateDirOnConversionLocation);
-            sourceOVFDirPath = String.format("%s/%s/", temporaryConvertPath, ovfTemplateDirOnConversionLocation);
-            ovfExported = exportOVAFromVMOnVcenter(exportInstanceOVAUrl, sourceOVFDirPath, noOfThreads, originalVMName, timeout);
-            if (!ovfExported) {
-                String err = String.format("Export OVA for the VM %s failed", sourceInstanceName);
-                logger.error(String.format("(%s) %s", originalVMName, err));
-                return new Answer(cmd, false, err);
-            }
-            sourceOVFDirPath = String.format("%s%s/", sourceOVFDirPath, sourceInstanceName);
-        } else {
-            ovfTemplateDirOnConversionLocation = cmd.getTemplateDirOnConversionLocation();
-            sourceOVFDirPath = String.format("%s/%s/", temporaryConvertPath, ovfTemplateDirOnConversionLocation);
-        }
+        try {
+            boolean result;
+            if (useVddk) {
+                logger.info("({}) Using VDDK-based conversion (direct from VMware)", originalVMName);
+                String vddkLibDir = resolveVddkSetting(cmd.getVddkLibDir(), serverResource.getVddkLibDir());
+                if (StringUtils.isBlank(vddkLibDir)) {
+                    String err = String.format("VDDK lib dir is not configured on the host. " +
+                            "Set '%s' in agent.properties to use VDDK-based conversion.", "vddk.lib.dir");

Review Comment:
   ```suggestion
                               "Set '%s' in agent.properties or in details parameter of the import api calll to use VDDK-based conversion.", "vddk.lib.dir");
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4213294934

   @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3077492241


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +296,196 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionUsingVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                         String temporaryConvertFolder, String vddkLibDir,
+                                                         String libguestfsBackend, String vddkTransports,
+                                                         String configuredVddkThumbprint,
+                                                         long timeout, boolean verboseModeEnabled, String extraParams,
+                                                         String temporaryConvertUuid, String passwordOption) {
+
+        String vcenterPassword = vmwareInstance.getVcenterPassword();
+        if (StringUtils.isBlank(vcenterPassword)) {
+            logger.error("({}) Could not determine vCenter password for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+
+        String passwordFilePath = String.format("/tmp/v2v.pass.cloud.%s.%s",
+                StringUtils.defaultIfBlank(vmwareInstance.getVcenterHost(), "unknown"),
+                UUID.randomUUID());
+        try {
+            Files.writeString(Path.of(passwordFilePath), vcenterPassword);
+            logger.debug("({}) Written vCenter password to {}", originalVMName, passwordFilePath);

Review Comment:
   file permission is missing, any issues with it?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4212979689

   @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049358018


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1156,6 +1178,21 @@ public boolean configure(final String name, final Map<String, Object> params) th
 
         setConvertInstanceEnv(convertEnvTmpDir, convertEnvVirtv2vTmpDir);
 
+        vddkLibDir = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VDDK_LIB_DIR);

Review Comment:
   can auto detect vddk lib path with some standard path for it and set it in agent.properties when vddk is installed?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049259268


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +299,194 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                    String temporaryConvertFolder, String vddkLibDir,
+                                                    String libguestfsBackend, String vddkTransports,
+                                                    String configuredVddkThumbprint,
+                                                    long timeout, boolean verboseModeEnabled, String extraParams,
+                                                    String temporaryConvertUuid, String passwordOption) {
+
+        String vcenterPassword = vmwareInstance.getVcenterPassword();
+        if (StringUtils.isBlank(vcenterPassword)) {
+            logger.error("({}) Could not determine vCenter password for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+
+        String passwordFilePath = String.format("/root/v2v.pass.cloud.%s",

Review Comment:
   use random file for each conversion as before, check below code ref. multiple conversions from the same vcenter would use the same hard-coded file, and earlier conversion can remove the file after conversion is finished.
   
   https://github.com/apache/cloudstack/blob/2746225b999612f156e421199e34ef8de98a3664/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java#L345-L355



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3077515143


##########
server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java:
##########
@@ -751,6 +761,35 @@ private void baseTestImportVmFromVmwareToKvm(VcenterParameter vcenterParameter,
         when(importVmCmd.getConvertInstanceHostId()).thenReturn(null);
         when(importVmCmd.getImportInstanceHostId()).thenReturn(null);
         when(importVmCmd.getConvertStoragePoolId()).thenReturn(null);
+        when(importVmCmd.getExistingVcenterId()).thenReturn(null);
+        when(importVmCmd.getVcenter()).thenReturn(null);
+        when(importVmCmd.getDatacenterName()).thenReturn(null);
+        when(importVmCmd.getUsername()).thenReturn(null);
+        when(importVmCmd.getPassword()).thenReturn(null);
+        when(importVmCmd.getDetails()).thenReturn(new HashMap<>());
+
+        boolean forceMsToImportVmFiles = false;
+        boolean useVddk = false;
+        boolean ovfExportSupported = false;
+        if (VcenterParameter.FORCE_MS_AND_USE_VDDK == vcenterParameter) {
+            forceMsToImportVmFiles = true;
+            useVddk = true;
+        } else if (VcenterParameter.USE_VDDK_OVF_UNSUPPORTED == vcenterParameter) {
+            useVddk = true;
+        } else if (VcenterParameter.USE_VDDK_OVF_SUPPORTED == vcenterParameter) {
+            useVddk = true;
+            ovfExportSupported = true;
+        } else if (VcenterParameter.USE_VDDK_DETAILS_OVERRIDES == vcenterParameter) {
+            useVddk = true;
+            ovfExportSupported = true;
+            when(importVmCmd.getDetails()).thenReturn(Map.of(
+                    "libguestfs.backend", "libvirt",

Review Comment:
   ```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


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3077522077


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +296,196 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionUsingVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                         String temporaryConvertFolder, String vddkLibDir,
+                                                         String libguestfsBackend, String vddkTransports,

Review Comment:
   ```suggestion
                                                            String vddkTransports,
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3213384911


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java:
##########
@@ -255,6 +263,10 @@ public Long getStoragePoolId() {
         return storagePoolId;
     }
 
+    public boolean getUseVddk() {
+        return BooleanUtils.toBooleanDefaultIfNull(useVddk, true);

Review Comment:
   @harikrishna-patnala @kiranchavala @rajujith IMO the default value of this boolean should be false. I'm not able to migrate VMs without VDDK installed anymore
   
   ````
   2026-05-09 15:51:17,643 ERROR [o.a.c.v.UnmanagedVMsManagerImpl] (API-Job-Executor-1:[ctx-961baab4, job-251, ctx-757448a4]) (logid:fd8b0e9d) The h
   ost Host {"id":1,"name":"ref-trl-11356-k-Mr9-nicolas-vazquez-kvm1","type":"Routing","uuid":"00504c23-253a-4c33-8268-1ba6c70aa837"} doesn't suppor
   t conversion of instance i-2-5-VM from VMware to KVM due to: Cannot convert the instance from VMware using VDDK on host 10.0.32.227. Please make 
   sure virt-v2v, nbdkit-vddk and a valid VDDK library directory are available on the host.
   ````



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4196864035

   @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4196879000

   @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049339110


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1785,6 +1798,41 @@ protected void checkConversionStoragePool(Long convertStoragePoolId, boolean for
         }
     }
 
+    protected void validateSelectedConversionStoragePoolForVddk(boolean useVddk, Long convertStoragePoolId,
+                                                                ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap) {
+        if (!useVddk || convertStoragePoolId == null) {
+            return;
+        }
+
+        StoragePoolVO selectedStoragePool = primaryDataStoreDao.findById(convertStoragePoolId);
+        if (selectedStoragePool == null) {
+            return;
+        }
+
+        if (serviceOffering.getDiskOfferingId() != null) {
+            DiskOfferingVO rootDiskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
+            if (rootDiskOffering == null) {
+                throw new InvalidParameterValueException(String.format("Cannot find disk offering with ID %s that belongs to the service offering %s",
+                        serviceOffering.getDiskOfferingId(), serviceOffering.getName()));
+            }
+            if (!volumeApiService.doesStoragePoolSupportDiskOffering(selectedStoragePool, rootDiskOffering)) {
+                throw new InvalidParameterValueException("Using VDDK, multiple storage pools cannot be used if a storage pool is selected for conversion");
+            }
+        }
+
+        if (MapUtils.isNotEmpty(dataDiskOfferingMap)) {
+            for (Long diskOfferingId : dataDiskOfferingMap.values()) {
+                DiskOfferingVO diskOffering = diskOfferingDao.findById(diskOfferingId);
+                if (diskOffering == null) {
+                    throw new InvalidParameterValueException(String.format("Cannot find disk offering with ID %s", diskOfferingId));
+                }
+                if (!volumeApiService.doesStoragePoolSupportDiskOffering(selectedStoragePool, diskOffering)) {
+                    throw new InvalidParameterValueException("Using VDDK, multiple storage pools cannot be used if a storage pool is selected for conversion");

Review Comment:
   same as above for disk offerings.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4207412111

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17407


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3055575315


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -821,6 +824,26 @@ protected AgentAttache notifyMonitorsOfConnection(final AgentAttache attache, fi
                         host.getDetails().put(Host.HOST_OVFTOOL_VERSION, ovftoolVersion);
                         updateNeeded = true;
                     }
+                    if (StringUtils.isNotBlank(vddkSupport) && !vddkSupport.equals(host.getDetails().get(Host.HOST_VDDK_SUPPORT))) {
+                        host.getDetails().put(Host.HOST_VDDK_SUPPORT, vddkSupport);
+                        updateNeeded = true;
+                    }
+                    if (!StringUtils.defaultString(vddkLibDir).equals(StringUtils.defaultString(host.getDetails().get(Host.HOST_VDDK_LIB_DIR)))) {
+                        if (StringUtils.isBlank(vddkLibDir)) {
+                            host.getDetails().remove(Host.HOST_VDDK_LIB_DIR);
+                        } else {
+                            host.getDetails().put(Host.HOST_VDDK_LIB_DIR, vddkLibDir);
+                        }
+                        updateNeeded = true;
+                    }
+                    if (!StringUtils.defaultString(vddkVersion).equals(StringUtils.defaultString(host.getDetails().get(Host.HOST_VDDK_VERSION)))) {
+                        if (StringUtils.isBlank(vddkVersion)) {
+                            host.getDetails().remove(Host.HOST_VDDK_VERSION);
+                        } else {
+                            host.getDetails().put(Host.HOST_VDDK_VERSION, vddkVersion);
+                        }
+                        updateNeeded = true;

Review Comment:
   do we need all these params in host_details? Persisting Host.HOST_VDDK_VERSION should be enough when vddk lib is properly set, to indicate vddk 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4220672866

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4220688054

   @sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4226146908

   <b>[SF] Trillian test result (tid-15846)</b>
   Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
   Total time taken: 48548 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12970-t15846-kvm-ol8.zip
   Smoke tests completed. 149 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4241716613

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17479


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049179825


##########
agent/conf/agent.properties:
##########
@@ -457,3 +457,19 @@ iscsi.session.cleanup.enabled=false
 
 # Instance conversion VIRT_V2V_TMPDIR env var
 #convert.instance.env.virtv2v.tmpdir=
+
+# LIBGUESTFS backend to use for VMware to KVM conversion via VDDK (default: direct)

Review Comment:
   should keep other possible options in the comment (other than _direct_) ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3055508484


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1684,9 +1695,16 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
         String ovfTemplateOnConvertLocation = null;
         ImportVmTask importVMTask = null;
         try {
-            HostVO convertHost = selectKVMHostForConversionInCluster(destinationCluster, convertInstanceHostId);
-            HostVO importHost = selectKVMHostForImportingInCluster(destinationCluster, importInstanceHostId);
-            CheckConvertInstanceAnswer conversionSupportAnswer = checkConversionSupportOnHost(convertHost, sourceVMName, false);
+            HostVO convertHost = selectKVMHostForConversionInCluster(destinationCluster, convertInstanceHostId, useVddk);
+            HostVO importHost = (useVddk && importInstanceHostId == null)
+                    ? convertHost
+                    : selectKVMHostForImportingInCluster(destinationCluster, importInstanceHostId);
+
+            boolean isOvfExportSupported = false;
+            if (!useVddk) {
+                CheckConvertInstanceAnswer conversionSupportAnswer = checkConversionSupportOnHost(convertHost, sourceVMName, false);

Review Comment:
   can add conversion support check for vddk as well? if convert host is from the api cmd and it doesn't support vddk, fail early.
   
   auto selection of convert host (when convert host is not passed from the api cmd) should return null and no hosts with vddk 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3057249150


##########
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java:
##########
@@ -821,6 +824,26 @@ protected AgentAttache notifyMonitorsOfConnection(final AgentAttache attache, fi
                         host.getDetails().put(Host.HOST_OVFTOOL_VERSION, ovftoolVersion);
                         updateNeeded = true;
                     }
+                    if (StringUtils.isNotBlank(vddkSupport) && !vddkSupport.equals(host.getDetails().get(Host.HOST_VDDK_SUPPORT))) {
+                        host.getDetails().put(Host.HOST_VDDK_SUPPORT, vddkSupport);
+                        updateNeeded = true;
+                    }
+                    if (!StringUtils.defaultString(vddkLibDir).equals(StringUtils.defaultString(host.getDetails().get(Host.HOST_VDDK_LIB_DIR)))) {
+                        if (StringUtils.isBlank(vddkLibDir)) {
+                            host.getDetails().remove(Host.HOST_VDDK_LIB_DIR);
+                        } else {
+                            host.getDetails().put(Host.HOST_VDDK_LIB_DIR, vddkLibDir);
+                        }
+                        updateNeeded = true;
+                    }
+                    if (!StringUtils.defaultString(vddkVersion).equals(StringUtils.defaultString(host.getDetails().get(Host.HOST_VDDK_VERSION)))) {
+                        if (StringUtils.isBlank(vddkVersion)) {
+                            host.getDetails().remove(Host.HOST_VDDK_VERSION);
+                        } else {
+                            host.getDetails().put(Host.HOST_VDDK_VERSION, vddkVersion);
+                        }
+                        updateNeeded = true;

Review Comment:
   Version identification is not just enough to claim the support but we will also need the directory where it is located as we need to pass that in the virt-v2v command with vddk.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3063916663


##########
agent/src/main/java/com/cloud/agent/properties/AgentProperties.java:
##########
@@ -808,6 +808,37 @@ public Property<Integer> getWorkers() {
      */
     public static final Property<String> CONVERT_ENV_VIRTV2V_TMPDIR = new Property<>("convert.instance.env.virtv2v.tmpdir", null, String.class);
 
+    /**
+     * Path to the VDDK library directory on the KVM conversion host, used when converting VMs from VMware to KVM via VDDK.
+     * This directory is passed to virt-v2v as <code>-io vddk-libdir=&lt;path&gt;</code>.
+     * Data type: String.<br>
+     * Default value: <code>null</code>
+     */
+    public static final Property<String> VDDK_LIB_DIR = new Property<>("vddk.lib.dir", null, String.class);
+
+    /**
+     * Value for the LIBGUESTFS_BACKEND env var used during VMware to KVM conversion via VDDK.
+     * Data type: String.<br>
+     * Default value: <code>direct</code>
+     */
+    public static final Property<String> LIBGUESTFS_BACKEND = new Property<>("libguestfs.backend", "direct", String.class);

Review Comment:
   ```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


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4213289098

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4241480235

   @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4242643949

   @sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4241376116

   @sureshanaparti thanks for all the reviews, I've addressed 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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049336807


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1785,6 +1798,41 @@ protected void checkConversionStoragePool(Long convertStoragePoolId, boolean for
         }
     }
 
+    protected void validateSelectedConversionStoragePoolForVddk(boolean useVddk, Long convertStoragePoolId,
+                                                                ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap) {
+        if (!useVddk || convertStoragePoolId == null) {
+            return;
+        }
+
+        StoragePoolVO selectedStoragePool = primaryDataStoreDao.findById(convertStoragePoolId);
+        if (selectedStoragePool == null) {
+            return;
+        }
+
+        if (serviceOffering.getDiskOfferingId() != null) {
+            DiskOfferingVO rootDiskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
+            if (rootDiskOffering == null) {
+                throw new InvalidParameterValueException(String.format("Cannot find disk offering with ID %s that belongs to the service offering %s",
+                        serviceOffering.getDiskOfferingId(), serviceOffering.getName()));
+            }
+            if (!volumeApiService.doesStoragePoolSupportDiskOffering(selectedStoragePool, rootDiskOffering)) {
+                throw new InvalidParameterValueException("Using VDDK, multiple storage pools cannot be used if a storage pool is selected for conversion");

Review Comment:
   better rephrase the msg, to indicate that the selected compute offering picked different storage pool other than the selected pool for conversion.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049438941


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1662,9 +1668,16 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
         }
 
         checkConversionStoragePool(convertStoragePoolId, forceConvertToPool);
+        validateSelectedConversionStoragePoolForVddk(useVddk, convertStoragePoolId, serviceOffering, dataDiskOfferingMap);
 
         checkExtraParamsAllowed(extraParams);
 
+        if (forceMsToImportVmFiles && useVddk) {
+            throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
+                    String.format("Parameters %s and %s are mutually exclusive",
+                            ApiConstants.FORCE_MS_TO_IMPORT_VM_FILES, ApiConstants.USE_VDDK));
+        }

Review Comment:
   can move this validation before checking for pool



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049461188


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1730,7 +1743,7 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
                 convertedInstance = convertVmwareInstanceToKVMAfterExportingOVFToConvertLocation(

Review Comment:
   check if we can have separate methods for conversion using vddk and export ovf on kvm host
   
   otherwise, change method name
   
   ```suggestion
                   convertedInstance = convertVmwareInstanceToKVMUsingVDDKOrAfterExportingOVFToConvertLocation(
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3213384911


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java:
##########
@@ -255,6 +263,10 @@ public Long getStoragePoolId() {
         return storagePoolId;
     }
 
+    public boolean getUseVddk() {
+        return BooleanUtils.toBooleanDefaultIfNull(useVddk, true);

Review Comment:
   @harikrishna-patnala @rajujith IMO the default value of this boolean should be false. I'm not able to migrate VMs without VDDK installed anymore
   
   ````
   2026-05-09 15:51:17,643 ERROR [o.a.c.v.UnmanagedVMsManagerImpl] (API-Job-Executor-1:[ctx-961baab4, job-251, ctx-757448a4]) (logid:fd8b0e9d) The h
   ost Host {"id":1,"name":"ref-trl-11356-k-Mr9-nicolas-vazquez-kvm1","type":"Routing","uuid":"00504c23-253a-4c33-8268-1ba6c70aa837"} doesn't suppor
   t conversion of instance i-2-5-VM from VMware to KVM due to: Cannot convert the instance from VMware using VDDK on host 10.0.32.227. Please make 
   sure virt-v2v, nbdkit-vddk and a valid VDDK library directory are available on the host.
   ````



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3077522077


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +296,196 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionUsingVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                         String temporaryConvertFolder, String vddkLibDir,
+                                                         String libguestfsBackend, String vddkTransports,

Review Comment:
   ```suggestion
                                                            String vddkTransports,
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4243143622

   @sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3063924187


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1156,6 +1186,39 @@ public boolean configure(final String name, final Map<String, Object> params) th
 
         setConvertInstanceEnv(convertEnvTmpDir, convertEnvVirtv2vTmpDir);
 
+        vddkLibDir = StringUtils.trimToNull(AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VDDK_LIB_DIR));
+        if (StringUtils.isNotBlank(vddkLibDir) && !isVddkLibDirValid(vddkLibDir)) {
+            LOGGER.warn("Configured VDDK library dir [{}] is invalid (missing lib64/libvixDiskLib.so), attempting auto-detection", vddkLibDir);
+            vddkLibDir = null;
+        }
+        if (StringUtils.isBlank(vddkLibDir)) {
+            vddkLibDir = detectVddkLibDir();
+        }
+        if (StringUtils.isNotBlank(vddkLibDir)) {
+            LOGGER.info("Detected VDDK library dir: {}", vddkLibDir);
+        } else {
+            LOGGER.warn("Could not detect a valid VDDK library dir; VDDK conversion will be unavailable");
+        }
+
+        vddkVersion = detectVddkVersion();
+        if (StringUtils.isNotBlank(vddkVersion)) {
+            LOGGER.info("Detected nbdkit VDDK plugin version: {}", vddkVersion);
+        }
+
+        libguestfsBackend = StringUtils.defaultIfBlank(

Review Comment:
   ```suggestion
   ```



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -1156,6 +1186,39 @@ public boolean configure(final String name, final Map<String, Object> params) th
 
         setConvertInstanceEnv(convertEnvTmpDir, convertEnvVirtv2vTmpDir);
 
+        vddkLibDir = StringUtils.trimToNull(AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VDDK_LIB_DIR));
+        if (StringUtils.isNotBlank(vddkLibDir) && !isVddkLibDirValid(vddkLibDir)) {
+            LOGGER.warn("Configured VDDK library dir [{}] is invalid (missing lib64/libvixDiskLib.so), attempting auto-detection", vddkLibDir);
+            vddkLibDir = null;
+        }
+        if (StringUtils.isBlank(vddkLibDir)) {
+            vddkLibDir = detectVddkLibDir();
+        }
+        if (StringUtils.isNotBlank(vddkLibDir)) {
+            LOGGER.info("Detected VDDK library dir: {}", vddkLibDir);
+        } else {
+            LOGGER.warn("Could not detect a valid VDDK library dir; VDDK conversion will be unavailable");
+        }
+
+        vddkVersion = detectVddkVersion();
+        if (StringUtils.isNotBlank(vddkVersion)) {
+            LOGGER.info("Detected nbdkit VDDK plugin version: {}", vddkVersion);
+        }
+
+        libguestfsBackend = StringUtils.defaultIfBlank(
+                AgentPropertiesFileHandler.getPropertyValue(AgentProperties.LIBGUESTFS_BACKEND), "direct");

Review Comment:
   ```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


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3062125550


##########
agent/conf/agent.properties:
##########
@@ -457,3 +457,19 @@ iscsi.session.cleanup.enabled=false
 
 # Instance conversion VIRT_V2V_TMPDIR env var
 #convert.instance.env.virtv2v.tmpdir=
+
+# LIBGUESTFS backend to use for VMware to KVM conversion via VDDK (default: direct)

Review Comment:
   if direct is the only option supported for vddk, can remove this config?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049441062


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java:
##########
@@ -179,6 +179,14 @@ public class ImportVmCmd extends ImportUnmanagedInstanceCmd {
             description = "(only for importing VMs from VMware to KVM) optional - the ID of the guest OS for the imported VM.")
     private Long guestOsId;
 
+    @Parameter(name = ApiConstants.USE_VDDK,
+            type = CommandType.BOOLEAN,
+            since = "4.22.1",
+            description = "(only for importing VMs from VMware to KVM) optional - if true, uses VDDK on the KVM conversion host for converting the VM. " +
+                    "This parameter is mutually exclusive with " + ApiConstants.FORCE_MS_TO_IMPORT_VM_FILES + ".")

Review Comment:
   ignore - noticed the validation in UnmanagedVMsManagerImpl



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4207043260

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3055458727


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +297,197 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionUsingVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                         String temporaryConvertFolder, String vddkLibDir,
+                                                         String libguestfsBackend, String vddkTransports,
+                                                         String configuredVddkThumbprint,
+                                                         long timeout, boolean verboseModeEnabled, String extraParams,
+                                                         String temporaryConvertUuid, String passwordOption) {
+
+        String vcenterPassword = vmwareInstance.getVcenterPassword();
+        if (StringUtils.isBlank(vcenterPassword)) {
+            logger.error("({}) Could not determine vCenter password for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+
+        String passwordFilePath = String.format("/root/v2v.pass.cloud.%s.%s",

Review Comment:
   ```suggestion
           String passwordFilePath = String.format("/tmp/v2v.pass.cloud.%s.%s",
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3063994096


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -208,6 +208,9 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
     private static final List<Storage.StoragePoolType> forceConvertToPoolAllowedTypes =
             Arrays.asList(Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.Filesystem,
                     Storage.StoragePoolType.SharedMountPoint);
+    private static final String DETAIL_LIBGUESTFS_BACKEND = "libguestfs.backend";

Review Comment:
   ```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


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3055036954


##########
agent/conf/agent.properties:
##########
@@ -457,3 +457,19 @@ iscsi.session.cleanup.enabled=false
 
 # Instance conversion VIRT_V2V_TMPDIR env var
 #convert.instance.env.virtv2v.tmpdir=
+
+# LIBGUESTFS backend to use for VMware to KVM conversion via VDDK (default: direct)
+#libguestfs.backend=direct
+
+# Path to the VDDK library directory for VMware to KVM conversion via VDDK,
+# passed to virt-v2v as -io vddk-libdir=<path>
+#vddk.lib.dir=
+
+# Ordered VDDK transport preference for VMware to KVM conversion via VDDK, passed as
+# -io vddk-transports=<value> to virt-v2v. Example: nbd:nbdssl
+#vddk.transports=
+
+# Optional vCenter SHA1 thumbprint for VMware to KVM conversion via VDDK, passed as
+# -io vddk-thumbprint=<value>. If unset, CloudStack computes it on the KVM host via openssl.
+#vddk.thumbprint=
+

Review Comment:
   added a doc PR here https://github.com/apache/cloudstack-documentation/pull/640



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3063927493


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -883,10 +887,16 @@ protected enum HealthCheckResult {
 
     private boolean convertInstanceVerboseMode = false;
     private Map<String, String> convertInstanceEnv = null;
+    private String vddkLibDir = null;
+    private String libguestfsBackend = "direct";

Review Comment:
   ```suggestion
       private static final String libguestfsBackend = "direct";
   ```
   
   
   libguestfs has two main backends:
   - libvirt (default)
   - direct (QEMU directly)
   
   The libvirt backend adds an extra virtualization layer and restricts device access and networking paths. This conflicts with how Virtual Disk Development Kit (VDDK) works, which needs direct disk/NBD access. So, need to keep 'direct' for VDDK conversion. No need of any detail/property for this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4223399597

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17446


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4243082854

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17482


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4241374748

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3077500128


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +296,196 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionUsingVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                         String temporaryConvertFolder, String vddkLibDir,
+                                                         String libguestfsBackend, String vddkTransports,
+                                                         String configuredVddkThumbprint,
+                                                         long timeout, boolean verboseModeEnabled, String extraParams,
+                                                         String temporaryConvertUuid, String passwordOption) {
+
+        String vcenterPassword = vmwareInstance.getVcenterPassword();
+        if (StringUtils.isBlank(vcenterPassword)) {
+            logger.error("({}) Could not determine vCenter password for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+
+        String passwordFilePath = String.format("/tmp/v2v.pass.cloud.%s.%s",
+                StringUtils.defaultIfBlank(vmwareInstance.getVcenterHost(), "unknown"),
+                UUID.randomUUID());
+        try {
+            Files.writeString(Path.of(passwordFilePath), vcenterPassword);
+            logger.debug("({}) Written vCenter password to {}", originalVMName, passwordFilePath);
+        } catch (Exception e) {
+            logger.error("({}) Failed to write vCenter password file {}: {}", originalVMName, passwordFilePath, e.getMessage());
+            return false;
+        }
+
+        try {
+            String vpxUrl = buildVpxUrl(vmwareInstance, originalVMName);
+
+            StringBuilder cmd = new StringBuilder();
+
+            cmd.append("export LIBGUESTFS_BACKEND=").append(libguestfsBackend).append(" && ");
+
+            cmd.append("virt-v2v ");
+            cmd.append("--root first ");
+            cmd.append("-ic '").append(vpxUrl).append("' ");
+            if (StringUtils.isBlank(passwordOption)) {
+                logger.error("({}) Could not determine supported password file option for virt-v2v", originalVMName);
+                return false;
+            }
+
+            cmd.append(passwordOption).append(" ").append(passwordFilePath).append(" ");
+            cmd.append("-it vddk ");
+            cmd.append("-io vddk-libdir=").append(vddkLibDir).append(" ");
+            String vddkThumbprint = StringUtils.trimToNull(configuredVddkThumbprint);
+            if (StringUtils.isBlank(vddkThumbprint)) {
+                vddkThumbprint = getVcenterThumbprint(vmwareInstance.getVcenterHost(), timeout, originalVMName);
+            }
+            if (StringUtils.isBlank(vddkThumbprint)) {
+                logger.error("({}) Could not determine vCenter thumbprint for {}", originalVMName, vmwareInstance.getVcenterHost());
+                return false;
+            }
+            cmd.append("-io vddk-thumbprint=").append(vddkThumbprint).append(" ");
+            if (StringUtils.isNotBlank(vddkTransports)) {
+                cmd.append("-io vddk-transports=").append(vddkTransports).append(" ");
+            }
+            cmd.append(originalVMName).append(" ");

Review Comment:
   should be source vm for stopped vms?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4241478423

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4196522882

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4196814462

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/12970?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `48.64865%` with `133 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 17.62%. Comparing base ([`4708121`](https://app.codecov.io/gh/apache/cloudstack/commit/470812100ea6e9b5790bbc3a9f747d7d733ee36d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7dc5d50`](https://app.codecov.io/gh/apache/cloudstack/commit/7dc5d506b7ad67570c7d3bd90d76eab878b8619b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/cloudstack/pull/12970?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [.../wrapper/LibvirtConvertInstanceCommandWrapper.java](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&filepath=plugins%2Fhypervisors%2Fkvm%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fhypervisor%2Fkvm%2Fresource%2Fwrapper%2FLibvirtConvertInstanceCommandWrapper.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vcmVzb3VyY2Uvd3JhcHBlci9MaWJ2aXJ0Q29udmVydEluc3RhbmNlQ29tbWFuZFdyYXBwZXIuamF2YQ==) | 47.30% | [79 Missing and 9 partials :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...ervisor/kvm/resource/LibvirtComputingResource.java](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&filepath=plugins%2Fhypervisors%2Fkvm%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fhypervisor%2Fkvm%2Fresource%2FLibvirtComputingResource.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGx1Z2lucy9oeXBlcnZpc29ycy9rdm0vc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci9rdm0vcmVzb3VyY2UvTGlidmlydENvbXB1dGluZ1Jlc291cmNlLmphdmE=) | 17.39% | [19 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...va/com/cloud/agent/api/ConvertInstanceCommand.java](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&filepath=core%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fagent%2Fapi%2FConvertInstanceCommand.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9hZ2VudC9hcGkvQ29udmVydEluc3RhbmNlQ29tbWFuZC5qYXZh) | 50.00% | [15 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [.../java/com/cloud/agent/api/to/RemoteInstanceTO.java](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&filepath=api%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fagent%2Fapi%2Fto%2FRemoteInstanceTO.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YXBpL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FnZW50L2FwaS90by9SZW1vdGVJbnN0YW5jZVRPLmphdmE=) | 53.84% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...e/cloudstack/api/command/admin/vm/ImportVmCmd.java](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&filepath=api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fapi%2Fcommand%2Fadmin%2Fvm%2FImportVmCmd.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-YXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL2FwaS9jb21tYW5kL2FkbWluL3ZtL0ltcG9ydFZtQ21kLmphdmE=) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [.../apache/cloudstack/vm/UnmanagedVMsManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fcloudstack%2Fvm%2FUnmanagedVMsManagerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL3ZtL1VubWFuYWdlZFZNc01hbmFnZXJJbXBsLmphdmE=) | 89.47% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/12970?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##               4.22   #12970      +/-   ##
   ============================================
   + Coverage     17.60%   17.62%   +0.01%     
   - Complexity    15677    15692      +15     
   ============================================
     Files          5918     5918              
     Lines        531681   531918     +237     
     Branches      65005    65028      +23     
   ============================================
   + Hits          93623    93757     +134     
   - Misses       427498   427592      +94     
   - Partials      10560    10569       +9     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/12970/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/12970/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `3.70% <ø> (-0.01%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/12970/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `18.69% <48.64%> (+0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/12970?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049174767


##########
agent/conf/agent.properties:
##########
@@ -457,3 +457,19 @@ iscsi.session.cleanup.enabled=false
 
 # Instance conversion VIRT_V2V_TMPDIR env var
 #convert.instance.env.virtv2v.tmpdir=
+
+# LIBGUESTFS backend to use for VMware to KVM conversion via VDDK (default: direct)
+#libguestfs.backend=direct
+
+# Path to the VDDK library directory for VMware to KVM conversion via VDDK,
+# passed to virt-v2v as -io vddk-libdir=<path>
+#vddk.lib.dir=
+
+# Ordered VDDK transport preference for VMware to KVM conversion via VDDK, passed as
+# -io vddk-transports=<value> to virt-v2v. Example: nbd:nbdssl
+#vddk.transports=
+
+# Optional vCenter SHA1 thumbprint for VMware to KVM conversion via VDDK, passed as
+# -io vddk-thumbprint=<value>. If unset, CloudStack computes it on the KVM host via openssl.
+#vddk.thumbprint=
+

Review Comment:
   @harikrishna-patnala can you update the docs with these settings here: https://docs.cloudstack.apache.org/en/4.22.0.0/adminguide/virtual_machines.html#importing-virtual-machines-from-vmware-into-kvm



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3055556488


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1970,6 +2026,13 @@ HostVO selectKVMHostForConversionInCluster(Cluster destinationCluster, Long conv
                 err = String.format(
                         "Cannot perform the conversion on the host %s as it is not in the same zone as the destination cluster",
                         selectedHost);
+            } else if (useVddk) {
+                hostDao.loadDetails(selectedHost);
+                if (StringUtils.isBlank(selectedHost.getDetail(Host.HOST_VDDK_SUPPORT))) {
+                    err = String.format(
+                            "Cannot use VDDK-based conversion on host %s as the '%s' is not there on the host",
+                            selectedHost, Host.HOST_VDDK_SUPPORT);

Review Comment:
   in some cases, agent is not restarted after updating vddk libs in the host, so this detail can be blank, check it using CheckConvertInstanceCommand (in _checkConversionSupportOnHost()_)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3055544467


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1785,6 +1802,45 @@ protected void checkConversionStoragePool(Long convertStoragePoolId, boolean for
         }
     }
 
+    protected void validateSelectedConversionStoragePoolForVddk(boolean useVddk, Long convertStoragePoolId,
+                                                                ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap) {
+        if (!useVddk || convertStoragePoolId == null) {
+            return;
+        }
+
+        StoragePoolVO selectedStoragePool = primaryDataStoreDao.findById(convertStoragePoolId);
+        if (selectedStoragePool == null) {
+            return;
+        }
+
+        if (serviceOffering.getDiskOfferingId() != null) {
+            DiskOfferingVO rootDiskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
+            if (rootDiskOffering == null) {
+                throw new InvalidParameterValueException(String.format("Cannot find disk offering with ID %s that belongs to the service offering %s",
+                        serviceOffering.getDiskOfferingId(), serviceOffering.getName()));
+            }
+            if (!volumeApiService.doesStoragePoolSupportDiskOffering(selectedStoragePool, rootDiskOffering)) {
+                throw new InvalidParameterValueException(String.format("The root disk offering '%s' is not supported by the selected conversion storage pool '%s'. " +
+                        "When using VDDK, all selected disk offerings must be compatible with the conversion storage pool, as it will become the primary storage for the imported volumes.",

Review Comment:
   conversion storage pool is option parameter, and for ovf export scenario, secondary storage is used as the default conversion storage. ensure conversion storage pool is set in case of vddk (and update the update the importVm API parameter `convertinstancepoolid` description that it is the primary storage for vddk enabled conversion).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049389804


##########
ui/src/views/tools/ImportUnmanagedInstance.vue:
##########
@@ -184,12 +190,13 @@
               </a-form-item>
               <a-form-item name="convertstorageoption" ref="convertstorageoption">
                 <check-box-select-pair
+                  :key="`convertstorageoption-${form.usevddk ? 'vddk' : 'default'}-${switches.forceConvertToPool ? 'pool' : 'tmp'}`"
                   layout="vertical"
                   v-if="cluster.hypervisortype === 'KVM' && selectedVmwareVcenter"
                   :resourceKey="cluster.id"
                   :selectOptions="storageOptionsForConversion"
                   :checkBoxLabel="switches.forceConvertToPool ? $t('message.select.destination.storage.instance.conversion') : $t('message.select.temporary.storage.instance.conversion')"
-                  :defaultCheckBoxValue="false"
+                  :defaultCheckBoxValue="form.usevddk"
                   :reversed="false"
                   @handle-checkselectpair-change="updateSelectedStorageOptionForConversion"

Review Comment:
   `:defaultCheckBoxValue="form.usevddk"` will render the checkbox as checked when VDDK is enabled, but `CheckBoxSelectPair` only sets `checked` in `created()` and does not initialize `selectedOption` or emit `handle-checkselectpair-change` unless the user toggles the checkbox. This can leave `selectedStorageOptionForConversion` unset and prevent storage pools from being loaded while the UI appears selected. Consider explicitly calling `updateSelectedStorageOptionForConversion(...)` from `onUseVddkChange`, or updating `CheckBoxSelectPair` to initialize `selectedOption` + emit when `defaultCheckBoxValue` starts as true.



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +299,194 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                    String temporaryConvertFolder, String vddkLibDir,
+                                                    String libguestfsBackend, String vddkTransports,
+                                                    String configuredVddkThumbprint,
+                                                    long timeout, boolean verboseModeEnabled, String extraParams,
+                                                    String temporaryConvertUuid, String passwordOption) {
+
+        String vcenterPassword = vmwareInstance.getVcenterPassword();
+        if (StringUtils.isBlank(vcenterPassword)) {
+            logger.error("({}) Could not determine vCenter password for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+
+        String passwordFilePath = String.format("/root/v2v.pass.cloud.%s",
+                StringUtils.defaultIfBlank(vmwareInstance.getVcenterHost(), "unknown"));
+        try {
+            Files.writeString(Path.of(passwordFilePath), vcenterPassword);
+            logger.debug("({}) Written vCenter password to {}", originalVMName, passwordFilePath);
+        } catch (Exception e) {
+            logger.error("({}) Failed to write vCenter password file {}: {}", originalVMName, passwordFilePath, e.getMessage());
+            return false;
+        }
+
+        String vpxUrl = buildVpxUrl(vmwareInstance, originalVMName);
+
+        StringBuilder cmd = new StringBuilder();
+
+        String effectiveLibguestfsBackend = StringUtils.defaultIfBlank(libguestfsBackend, "direct");
+        cmd.append("export LIBGUESTFS_BACKEND=").append(effectiveLibguestfsBackend).append(" && ");
+
+        cmd.append("virt-v2v ");
+        cmd.append("--root first ");
+        cmd.append("-ic '").append(vpxUrl).append("' ");
+        if (StringUtils.isBlank(passwordOption)) {
+            logger.error("({}) Could not determine supported password file option for virt-v2v", originalVMName);
+            return false;
+        }
+
+        cmd.append(passwordOption).append(" ").append(passwordFilePath).append(" ");
+        cmd.append("-it vddk ");
+        cmd.append("-io vddk-libdir=").append(vddkLibDir).append(" ");
+        String vddkThumbprint = StringUtils.trimToNull(configuredVddkThumbprint);
+        if (StringUtils.isBlank(vddkThumbprint)) {
+            vddkThumbprint = getVcenterThumbprint(vmwareInstance.getVcenterHost(), timeout, originalVMName);
+        }
+        if (StringUtils.isBlank(vddkThumbprint)) {
+            logger.error("({}) Could not determine vCenter thumbprint for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+        cmd.append("-io vddk-thumbprint=").append(vddkThumbprint).append(" ");
+        if (StringUtils.isNotBlank(vddkTransports)) {
+            cmd.append("-io vddk-transports=").append(vddkTransports).append(" ");
+        }
+        cmd.append(originalVMName).append(" ");
+        cmd.append("-o local ");
+        cmd.append("-os ").append(temporaryConvertFolder).append(" ");
+        cmd.append("-of qcow2 ");
+        cmd.append("-on ").append(temporaryConvertUuid).append(" ");
+
+        if (verboseModeEnabled) {
+            cmd.append("-v ");
+        }
+
+        if (StringUtils.isNotBlank(extraParams)) {
+            cmd.append(extraParams).append(" ");
+        }
+
+        Script script = new Script("/bin/bash", timeout, logger);
+        script.add("-c");
+        script.add(cmd.toString());
+
+        String logPrefix = String.format("(%s) virt-v2v vddk import", originalVMName);
+        OutputInterpreter.LineByLineOutputLogger outputLogger =
+                new OutputInterpreter.LineByLineOutputLogger(logger, logPrefix);
+
+        logger.info("({}) Starting virt-v2v VDDK conversion", originalVMName);
+        script.execute(outputLogger);
+
+        int exitValue = script.getExitValue();
+        if (exitValue != 0) {
+            logger.error("({}) virt-v2v failed with exit code {}", originalVMName, exitValue);
+        }
+        try {
+            Files.deleteIfExists(Path.of(passwordFilePath));
+            logger.debug("({}) Deleted password file {}", originalVMName, passwordFilePath);
+        } catch (Exception e) {
+            logger.warn("({}) Failed to delete password file {}: {}", originalVMName, passwordFilePath, e.getMessage());
+        }
+
+        return exitValue == 0;
+    }
+
+
+    protected String getVcenterThumbprint(String vcenterHost, long timeout, String originalVMName) {
+        if (StringUtils.isBlank(vcenterHost)) {
+            return null;
+        }
+
+        String endpoint = String.format("%s:443", vcenterHost);
+        String command = String.format("openssl s_client -connect '%s' </dev/null 2>/dev/null | " +
+                "openssl x509 -fingerprint -sha1 -noout", endpoint);
+
+        Script script = new Script("/bin/bash", timeout, logger);
+        script.add("-c");
+        script.add(command);
+
+        OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();
+        script.execute(parser);
+
+        String output = parser.getLines();
+        if (script.getExitValue() != 0) {
+            logger.error("({}) Failed to fetch vCenter thumbprint for {}", originalVMName, vcenterHost);
+            return null;
+        }
+
+        String thumbprint = extractSha1Fingerprint(output);
+        if (StringUtils.isBlank(thumbprint)) {
+            logger.error("({}) Failed to parse vCenter thumbprint from output for {}", originalVMName, vcenterHost);
+            return null;
+        }
+        return thumbprint;
+    }
+
+    private String extractSha1Fingerprint(String output) {
+        String parsedOutput = StringUtils.trimToEmpty(output);
+        if (StringUtils.isBlank(parsedOutput)) {
+            return null;
+        }
+
+        for (String line : parsedOutput.split("\\R")) {
+            String trimmedLine = StringUtils.trimToEmpty(line);
+            if (StringUtils.isBlank(trimmedLine)) {
+                continue;
+            }
+
+            Matcher matcher = SHA1_FINGERPRINT_PATTERN.matcher(trimmedLine);
+            if (matcher.find()) {
+                return matcher.group(1).toUpperCase(Locale.ROOT);
+            }
+
+            // Fallback for raw fingerprint-only output.
+            if (trimmedLine.matches("(?i)[0-9a-f]{2}(:[0-9a-f]{2})+")) {
+                return trimmedLine.toUpperCase(Locale.ROOT);
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Build vpx:// URL for virt-v2v
+     *
+     * Format:
+     * vpx://user@vcenter/DC/cluster/host?no_verify=1
+     */
+    private String buildVpxUrl(RemoteInstanceTO vmwareInstance, String originalVMName) {
+
+        String vcenter = vmwareInstance.getVcenterHost();
+        String username = vmwareInstance.getVcenterUsername();
+        String datacenter = vmwareInstance.getDatacenterName();
+        String cluster = vmwareInstance.getClusterName();
+        String host = vmwareInstance.getHostName();
+
+        String encodedUsername = encodeUsername(username);
+
+        StringBuilder url = new StringBuilder();
+        url.append("vpx://")
+                .append(encodedUsername)
+                .append("@")
+                .append(vcenter)
+                .append("/")
+                .append(datacenter);
+
+        if (StringUtils.isNotBlank(cluster)) {
+            url.append("/").append(cluster);
+        }
+
+        if (StringUtils.isNotBlank(host)) {
+            url.append("/").append(host);
+        }
+
+        url.append("?no_verify=1");
+
+        logger.info("({}) Using VPX URL: {}", originalVMName, url);
+        return url.toString();

Review Comment:
   `buildVpxUrl` appends `?no_verify=1` and logs the full VPX URL (including the username) at INFO. Disabling verification undermines the thumbprint-based trust model, and logging credentials-related identifiers is risky. Consider removing `no_verify=1` if not strictly required, URL-encoding datacenter/cluster/host path segments, and sanitizing logs (e.g., log only vCenter host + datacenter, or redact the username).



##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1730,7 +1743,7 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
                 convertedInstance = convertVmwareInstanceToKVMAfterExportingOVFToConvertLocation(
                         sourceVMName, sourceVMwareInstance, convertHost, importHost,
                         convertStoragePools, serviceOffering, dataDiskOfferingMap,
-                        temporaryConvertLocation, vcenter, username, password, datacenterName, forceConvertToPool, extraParams);
+                        temporaryConvertLocation, vcenter, username, password, datacenterName, forceConvertToPool, extraParams, useVddk, details);

Review Comment:
   The comment and log message in this branch say it always performs OVF export through ovftool, but this same path is now also used when `useVddk` is true (which does not export OVF). This is misleading for troubleshooting and future maintenance; update the comment/logs (or split into a dedicated VDDK method) so the selected conversion strategy is accurately reflected.



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -5946,6 +5988,40 @@ public boolean hostSupportsOvfExport() {
         return exitValue == 0;
     }
 
+    /**
+     * Detect which password option virt-v2v supports by examining its --help output
+     * @return "-ip" if supported (virt-v2v >= 2.8.1), "--password-file" if older version, or null if detection fails
+     */
+    protected String detectPasswordFileOption() {
+        try {
+            ProcessBuilder pb = new ProcessBuilder("virt-v2v", "--help");
+            Process process = pb.start();
+
+            String output = new String(process.getInputStream().readAllBytes());
+            process.waitFor();
+
+            if (output.contains("-ip <filename>")) {
+                return "-ip";
+            } else if (output.contains("--password-file")) {
+                return "--password-file";
+            } else {
+                LOGGER.error("virt-v2v does not support -ip or --password-file");
+                return null;
+            }
+        } catch (Exception e) {
+            LOGGER.error("Failed to detect virt-v2v password option: {}", e.getMessage());
+            return null;
+        }

Review Comment:
   `detectPasswordFileOption()` runs `virt-v2v --help` via `ProcessBuilder` without a timeout and without consuming stderr; if `virt-v2v` hangs or produces enough stderr output, agent startup could block indefinitely. Consider using the existing `Script` helper with a timeout (and redirected stderr), and check/handle the process exit code before trusting output.
   ```suggestion
           final int timeout = 30;
           final Script script = new Script("/bin/bash", timeout, LOGGER);
           script.add("-c");
           script.add("virt-v2v --help 2>&1");
   
           final AllLinesParser parser = new AllLinesParser();
           final String executionResult = script.execute(parser);
           if (executionResult != null) {
               LOGGER.error("Failed to detect virt-v2v password option: {}", executionResult);
               return null;
           }
   
           final String output = parser.getLines();
           if (StringUtils.isBlank(output)) {
               LOGGER.error("Failed to detect virt-v2v password option: empty output from virt-v2v --help");
               return null;
           }
   
           if (output.contains("-ip <filename>")) {
               return "-ip";
           } else if (output.contains("--password-file")) {
               return "--password-file";
           } else {
               LOGGER.error("virt-v2v does not support -ip or --password-file");
               return null;
           }
   ```



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +299,194 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                    String temporaryConvertFolder, String vddkLibDir,
+                                                    String libguestfsBackend, String vddkTransports,
+                                                    String configuredVddkThumbprint,
+                                                    long timeout, boolean verboseModeEnabled, String extraParams,
+                                                    String temporaryConvertUuid, String passwordOption) {
+
+        String vcenterPassword = vmwareInstance.getVcenterPassword();
+        if (StringUtils.isBlank(vcenterPassword)) {
+            logger.error("({}) Could not determine vCenter password for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+
+        String passwordFilePath = String.format("/root/v2v.pass.cloud.%s",
+                StringUtils.defaultIfBlank(vmwareInstance.getVcenterHost(), "unknown"));
+        try {
+            Files.writeString(Path.of(passwordFilePath), vcenterPassword);
+            logger.debug("({}) Written vCenter password to {}", originalVMName, passwordFilePath);
+        } catch (Exception e) {
+            logger.error("({}) Failed to write vCenter password file {}: {}", originalVMName, passwordFilePath, e.getMessage());
+            return false;
+        }
+
+        String vpxUrl = buildVpxUrl(vmwareInstance, originalVMName);
+
+        StringBuilder cmd = new StringBuilder();
+
+        String effectiveLibguestfsBackend = StringUtils.defaultIfBlank(libguestfsBackend, "direct");
+        cmd.append("export LIBGUESTFS_BACKEND=").append(effectiveLibguestfsBackend).append(" && ");
+
+        cmd.append("virt-v2v ");
+        cmd.append("--root first ");
+        cmd.append("-ic '").append(vpxUrl).append("' ");
+        if (StringUtils.isBlank(passwordOption)) {
+            logger.error("({}) Could not determine supported password file option for virt-v2v", originalVMName);
+            return false;
+        }
+
+        cmd.append(passwordOption).append(" ").append(passwordFilePath).append(" ");
+        cmd.append("-it vddk ");
+        cmd.append("-io vddk-libdir=").append(vddkLibDir).append(" ");
+        String vddkThumbprint = StringUtils.trimToNull(configuredVddkThumbprint);
+        if (StringUtils.isBlank(vddkThumbprint)) {
+            vddkThumbprint = getVcenterThumbprint(vmwareInstance.getVcenterHost(), timeout, originalVMName);
+        }
+        if (StringUtils.isBlank(vddkThumbprint)) {
+            logger.error("({}) Could not determine vCenter thumbprint for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+        cmd.append("-io vddk-thumbprint=").append(vddkThumbprint).append(" ");
+        if (StringUtils.isNotBlank(vddkTransports)) {
+            cmd.append("-io vddk-transports=").append(vddkTransports).append(" ");
+        }
+        cmd.append(originalVMName).append(" ");
+        cmd.append("-o local ");
+        cmd.append("-os ").append(temporaryConvertFolder).append(" ");
+        cmd.append("-of qcow2 ");
+        cmd.append("-on ").append(temporaryConvertUuid).append(" ");
+
+        if (verboseModeEnabled) {
+            cmd.append("-v ");
+        }
+
+        if (StringUtils.isNotBlank(extraParams)) {
+            cmd.append(extraParams).append(" ");
+        }
+
+        Script script = new Script("/bin/bash", timeout, logger);
+        script.add("-c");
+        script.add(cmd.toString());
+

Review Comment:
   The VDDK path builds a single shell string and runs it via `/bin/bash -c`, concatenating values like VM name, paths, transports, and `extraParams`. Several of these are user/DB configurable and are not shell-escaped, which can break conversions for names with spaces/special chars and also introduces command-injection risk. Prefer constructing a `Script("virt-v2v", ...)` and passing arguments with `script.add(...)` (similar to `performInstanceConversion`), and set `LIBGUESTFS_BACKEND` via the env-map rather than `export ... &&`.



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +299,194 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                    String temporaryConvertFolder, String vddkLibDir,
+                                                    String libguestfsBackend, String vddkTransports,
+                                                    String configuredVddkThumbprint,
+                                                    long timeout, boolean verboseModeEnabled, String extraParams,
+                                                    String temporaryConvertUuid, String passwordOption) {
+
+        String vcenterPassword = vmwareInstance.getVcenterPassword();
+        if (StringUtils.isBlank(vcenterPassword)) {
+            logger.error("({}) Could not determine vCenter password for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+
+        String passwordFilePath = String.format("/root/v2v.pass.cloud.%s",
+                StringUtils.defaultIfBlank(vmwareInstance.getVcenterHost(), "unknown"));
+        try {
+            Files.writeString(Path.of(passwordFilePath), vcenterPassword);
+            logger.debug("({}) Written vCenter password to {}", originalVMName, passwordFilePath);
+        } catch (Exception e) {
+            logger.error("({}) Failed to write vCenter password file {}: {}", originalVMName, passwordFilePath, e.getMessage());
+            return false;
+        }
+
+        String vpxUrl = buildVpxUrl(vmwareInstance, originalVMName);
+
+        StringBuilder cmd = new StringBuilder();
+
+        String effectiveLibguestfsBackend = StringUtils.defaultIfBlank(libguestfsBackend, "direct");
+        cmd.append("export LIBGUESTFS_BACKEND=").append(effectiveLibguestfsBackend).append(" && ");
+
+        cmd.append("virt-v2v ");
+        cmd.append("--root first ");
+        cmd.append("-ic '").append(vpxUrl).append("' ");
+        if (StringUtils.isBlank(passwordOption)) {
+            logger.error("({}) Could not determine supported password file option for virt-v2v", originalVMName);
+            return false;
+        }
+
+        cmd.append(passwordOption).append(" ").append(passwordFilePath).append(" ");
+        cmd.append("-it vddk ");
+        cmd.append("-io vddk-libdir=").append(vddkLibDir).append(" ");
+        String vddkThumbprint = StringUtils.trimToNull(configuredVddkThumbprint);
+        if (StringUtils.isBlank(vddkThumbprint)) {
+            vddkThumbprint = getVcenterThumbprint(vmwareInstance.getVcenterHost(), timeout, originalVMName);
+        }
+        if (StringUtils.isBlank(vddkThumbprint)) {
+            logger.error("({}) Could not determine vCenter thumbprint for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }

Review Comment:
   `performInstanceConversionVddk` writes the vCenter password to a fixed path under `/root` derived from the vCenter host. This is vulnerable to races between concurrent conversions (overwriting/deleting each other’s password file) and may leave the password on disk on early returns (e.g., when `passwordOption` is blank or thumbprint lookup fails). Use a unique temp file (e.g., `Files.createTempFile`), set restrictive permissions (0600), and ensure deletion happens in a `finally` block that runs for all exit paths.



##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1785,6 +1798,41 @@ protected void checkConversionStoragePool(Long convertStoragePoolId, boolean for
         }
     }
 
+    protected void validateSelectedConversionStoragePoolForVddk(boolean useVddk, Long convertStoragePoolId,
+                                                                ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap) {
+        if (!useVddk || convertStoragePoolId == null) {
+            return;
+        }
+
+        StoragePoolVO selectedStoragePool = primaryDataStoreDao.findById(convertStoragePoolId);
+        if (selectedStoragePool == null) {
+            return;
+        }
+
+        if (serviceOffering.getDiskOfferingId() != null) {
+            DiskOfferingVO rootDiskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
+            if (rootDiskOffering == null) {
+                throw new InvalidParameterValueException(String.format("Cannot find disk offering with ID %s that belongs to the service offering %s",
+                        serviceOffering.getDiskOfferingId(), serviceOffering.getName()));
+            }
+            if (!volumeApiService.doesStoragePoolSupportDiskOffering(selectedStoragePool, rootDiskOffering)) {
+                throw new InvalidParameterValueException("Using VDDK, multiple storage pools cannot be used if a storage pool is selected for conversion");
+            }
+        }
+
+        if (MapUtils.isNotEmpty(dataDiskOfferingMap)) {
+            for (Long diskOfferingId : dataDiskOfferingMap.values()) {
+                DiskOfferingVO diskOffering = diskOfferingDao.findById(diskOfferingId);
+                if (diskOffering == null) {
+                    throw new InvalidParameterValueException(String.format("Cannot find disk offering with ID %s", diskOfferingId));
+                }
+                if (!volumeApiService.doesStoragePoolSupportDiskOffering(selectedStoragePool, diskOffering)) {
+                    throw new InvalidParameterValueException("Using VDDK, multiple storage pools cannot be used if a storage pool is selected for conversion");
+                }
+            }

Review Comment:
   `validateSelectedConversionStoragePoolForVddk` throws "Using VDDK, multiple storage pools cannot be used if a storage pool is selected for conversion" when a disk offering is unsupported by the selected pool. The message doesn’t match the actual failure condition (disk offering incompatibility) and will be confusing to users. Consider a clearer message that names the selected pool and the disk offering that isn’t supported.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4212968734

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4236984391

   @harikrishna-patnala can you address the outstanding comments, and resolve the conflicts.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4196525751

   @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4197089127

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17380


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049230432


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +299,194 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
+                                                    String temporaryConvertFolder, String vddkLibDir,
+                                                    String libguestfsBackend, String vddkTransports,
+                                                    String configuredVddkThumbprint,
+                                                    long timeout, boolean verboseModeEnabled, String extraParams,
+                                                    String temporaryConvertUuid, String passwordOption) {
+
+        String vcenterPassword = vmwareInstance.getVcenterPassword();
+        if (StringUtils.isBlank(vcenterPassword)) {
+            logger.error("({}) Could not determine vCenter password for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+
+        String passwordFilePath = String.format("/root/v2v.pass.cloud.%s",
+                StringUtils.defaultIfBlank(vmwareInstance.getVcenterHost(), "unknown"));
+        try {
+            Files.writeString(Path.of(passwordFilePath), vcenterPassword);
+            logger.debug("({}) Written vCenter password to {}", originalVMName, passwordFilePath);
+        } catch (Exception e) {
+            logger.error("({}) Failed to write vCenter password file {}: {}", originalVMName, passwordFilePath, e.getMessage());
+            return false;
+        }
+
+        String vpxUrl = buildVpxUrl(vmwareInstance, originalVMName);
+
+        StringBuilder cmd = new StringBuilder();
+
+        String effectiveLibguestfsBackend = StringUtils.defaultIfBlank(libguestfsBackend, "direct");
+        cmd.append("export LIBGUESTFS_BACKEND=").append(effectiveLibguestfsBackend).append(" && ");
+
+        cmd.append("virt-v2v ");
+        cmd.append("--root first ");
+        cmd.append("-ic '").append(vpxUrl).append("' ");
+        if (StringUtils.isBlank(passwordOption)) {
+            logger.error("({}) Could not determine supported password file option for virt-v2v", originalVMName);
+            return false;
+        }
+
+        cmd.append(passwordOption).append(" ").append(passwordFilePath).append(" ");
+        cmd.append("-it vddk ");
+        cmd.append("-io vddk-libdir=").append(vddkLibDir).append(" ");
+        String vddkThumbprint = StringUtils.trimToNull(configuredVddkThumbprint);
+        if (StringUtils.isBlank(vddkThumbprint)) {
+            vddkThumbprint = getVcenterThumbprint(vmwareInstance.getVcenterHost(), timeout, originalVMName);
+        }
+        if (StringUtils.isBlank(vddkThumbprint)) {
+            logger.error("({}) Could not determine vCenter thumbprint for {}", originalVMName, vmwareInstance.getVcenterHost());
+            return false;
+        }
+        cmd.append("-io vddk-thumbprint=").append(vddkThumbprint).append(" ");
+        if (StringUtils.isNotBlank(vddkTransports)) {
+            cmd.append("-io vddk-transports=").append(vddkTransports).append(" ");
+        }
+        cmd.append(originalVMName).append(" ");
+        cmd.append("-o local ");
+        cmd.append("-os ").append(temporaryConvertFolder).append(" ");
+        cmd.append("-of qcow2 ");
+        cmd.append("-on ").append(temporaryConvertUuid).append(" ");
+
+        if (verboseModeEnabled) {
+            cmd.append("-v ");
+        }
+
+        if (StringUtils.isNotBlank(extraParams)) {
+            cmd.append(extraParams).append(" ");
+        }
+
+        Script script = new Script("/bin/bash", timeout, logger);
+        script.add("-c");
+        script.add(cmd.toString());
+
+        String logPrefix = String.format("(%s) virt-v2v vddk import", originalVMName);
+        OutputInterpreter.LineByLineOutputLogger outputLogger =
+                new OutputInterpreter.LineByLineOutputLogger(logger, logPrefix);
+
+        logger.info("({}) Starting virt-v2v VDDK conversion", originalVMName);
+        script.execute(outputLogger);
+
+        int exitValue = script.getExitValue();
+        if (exitValue != 0) {
+            logger.error("({}) virt-v2v failed with exit code {}", originalVMName, exitValue);
+        }
+        try {
+            Files.deleteIfExists(Path.of(passwordFilePath));
+            logger.debug("({}) Deleted password file {}", originalVMName, passwordFilePath);
+        } catch (Exception e) {
+            logger.warn("({}) Failed to delete password file {}: {}", originalVMName, passwordFilePath, e.getMessage());
+        }
+
+        return exitValue == 0;
+    }
+
+

Review Comment:
   ```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


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049449936


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########


Review Comment:
   ```suggestion
                   // Uses KVM Host for conversion using VDDK, or for OVF export to temporary conversion location through ovftool 
   ```



##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########


Review Comment:
   ```suggestion
                   // Uses KVM Host for direct conversion using VDDK, or for OVF export to temporary conversion location through ovftool 
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049348004


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java:
##########
@@ -179,6 +179,14 @@ public class ImportVmCmd extends ImportUnmanagedInstanceCmd {
             description = "(only for importing VMs from VMware to KVM) optional - the ID of the guest OS for the imported VM.")
     private Long guestOsId;
 
+    @Parameter(name = ApiConstants.USE_VDDK,
+            type = CommandType.BOOLEAN,
+            since = "4.22.1",
+            description = "(only for importing VMs from VMware to KVM) optional - if true, uses VDDK on the KVM conversion host for converting the VM. " +
+                    "This parameter is mutually exclusive with " + ApiConstants.FORCE_MS_TO_IMPORT_VM_FILES + ".")

Review Comment:
   can validate when both parameters are set and throw relevant error



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049264212


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -84,61 +91,78 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve
         logger.info(String.format("(%s) Attempting to convert the instance %s from %s to KVM",
                 originalVMName, sourceInstanceName, sourceHypervisorType));
         final String temporaryConvertPath = temporaryStoragePool.getLocalPath();
+        final String temporaryConvertUuid = UUID.randomUUID().toString();
+        boolean verboseModeEnabled = serverResource.isConvertInstanceVerboseModeEnabled();
 
-        String ovfTemplateDirOnConversionLocation;
-        String sourceOVFDirPath;
+        boolean cleanupSecondaryStorage = false;
         boolean ovfExported = false;
-        if (cmd.getExportOvfToConversionLocation()) {
-            String exportInstanceOVAUrl = getExportInstanceOVAUrl(sourceInstance, originalVMName);
-            if (StringUtils.isBlank(exportInstanceOVAUrl)) {
-                String err = String.format("Couldn't export OVA for the VM %s, due to empty url", sourceInstanceName);
-                logger.error(String.format("(%s) %s", originalVMName, err));
-                return new Answer(cmd, false, err);
-            }
+        String ovfTemplateDirOnConversionLocation = null;
 
-            int noOfThreads = cmd.getThreadsCountToExportOvf();
-            if (noOfThreads > 1 && !serverResource.ovfExportToolSupportsParallelThreads()) {
-                noOfThreads = 0;
-            }
-            ovfTemplateDirOnConversionLocation = UUID.randomUUID().toString();
-            temporaryStoragePool.createFolder(ovfTemplateDirOnConversionLocation);
-            sourceOVFDirPath = String.format("%s/%s/", temporaryConvertPath, ovfTemplateDirOnConversionLocation);
-            ovfExported = exportOVAFromVMOnVcenter(exportInstanceOVAUrl, sourceOVFDirPath, noOfThreads, originalVMName, timeout);
-            if (!ovfExported) {
-                String err = String.format("Export OVA for the VM %s failed", sourceInstanceName);
-                logger.error(String.format("(%s) %s", originalVMName, err));
-                return new Answer(cmd, false, err);
-            }
-            sourceOVFDirPath = String.format("%s%s/", sourceOVFDirPath, sourceInstanceName);
-        } else {
-            ovfTemplateDirOnConversionLocation = cmd.getTemplateDirOnConversionLocation();
-            sourceOVFDirPath = String.format("%s/%s/", temporaryConvertPath, ovfTemplateDirOnConversionLocation);
-        }
+        try {
+            boolean result;
+            if (useVddk) {
+                logger.info("({}) Using VDDK-based conversion (direct from VMware)", originalVMName);
+                String vddkLibDir = resolveVddkSetting(cmd.getVddkLibDir(), serverResource.getVddkLibDir());
+                if (StringUtils.isBlank(vddkLibDir)) {
+                    String err = String.format("VDDK lib dir is not configured on the host. " +
+                            "Set '%s' in agent.properties to use VDDK-based conversion.", "vddk.lib.dir");
+                    logger.error("({}) {}", originalVMName, err);
+                    return new Answer(cmd, false, err);
+                }
+                String libguestfsBackend = StringUtils.defaultIfBlank(resolveVddkSetting(cmd.getLibguestfsBackend(), serverResource.getLibguestfsBackend()), "direct");
+                String vddkTransports = resolveVddkSetting(cmd.getVddkTransports(), serverResource.getVddkTransports());
+                String configuredVddkThumbprint = resolveVddkSetting(cmd.getVddkThumbprint(), serverResource.getVddkThumbprint());
+                String passwordOption = serverResource.getDetectedPasswordFileOption();
+                result = performInstanceConversionVddk(sourceInstance, originalVMName, temporaryConvertPath,
+                        vddkLibDir, libguestfsBackend, vddkTransports, configuredVddkThumbprint,
+                        timeout, verboseModeEnabled, extraParams, temporaryConvertUuid, passwordOption);
+            } else {
+                logger.info("({}) Using OVF-based conversion (export + local convert)", originalVMName);
+                String sourceOVFDirPath;
+                if (cmd.getExportOvfToConversionLocation()) {
+                    String exportInstanceOVAUrl = getExportInstanceOVAUrl(sourceInstance, originalVMName);
 
-        logger.info(String.format("(%s) Attempting to convert the OVF %s of the instance %s from %s to KVM",
-                originalVMName, ovfTemplateDirOnConversionLocation, sourceInstanceName, sourceHypervisorType));
+                    if (StringUtils.isBlank(exportInstanceOVAUrl)) {
+                        String err = String.format("Couldn't export OVA for the VM %s, due to empty url", sourceInstanceName);
+                        logger.error("({}) {}", originalVMName, err);
+                        return new Answer(cmd, false, err);
+                    }
 
-        final String temporaryConvertUuid = UUID.randomUUID().toString();
-        boolean verboseModeEnabled = serverResource.isConvertInstanceVerboseModeEnabled();
+                    int noOfThreads = cmd.getThreadsCountToExportOvf();
+                    if (noOfThreads > 1 && !serverResource.ovfExportToolSupportsParallelThreads()) {
+                        noOfThreads = 0;
+                    }
+                    ovfTemplateDirOnConversionLocation = UUID.randomUUID().toString();
+                    temporaryStoragePool.createFolder(ovfTemplateDirOnConversionLocation);
+                    sourceOVFDirPath = String.format("%s/%s/", temporaryConvertPath, ovfTemplateDirOnConversionLocation);
+                    ovfExported = exportOVAFromVMOnVcenter(exportInstanceOVAUrl, sourceOVFDirPath, noOfThreads, originalVMName, timeout);
+
+                    if (!ovfExported) {
+                        String err = String.format("Export OVA for the VM %s failed", sourceInstanceName);
+                        logger.error("({}) {}", originalVMName, err);
+                        return new Answer(cmd, false, err);
+                    }
+                    sourceOVFDirPath = String.format("%s%s/", sourceOVFDirPath, sourceInstanceName);
+                } else {
+                    ovfTemplateDirOnConversionLocation = cmd.getTemplateDirOnConversionLocation();
+                    sourceOVFDirPath = String.format("%s/%s/", temporaryConvertPath, ovfTemplateDirOnConversionLocation);
+                }
+
+                result = performInstanceConversion(originalVMName, sourceOVFDirPath, temporaryConvertPath, temporaryConvertUuid,
+                        timeout, verboseModeEnabled, extraParams, serverResource);
+            }
 
-        boolean cleanupSecondaryStorage = false;
-        try {
-            boolean result = performInstanceConversion(originalVMName, sourceOVFDirPath, temporaryConvertPath, temporaryConvertUuid,
-                    timeout, verboseModeEnabled, extraParams, serverResource);
             if (!result) {
-                String err = String.format(
-                        "The virt-v2v conversion for the OVF %s failed. Please check the agent logs " +
-                                "for the virt-v2v output. Please try on a different kvm host which " +
-                                "has a different virt-v2v version.",
-                        ovfTemplateDirOnConversionLocation);
-                logger.error(String.format("(%s) %s", originalVMName, err));
+                String err = String.format("Instance conversion failed for VM %s. Please check virt-v2v logs.", sourceInstanceName);
+                logger.error("({}) {}", originalVMName, err);
                 return new Answer(cmd, false, err);
             }
+
             return new ConvertInstanceAnswer(cmd, temporaryConvertUuid);
+

Review Comment:
   ```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


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4248364743

   <b>[SF] Trillian test result (tid-15871)</b>
   Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
   Total time taken: 49249 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12970-t15871-kvm-ol8.zip
   Smoke tests completed. 149 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4196864369

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4204078032

   CheckConvertInstanceCommand should ensure vddk conversion support on host when usevddk is enabled (in LibvirtCheckConvertInstanceCommandWrapper)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4210592708

   @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4210590352

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4207058990

   @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3055521882


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1725,12 +1743,11 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
                         serviceOffering, dataDiskOfferingMap, temporaryConvertLocation,
                         ovfTemplateOnConvertLocation, forceConvertToPool, extraParams);
             } else {
-                // Uses KVM Host for OVF export to temporary conversion location, through ovftool
-                importVmTasksManager.updateImportVMTaskStep(importVMTask, zone, owner, convertHost, importHost, null, ConvertingInstance);
-                convertedInstance = convertVmwareInstanceToKVMAfterExportingOVFToConvertLocation(
+                // Uses KVM Host for direct conversion using VDDK, or for OVF export to temporary conversion location through ovftool                 importVmTasksManager.updateImportVMTaskStep(importVMTask, zone, owner, convertHost, importHost, null, ConvertingInstance);

Review Comment:
   ```suggestion
                   // Uses KVM Host for direct conversion using VDDK, or for OVF export to temporary conversion location through ovftool             
                   importVmTasksManager.updateImportVMTaskStep(importVMTask, zone, owner, convertHost, importHost, null, ConvertingInstance);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4222865947

   @harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4222861782

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3063921751


##########
core/src/main/java/com/cloud/agent/api/ConvertInstanceCommand.java:
##########
@@ -31,6 +31,11 @@ public class ConvertInstanceCommand extends Command {
     private boolean exportOvfToConversionLocation;
     private int threadsCountToExportOvf = 0;
     private String extraParams;
+    private boolean useVddk;
+    private String libguestfsBackend;

Review Comment:
   ```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


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4243135152

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3213417584


##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportVmCmd.java:
##########
@@ -255,6 +263,10 @@ public Long getStoragePoolId() {
         return storagePoolId;
     }
 
+    public boolean getUseVddk() {
+        return BooleanUtils.toBooleanDefaultIfNull(useVddk, true);

Review Comment:
   Sorry discard, UI somehow was not updated, I'm seeing the use VDDK set as false now



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4213223755

   Packaging result [SF]: ✖️ el8 ✖️ el9  ✖️ debian ✖️ suse15. SL-JID 17420


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4210817264

   Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17411


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049336807


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1785,6 +1798,41 @@ protected void checkConversionStoragePool(Long convertStoragePoolId, boolean for
         }
     }
 
+    protected void validateSelectedConversionStoragePoolForVddk(boolean useVddk, Long convertStoragePoolId,
+                                                                ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap) {
+        if (!useVddk || convertStoragePoolId == null) {
+            return;
+        }
+
+        StoragePoolVO selectedStoragePool = primaryDataStoreDao.findById(convertStoragePoolId);
+        if (selectedStoragePool == null) {
+            return;
+        }
+
+        if (serviceOffering.getDiskOfferingId() != null) {
+            DiskOfferingVO rootDiskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
+            if (rootDiskOffering == null) {
+                throw new InvalidParameterValueException(String.format("Cannot find disk offering with ID %s that belongs to the service offering %s",
+                        serviceOffering.getDiskOfferingId(), serviceOffering.getName()));
+            }
+            if (!volumeApiService.doesStoragePoolSupportDiskOffering(selectedStoragePool, rootDiskOffering)) {
+                throw new InvalidParameterValueException("Using VDDK, multiple storage pools cannot be used if a storage pool is selected for conversion");

Review Comment:
   better rephrase the msg, to indicate that the selected compute/disk offerings picked multiple storage pools other than the selected pool for conversion.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049239325


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java:
##########
@@ -275,4 +299,194 @@ protected void addExtraParamsToScript(String extraParams, Script script) {
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }
+
+    private String resolveVddkSetting(String commandValue, String agentValue) {
+        return StringUtils.defaultIfBlank(StringUtils.trimToNull(commandValue), StringUtils.trimToNull(agentValue));
+    }
+
+    protected boolean performInstanceConversionVddk(RemoteInstanceTO vmwareInstance, String originalVMName,

Review Comment:
   ```suggestion
       protected boolean performInstanceConversionUsingVddk(RemoteInstanceTO vmwareInstance, String originalVMName,
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3049336807


##########
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java:
##########
@@ -1785,6 +1798,41 @@ protected void checkConversionStoragePool(Long convertStoragePoolId, boolean for
         }
     }
 
+    protected void validateSelectedConversionStoragePoolForVddk(boolean useVddk, Long convertStoragePoolId,
+                                                                ServiceOfferingVO serviceOffering, Map<String, Long> dataDiskOfferingMap) {
+        if (!useVddk || convertStoragePoolId == null) {
+            return;
+        }
+
+        StoragePoolVO selectedStoragePool = primaryDataStoreDao.findById(convertStoragePoolId);
+        if (selectedStoragePool == null) {
+            return;
+        }
+
+        if (serviceOffering.getDiskOfferingId() != null) {
+            DiskOfferingVO rootDiskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
+            if (rootDiskOffering == null) {
+                throw new InvalidParameterValueException(String.format("Cannot find disk offering with ID %s that belongs to the service offering %s",
+                        serviceOffering.getDiskOfferingId(), serviceOffering.getName()));
+            }
+            if (!volumeApiService.doesStoragePoolSupportDiskOffering(selectedStoragePool, rootDiskOffering)) {
+                throw new InvalidParameterValueException("Using VDDK, multiple storage pools cannot be used if a storage pool is selected for conversion");

Review Comment:
   better rephrase the msg, to indicate that the selected compute offering picked different storage pool other than the selected pool for conversion.
   
   (temporary) conversion storage in the earlier migration process is being considered as the primary storage allocated for the imported volumes while using VDDK and the offerings selected should honor that. Check if it is possible to list suitable compute/disk offerings with the selected pool in the UI? UI and Docs need to indicate this change for VDDK.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4227269328

   This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#discussion_r3063927493


##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -883,10 +887,16 @@ protected enum HealthCheckResult {
 
     private boolean convertInstanceVerboseMode = false;
     private Map<String, String> convertInstanceEnv = null;
+    private String vddkLibDir = null;
+    private String libguestfsBackend = "direct";

Review Comment:
   ```suggestion
       private static final String libguestfsBackend = "direct";
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4242641309

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4196847304

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "harikrishna-patnala (via GitHub)" <gi...@apache.org>.
harikrishna-patnala commented on PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970#issuecomment-4199390563

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Added vddk support in vmware to kvm migrations [cloudstack]

Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti merged PR #12970:
URL: https://github.com/apache/cloudstack/pull/12970


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org