weizhouapache commented on code in PR #7881:
URL: https://github.com/apache/cloudstack/pull/7881#discussion_r1308987156


##########
api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateFromVMwareVMCmd.java:
##########
@@ -0,0 +1,155 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.template;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.template.VirtualMachineTemplate;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.TemplateResponse;
+import org.apache.cloudstack.api.response.VmwareDatacenterResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.List;
+
+@APICommand(name = "registerTemplateFromVmware",
+        description = "Registers a template from a stopped VM in an existing 
VMware vCenter.",
+        responseObject = TemplateResponse.class, responseView = 
ResponseObject.ResponseView.Restricted,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class RegisterTemplateFromVMwareVMCmd extends RegisterTemplateCmd {
+
+    @Parameter(name = ApiConstants.HOST_IP,
+            type = BaseCmd.CommandType.STRING,
+            description = "VMware ESXi host IP/Name.")
+    private String host;
+
+    @Parameter(name = ApiConstants.VIRTUAL_MACHINE_NAME,
+            type = BaseCmd.CommandType.STRING,
+            description = "VMware VM Name.")
+    private String vmName;
+
+    @Parameter(name = ApiConstants.ZONE_ID,
+            type = CommandType.UUID,
+            entityType = ZoneResponse.class,
+            description = "KVM zone where the end result template is 
registered")
+    private Long zoneId;
+
+    @Parameter(name = ApiConstants.EXISTING_VCENTER_ID,
+            type = CommandType.UUID,
+            entityType = VmwareDatacenterResponse.class,
+            description = "UUID of a linked existing vCenter")
+    private Long existingVcenterId;
+
+    @Parameter(name = ApiConstants.VCENTER,
+            type = CommandType.STRING,
+            description = "The name/ip of vCenter. Make sure it is IP address 
or full qualified domain name for host running vCenter server.")
+    private String vcenter;
+
+    @Parameter(name = ApiConstants.DATACENTER_NAME, type = CommandType.STRING,
+            description = "Name of VMware datacenter.")
+    private String datacenterName;
+
+    @Parameter(name = ApiConstants.CLUSTER_NAME, type = CommandType.STRING,
+            description = "Name of VMware cluster.")
+    private String clusterName;
+
+    @Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING,
+            description = "The Username required to connect to resource.")
+    private String username;
+
+    @Parameter(name = ApiConstants.PASSWORD, type = CommandType.STRING,
+            description = "The password for the specified username.")
+    private String password;
+
+    public String getVcenter() {
+        return vcenter;
+    }
+
+    public String getHost() {
+        return host;
+    }
+
+    public String getVmName() {
+        return vmName;
+    }
+
+    public Long getZoneId() {
+        return zoneId;
+    }
+
+    public String getDatacenterName() {
+        return datacenterName;
+    }
+
+    public String getUsername() {
+        return username;
+    }
+
+    public String getPassword() {
+        return password;
+    }
+
+    public String getClusterName() {
+        return clusterName;
+    }
+
+    public Long getExistingVcenterId() {
+        return existingVcenterId;
+    }
+
+    @Override
+    protected void validateParameters() {
+        if ((existingVcenterId == null && vcenter == null) || 
(existingVcenterId != null && vcenter != null)) {

Review Comment:
   can use 
   
[isAllEmpty](https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/StringUtils.html#isAllEmpty-java.lang.CharSequence...-)
 or isAllBlank
   and
   
[isNoneEmpty](https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/StringUtils.html#isNoneEmpty-java.lang.CharSequence...-)
 or isNoneBlank
   



##########
engine/schema/src/main/java/com/cloud/storage/VMTemplateVO.java:
##########
@@ -649,6 +650,11 @@ public void setParentTemplateId(Long parentTemplateId) {
         return deployAsIs;
     }
 
+    @Override
+    public boolean isMigratedFromVmwareVM() {

Review Comment:
   add `ToKvm` in the method name , or change the check from `hypervisorType == 
HypervisorType.KVM` to `hypervisorType != HypervisorType.VMware` ?



##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -1217,4 +1222,95 @@ private String getTargetHostGuid(StoragePool 
targetLocalPoolForVM, Long destClus
     protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile 
vmProfile) {
         return super.toVirtualMachineTO(vmProfile);
     }
+
+    private VmwareContext connectToVcenter(String vcenter, String username, 
String password) throws Exception {
+        VmwareClient vimClient = new VmwareClient(vcenter);
+        String serviceUrl = "https://"; + vcenter + "/sdk/vimService";
+        vimClient.connect(serviceUrl, username, password);
+        return new VmwareContext(vimClient, vcenter);
+    }
+
+    private void relocateClonedVMToSourceHost(VirtualMachineMO clonedVM, 
HostMO sourceHost) throws Exception {
+        if (!clonedVM.getRunningHost().getMor().equals(sourceHost.getMor())) {
+            s_logger.debug(String.format("Relocating VM to the same host as 
the source VM: %s", sourceHost.getHostName()));
+            if (!clonedVM.relocate(sourceHost.getMor())) {
+                String err = String.format("Cannot relocate cloned VM %s to 
the source host %s", clonedVM.getVmName(), sourceHost.getHostName());
+                s_logger.error(err);
+                throw new CloudRuntimeException(err);
+            }
+        }
+    }
+
+    private VirtualMachineMO createCloneFromSourceVM(String vmName, 
VirtualMachineMO vmMo,
+                                                     DatacenterMO 
dataCenterMO) throws Exception {
+        HostMO sourceHost = vmMo.getRunningHost();
+        String cloneName = String.format("%s-%s", vmName, UUID.randomUUID());
+        DatastoreMO datastoreMO = vmMo.getAllDatastores().get(0); //pick the 
first datastore
+        ManagedObjectReference morPool = 
vmMo.getRunningHost().getHyperHostOwnerResourcePool();
+        boolean result = vmMo.createFullClone(cloneName, 
dataCenterMO.getVmFolder(), morPool, datastoreMO.getMor(), 
Storage.ProvisioningType.THIN);
+        VirtualMachineMO clonedVM = dataCenterMO.findVm(cloneName);
+        if (!result || clonedVM == null) {
+            String err = String.format("Could not clone VM %s before migration 
from VMware", vmName);
+            s_logger.error(err);
+            throw new CloudRuntimeException(err);
+        }
+        relocateClonedVMToSourceHost(clonedVM, sourceHost);
+        return clonedVM;
+    }
+
+    private HypervisorOutOfBandVMClone 
generateResponseFromClone(VirtualMachineMO clonedVM) throws Exception {
+        List<VirtualDisk> virtualDisks = clonedVM.getVirtualDisks();
+        long capacity = 0;
+        for (VirtualDisk disk : virtualDisks) {
+            capacity = Long.sum(capacity, disk.getCapacityInBytes());
+        }
+        return new HypervisorOutOfBandVMClone(clonedVM.getVmName(), 
virtualDisks.size(), capacity);
+    }
+
+    @Override
+    public HypervisorOutOfBandVMClone cloneHypervisorVMOutOfBand(String 
hostIp, String vmName, Map<String, String> params) {
+        s_logger.debug(String.format("Cloning VM %s on external vCenter %s", 
vmName, hostIp));
+        String vcenter = params.get(VmDetailConstants.VMWARE_VCENTER);
+        String datacenter = params.get(VmDetailConstants.VMWARE_DATACENTER);
+        String username = 
params.get(VmDetailConstants.VMWARE_VCENTER_USERNAME);
+        String password = 
params.get(VmDetailConstants.VMWARE_VCENTER_PASSWORD);
+
+        try {
+            VmwareContext context = connectToVcenter(vcenter, username, 
password);
+            DatacenterMO dataCenterMO = new DatacenterMO(context, datacenter);
+            VirtualMachineMO vmMo = dataCenterMO.findVm(vmName);
+            if (vmMo == null) {
+                String err = String.format("Cannot find VM with name %s on 
%s/%s", vmName, vcenter, datacenter);
+                s_logger.error(err);
+                throw new CloudRuntimeException(err);
+            }
+            VirtualMachineMO clonedVM = createCloneFromSourceVM(vmName, vmMo, 
dataCenterMO);
+            HypervisorOutOfBandVMClone clone = 
generateResponseFromClone(clonedVM);
+            s_logger.debug(String.format("VM cloned successfully"));
+            return clone;
+        } catch (Exception e) {
+            String err = String.format("Error cloning VM: %s from external 
vCenter %s: %s", vmName, vcenter, e.getMessage());
+            s_logger.error(err, e);
+            throw new CloudRuntimeException(err, e);
+        }
+    }
+
+    @Override
+    public boolean removeHypervisorVMOutOfBand(String hostIp, String vmName, 
Map<String, String> params) {
+        s_logger.debug(String.format("Removing VM %s on external vCenter %s", 
vmName, hostIp));
+        String vcenter = params.get(VmDetailConstants.VMWARE_VCENTER);
+        String datacenter = params.get(VmDetailConstants.VMWARE_DATACENTER);
+        String username = 
params.get(VmDetailConstants.VMWARE_VCENTER_USERNAME);
+        String password = 
params.get(VmDetailConstants.VMWARE_VCENTER_PASSWORD);
+        try {
+            VmwareContext context = connectToVcenter(vcenter, username, 
password);
+            DatacenterMO dataCenterMO = new DatacenterMO(context, datacenter);
+            VirtualMachineMO vmMo = dataCenterMO.findVm(vmName);
+            return vmMo.destroy();

Review Comment:
   is it possible NPE here ?



##########
plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/api/command/admin/zone/ListVmwareDcStoppedVmsCmd.java:
##########
@@ -0,0 +1,138 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.zone;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.hypervisor.vmware.VmwareDatacenterService;
+import com.cloud.hypervisor.vmware.mo.VmwareStoppedVmInDatacenter;
+import com.cloud.user.Account;
+import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseListCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.VmwareDatacenterResponse;
+import org.apache.cloudstack.api.response.VmwareStoppedVmResponse;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import javax.inject.Inject;
+import java.util.ArrayList;
+import java.util.List;
+
+@APICommand(name = "listVmwareDcStoppedVms", responseObject = 
VmwareStoppedVmResponse.class,

Review Comment:
   one more question, can we clone a running vm in vmware vcenter, and then 
import the clonevm into kvm ?



##########
plugins/hypervisors/vmware/src/main/java/org/apache/cloudstack/api/command/admin/zone/ListVmwareDcStoppedVmsCmd.java:
##########
@@ -0,0 +1,138 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.zone;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.hypervisor.vmware.VmwareDatacenterService;
+import com.cloud.hypervisor.vmware.mo.VmwareStoppedVmInDatacenter;
+import com.cloud.user.Account;
+import com.cloud.utils.exception.CloudRuntimeException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseListCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.VmwareDatacenterResponse;
+import org.apache.cloudstack.api.response.VmwareStoppedVmResponse;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import javax.inject.Inject;
+import java.util.ArrayList;
+import java.util.List;
+
+@APICommand(name = "listVmwareDcStoppedVms", responseObject = 
VmwareStoppedVmResponse.class,

Review Comment:
   why only Stopped ?
   it would be better to list All VMs



##########
engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java:
##########
@@ -220,6 +235,20 @@ protected Void 
createTemplateAsyncCallback(AsyncCallbackDispatcher<? extends Bas
                     LOGGER.debug("Template is already in DOWNLOADED state, 
ignore further incoming DownloadAnswer");
                 }
                 return null;
+            } else if (template.isMigratedFromVmwareVM() && answer != null && 
answer.getDownloadStatus() == VMTemplateStorageResourceAssoc.Status.DOWNLOADED) 
{
+                OVFInformationTO ovfInformationTO = 
answer.getOvfInformationTO();
+                if (ovfInformationTO != null) {
+                    List<DatadiskTO> disks = ovfInformationTO.getDisks();
+                    if (CollectionUtils.isNotEmpty(disks)) {
+                        
persistTemplateDisksAsChildrenFromMigratedVmwareVm(template.getId(), disks, 
store.getId());
+                    }
+                }
+                List<VMTemplateDetailVO> details = 
templateDetailsDao.listDetails(template.getId());
+                if (CollectionUtils.isNotEmpty(details)) {
+                    Map<String, String> params = createRemoveDetails(details);
+                    HypervisorGuru vmwareGuru = 
hypervisorGuruManager.getGuru(Hypervisor.HypervisorType.VMware);
+                    
vmwareGuru.removeHypervisorVMOutOfBand(params.get(VmDetailConstants.VMWARE_HOST),
 params.get(VmDetailConstants.VMWARE_VM_NAME), params);

Review Comment:
   can define a method with `params` as the only parameter



##########
engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java:
##########
@@ -84,7 +85,8 @@ public class DefaultEndPointSelector implements 
EndPointSelector {
                             + "left join host_details hd on h.id=hd.host_id 
and hd.name='" + HOST_VOLUME_ENCRYPTION + "' "
                             + "where h.status = 'Up' and h.type = 'Routing' 
and h.resource_state = 'Enabled' and s.pool_id = ? ";
 
-    private String findOneHypervisorHostInScopeByType = "select h.id from host 
h where h.status = 'Up' and h.hypervisor_type = ? ";
+    private String findOneHypervisorHostInScopeByHypervisorType = "select h.id 
from " +
+            "host h where h.status = 'Up' and h.type = 'Routing' and 
h.resource_state = 'Enabled' and h.hypervisor_type = ? ";

Review Comment:
   `h.resource_state = 'Enabled'`
   
   can disabled hosts handle the process ?



##########
api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateFromVMwareVMCmd.java:
##########
@@ -0,0 +1,155 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.template;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.template.VirtualMachineTemplate;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.TemplateResponse;
+import org.apache.cloudstack.api.response.VmwareDatacenterResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.List;
+
+@APICommand(name = "registerTemplateFromVmware",
+        description = "Registers a template from a stopped VM in an existing 
VMware vCenter.",
+        responseObject = TemplateResponse.class, responseView = 
ResponseObject.ResponseView.Restricted,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class RegisterTemplateFromVMwareVMCmd extends RegisterTemplateCmd {
+
+    @Parameter(name = ApiConstants.HOST_IP,
+            type = BaseCmd.CommandType.STRING,
+            description = "VMware ESXi host IP/Name.")
+    private String host;
+
+    @Parameter(name = ApiConstants.VIRTUAL_MACHINE_NAME,
+            type = BaseCmd.CommandType.STRING,
+            description = "VMware VM Name.")
+    private String vmName;

Review Comment:
   is there any parameters required ? for example vmName
   if yes, please add `required = true`



##########
api/src/main/java/com/cloud/vm/VmDetailConstants.java:
##########
@@ -84,4 +84,15 @@ public interface VmDetailConstants {
     String DEPLOY_AS_IS_CONFIGURATION = "configurationId";
     String KEY_PAIR_NAMES = "keypairnames";
     String CKS_CONTROL_NODE_LOGIN_USER = "controlNodeLoginUser";
+
+    // VMware to KVM VM migrations specific
+    String VMWARE_TO_KVM_PREFIX = "vmw-to-kvm";

Review Comment:
   ```suggestion
       String VMWARE_TO_KVM_PREFIX = "vmware-to-kvm";
   ```



##########
api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateFromVMwareVMCmd.java:
##########
@@ -0,0 +1,155 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.template;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.template.VirtualMachineTemplate;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ResponseObject;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.TemplateResponse;
+import org.apache.cloudstack.api.response.VmwareDatacenterResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.List;
+
+@APICommand(name = "registerTemplateFromVmware",
+        description = "Registers a template from a stopped VM in an existing 
VMware vCenter.",
+        responseObject = TemplateResponse.class, responseView = 
ResponseObject.ResponseView.Restricted,
+        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class RegisterTemplateFromVMwareVMCmd extends RegisterTemplateCmd {
+
+    @Parameter(name = ApiConstants.HOST_IP,
+            type = BaseCmd.CommandType.STRING,
+            description = "VMware ESXi host IP/Name.")
+    private String host;
+
+    @Parameter(name = ApiConstants.VIRTUAL_MACHINE_NAME,
+            type = BaseCmd.CommandType.STRING,
+            description = "VMware VM Name.")
+    private String vmName;
+
+    @Parameter(name = ApiConstants.ZONE_ID,
+            type = CommandType.UUID,
+            entityType = ZoneResponse.class,
+            description = "KVM zone where the end result template is 
registered")
+    private Long zoneId;
+
+    @Parameter(name = ApiConstants.EXISTING_VCENTER_ID,
+            type = CommandType.UUID,
+            entityType = VmwareDatacenterResponse.class,
+            description = "UUID of a linked existing vCenter")
+    private Long existingVcenterId;
+
+    @Parameter(name = ApiConstants.VCENTER,
+            type = CommandType.STRING,
+            description = "The name/ip of vCenter. Make sure it is IP address 
or full qualified domain name for host running vCenter server.")
+    private String vcenter;
+
+    @Parameter(name = ApiConstants.DATACENTER_NAME, type = CommandType.STRING,
+            description = "Name of VMware datacenter.")
+    private String datacenterName;
+
+    @Parameter(name = ApiConstants.CLUSTER_NAME, type = CommandType.STRING,
+            description = "Name of VMware cluster.")
+    private String clusterName;
+
+    @Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING,
+            description = "The Username required to connect to resource.")
+    private String username;
+
+    @Parameter(name = ApiConstants.PASSWORD, type = CommandType.STRING,
+            description = "The password for the specified username.")
+    private String password;
+
+    public String getVcenter() {
+        return vcenter;
+    }
+
+    public String getHost() {
+        return host;
+    }
+
+    public String getVmName() {
+        return vmName;
+    }
+
+    public Long getZoneId() {
+        return zoneId;
+    }
+
+    public String getDatacenterName() {
+        return datacenterName;
+    }
+
+    public String getUsername() {
+        return username;
+    }
+
+    public String getPassword() {
+        return password;
+    }
+
+    public String getClusterName() {
+        return clusterName;
+    }
+
+    public Long getExistingVcenterId() {
+        return existingVcenterId;
+    }
+
+    @Override
+    protected void validateParameters() {
+        if ((existingVcenterId == null && vcenter == null) || 
(existingVcenterId != null && vcenter != null)) {
+            throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
+                    "Please provide an existing vCenter ID or a vCenter 
IP/Name, parameters are mutually exclusive");
+        }
+        if (existingVcenterId == null && StringUtils.isAnyBlank(vcenter, 
datacenterName, username, password)) {
+            throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
+                    "Please set all the information for a vCenter IP/Name, 
datacenter, username and password");
+        }
+    }

Review Comment:
   there is no validation on the other parameters in the parent class 
`RegisterTemplateCmd`
   maybe call `super.validateParameters()`
   
   it might be a problem as url is now optional



##########
engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java:
##########
@@ -179,6 +174,36 @@ protected EndPoint findEndPointInScope(Scope scope, String 
sqlBase, Long poolId,
         } catch (SQLException e) {
             s_logger.warn("can't find endpoint", e);
         }
+        return host;
+    }
+
+    private void appendOrderAndLimitToSql(StringBuilder sbuilder, List<Long> 
dedicatedHosts) {
+        // TODO: order by rand() is slow if there are lot of hosts
+        sbuilder.append(" ORDER by ");
+        if (dedicatedHosts.size() > 0) {
+            moveDedicatedHostsToLowerPriority(sbuilder, dedicatedHosts);
+        }
+        sbuilder.append(" rand() limit 1");
+    }
+
+    @DB
+    protected EndPoint findEndPointInScope(Scope scope, String sqlBase, Long 
poolId, String hypervisor, boolean volumeEncryptionSupportRequired) {
+        StringBuilder sbuilder = new StringBuilder();
+        sbuilder.append(sqlBase);

Review Comment:
   ```suggestion
           StringBuilder sbuilder = new StringBuilder(sqlBase);
   ```



##########
server/src/main/java/com/cloud/template/TemplateManagerImpl.java:
##########
@@ -366,6 +372,76 @@ public VirtualMachineTemplate 
registerTemplate(RegisterTemplateCmd cmd) throws U
         }
     }
 
+    @Override
+    public VirtualMachineTemplate 
registerTemplateFromVMwareVM(RegisterTemplateFromVMwareVMCmd cmd) throws 
ResourceAllocationException {
+        Long zoneId = cmd.getZoneId();
+        Long existingVcenterId = cmd.getExistingVcenterId();
+        String vcenter = cmd.getVcenter();
+        String datacenterName = cmd.getDatacenterName();
+        String username = cmd.getUsername();
+        String password = cmd.getPassword();
+        String clusterName = cmd.getClusterName();
+        String sourceVM = cmd.getVmName();
+        String sourceHostName = cmd.getHost();
+
+        DataCenterVO zone = _dcDao.findById(zoneId);
+        if (zone == null) {
+            s_logger.error(String.format("Cannot find a zone with ID %s", 
zoneId));
+            return null;
+        }
+
+        HypervisorGuru vmwareGuru = _hvGuruMgr.getGuru(HypervisorType.VMware);
+
+        if ((existingVcenterId == null && vcenter == null) || 
(existingVcenterId != null && vcenter != null)) {

Review Comment:
   this part looks same as the validateparameters in api



##########
engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java:
##########
@@ -263,6 +292,68 @@ protected Void 
createTemplateAsyncCallback(AsyncCallbackDispatcher<? extends Bas
         return null;
     }
 
+    private Map<String, String> createRemoveDetails(List<VMTemplateDetailVO> 
details) {
+        Map<String, String> params = new HashMap<>();
+        List<String> keys = Arrays.asList(VmDetailConstants.VMWARE_VCENTER, 
VmDetailConstants.VMWARE_DATACENTER,

Review Comment:
   can be defined as a constant



##########
server/src/main/java/com/cloud/template/TemplateManagerImpl.java:
##########
@@ -366,6 +372,76 @@ public VirtualMachineTemplate 
registerTemplate(RegisterTemplateCmd cmd) throws U
         }
     }
 
+    @Override
+    public VirtualMachineTemplate 
registerTemplateFromVMwareVM(RegisterTemplateFromVMwareVMCmd cmd) throws 
ResourceAllocationException {
+        Long zoneId = cmd.getZoneId();
+        Long existingVcenterId = cmd.getExistingVcenterId();
+        String vcenter = cmd.getVcenter();
+        String datacenterName = cmd.getDatacenterName();
+        String username = cmd.getUsername();
+        String password = cmd.getPassword();
+        String clusterName = cmd.getClusterName();
+        String sourceVM = cmd.getVmName();
+        String sourceHostName = cmd.getHost();
+
+        DataCenterVO zone = _dcDao.findById(zoneId);
+        if (zone == null) {
+            s_logger.error(String.format("Cannot find a zone with ID %s", 
zoneId));
+            return null;
+        }
+
+        HypervisorGuru vmwareGuru = _hvGuruMgr.getGuru(HypervisorType.VMware);
+
+        if ((existingVcenterId == null && vcenter == null) || 
(existingVcenterId != null && vcenter != null)) {
+            throw new CloudRuntimeException("Please provide an existing 
vCenter ID or a vCenter IP/Name, parameters are mutually exclusive");
+        } else if (existingVcenterId == null && 
org.apache.commons.lang3.StringUtils.isAnyBlank(vcenter, datacenterName, 
username, password)) {
+            throw new CloudRuntimeException("Please set all the information 
for a vCenter IP/Name, datacenter, username and password");
+        }
+
+        Map<String, String> params = 
createParamsForTemplateFromVmwareVmMigration(existingVcenterId,
+                vcenter, datacenterName, username, password, clusterName, 
sourceHostName, sourceVM);
+
+        HypervisorOutOfBandVMClone cloneResult = 
vmwareGuru.cloneHypervisorVMOutOfBand(cmd.getHost(), cmd.getVmName(), params);
+        String cloneName = cloneResult.getCloneName();
+        params.put(VmDetailConstants.VMWARE_VM_NAME, cloneName);
+
+        TemplateAdapter adapter = getAdapter(HypervisorType.KVM);
+        TemplateProfile profile = 
adapter.prepareTemplateFromVmwareMigration(cmd, params, cloneResult);
+        VMTemplateVO vmTemplate = adapter.create(profile);
+
+        if (vmTemplate != null) {
+            
CallContext.current().putContextParameter(VirtualMachineTemplate.class, 
vmTemplate.getUuid());
+            return vmTemplate;
+        } else {
+            throw new CloudRuntimeException("Failed to create a template");
+        }
+    }
+
+    protected Map<String, String> 
createParamsForTemplateFromVmwareVmMigration(Long existingVcenterId,
+                                                                               
String vcenter, String datacenterName,
+                                                                               
String username, String password,
+                                                                               
String clusterName, String sourceHostName,
+                                                                               
String sourceVMName) {
+        Map<String, String> params = new HashMap<>();
+        VmwareDatacenterVO existingDC = null;
+        if (existingVcenterId != null) {
+            existingDC = vmwareDatacenterDao.findById(existingVcenterId);
+            if (existingDC == null) {
+                String err = String.format("Cannot find any existing Vmware DC 
with ID %s", existingDC);
+                s_logger.error(err);
+                throw new CloudRuntimeException(err);
+            }
+        }
+        params.put(VmDetailConstants.VMWARE_VCENTER, existingDC != null ? 
existingDC.getVcenterHost() : vcenter);
+        params.put(VmDetailConstants.VMWARE_DATACENTER, existingDC != null ? 
existingDC.getVmwareDatacenterName() : datacenterName);
+        params.put(VmDetailConstants.VMWARE_VCENTER_USERNAME, existingDC != 
null ? existingDC.getUser() : username);
+        params.put(VmDetailConstants.VMWARE_VCENTER_PASSWORD, existingDC != 
null ? existingDC.getPassword() : password);
+        params.put(VmDetailConstants.VMWARE_CLUSTER, clusterName);
+        params.put(VmDetailConstants.VMWARE_HOST, sourceHostName);
+        params.put(VmDetailConstants.VMWARE_VM_NAME, sourceVMName);

Review Comment:
   can we use the same variable/constant name ? for example
   
   ```suggestion
           params.put(VmDetailConstants.VMWARE_VCENTER_HOST, existingDC != null 
? existingDC.getVcenterHost() : vcenterHost);
           params.put(VmDetailConstants.VMWARE_DATACENTER_NAME, existingDC != 
null ? existingDC.getVmwareDatacenterName() : datacenterName);
           params.put(VmDetailConstants.VMWARE_VCENTER_USERNAME, existingDC != 
null ? existingDC.getUserName() : username);
           params.put(VmDetailConstants.VMWARE_VCENTER_PASSWORD, existingDC != 
null ? existingDC.getPassword() : password);
           params.put(VmDetailConstants.VMWARE_CLUSTER_NAME, clusterName);
           params.put(VmDetailConstants.VMWARE_HOST_NAME, sourceHostName);
           params.put(VmDetailConstants.VMWARE_VM_NAME, sourceVMName);
   ```
   



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDownloadCommandWrapper.java:
##########
@@ -0,0 +1,230 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.storage.DownloadAnswer;
+import com.cloud.agent.api.to.DatadiskTO;
+import com.cloud.agent.api.to.OVFInformationTO;
+import com.cloud.agent.api.to.VmwareVmForMigrationTO;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser;
+import com.cloud.hypervisor.kvm.resource.LibvirtVMDef;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.storage.VMTemplateStorageResourceAssoc;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.storage.command.DownloadCommand;
+import org.apache.commons.io.IOUtils;
+import org.apache.log4j.Logger;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URLEncoder;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
+@ResourceWrapper(handles =  DownloadCommand.class)
+public class LibvirtDownloadCommandWrapper extends 
CommandWrapper<DownloadCommand, Answer, LibvirtComputingResource> {
+
+    private static final Logger s_logger = 
Logger.getLogger(LibvirtDownloadCommandWrapper.class);
+
+    @Override
+    public Answer execute(DownloadCommand cmd, LibvirtComputingResource 
serverResource) {
+        s_logger.info("Executing DownloadCommand");
+        final KVMStoragePoolManager storagePoolMgr = 
serverResource.getStoragePoolMgr();
+        final long templateId = cmd.getId();
+        final long accountId = cmd.getAccountId();
+        String templateDescription = cmd.getDescription();
+        String templateUniqueName = cmd.getName();
+
+        VmwareVmForMigrationTO vmForMigrationTO = 
cmd.getVmwareVmForMigrationTO();
+        String vcenter = vmForMigrationTO.getVcenter();
+        String datacenter = vmForMigrationTO.getDatacenter();
+        String username = vmForMigrationTO.getUsername();
+        String password = vmForMigrationTO.getPassword();
+        String host = vmForMigrationTO.getHost();
+        String vmName = vmForMigrationTO.getVmName();
+        String cluster = vmForMigrationTO.getCluster();
+        String secondaryStorageUrl = vmForMigrationTO.getSecondaryStorageUrl();
+
+        KVMStoragePool secondaryPool = 
storagePoolMgr.getStoragePoolByURI(secondaryStorageUrl);
+        String mountPoint = secondaryPool.getLocalPath();
+        s_logger.info(String.format("Secondary storage pool: uuid = %s, local 
path = %s", secondaryPool.getUuid(), mountPoint));
+
+        String encodedUsername = encodeUsername(username);
+        s_logger.info(String.format("Encoded username: %s", encodedUsername));
+        String url = String.format("vpx://%s@%s/%s/%s/%s?no_verify=1",
+                encodedUsername, vcenter, datacenter, cluster, host);
+
+        s_logger.info(String.format("Migrating VM from vCenter: %s", vcenter));
+
+        String passwordFile = String.format("/tmp/vmw-%s", UUID.randomUUID());
+        Script.runSimpleBashScript(String.format("echo \"%s\" > %s", password, 
passwordFile));
+
+        final String templateFolder = String.format("template/tmpl/%s/%s/", 
accountId, templateId);
+        final String templateInstallFolder = String.format("%s/%s", 
mountPoint, templateFolder);
+        final String baseName = UUID.randomUUID().toString();
+        final String installBasePath = templateInstallFolder + baseName;
+        s_logger.info(String.format("Creating template folder %s", 
templateInstallFolder));
+        Script.runSimpleBashScript(String.format("mkdir -p %s", 
templateInstallFolder));
+
+        try {
+            int timeout = cmd.getWait();
+            Script script = new Script("virt-v2v", timeout, s_logger);
+            script.add("--root", "first");
+            script.add("-ic", url);
+            script.add(vmName);
+            script.add("--password-file", passwordFile);
+            script.add("-o", "local");
+            script.add("-os", templateInstallFolder);
+            script.add("-of", "qcow2");
+            script.add("-on", baseName);
+            String result = script.execute();
+            s_logger.info(String.format("Execution result: %s", result));
+
+            s_logger.info("Finished downloading template from Vmware 
migration, checking for template sizes");
+            List<LibvirtVMDef.DiskDef> disks = 
getConvertedVmDisks(installBasePath);
+            if (disks.size() < 1) {
+                String err = String.format("Cannot find any disk for the 
migrated VM %s on path: %s", vmName, installBasePath);
+                s_logger.error(err);
+                return new DownloadAnswer(err, 
VMTemplateStorageResourceAssoc.Status.DOWNLOAD_ERROR);
+            }
+
+            List<DatadiskTO> vmDisks = getVMDisksFromDiskDefList(disks, 
mountPoint);
+            List<DatadiskTO> dataDisks = getVMDataDisks(vmDisks);
+            DatadiskTO rootDisk = vmDisks.get(0);
+            String installPath = rootDisk.getPath();
+            long virtualSize = rootDisk.getVirtualSize();
+            long physicalSize = rootDisk.getFileSize();
+
+            createTemplatePropertiesFileAfterMigration(templateInstallFolder, 
templateId, accountId,
+                    templateDescription, installPath, virtualSize, 
physicalSize, templateUniqueName);
+
+            DownloadAnswer answer = new DownloadAnswer(null, 100, null, 
VMTemplateStorageResourceAssoc.Status.DOWNLOADED,
+                    null, installPath, virtualSize, physicalSize, null);
+            answer.setOvfInformationTO(new OVFInformationTO(dataDisks));
+            return answer;
+        } catch (Exception e) {
+            String error = String.format("Error migrating VM from vcenter %s, 
%s", vcenter, e.getMessage());
+            s_logger.error(error, e);
+            return new DownloadAnswer(error, 
VMTemplateStorageResourceAssoc.Status.DOWNLOAD_ERROR);
+        } finally {
+            Script.runSimpleBashScript(String.format("rm -rf %s", 
passwordFile));
+        }
+    }
+
+    private void createTemplatePropertiesFileAfterMigration(String 
templateInstallFolder, long templateId,
+                                                            long accountId, 
String templateDescription,
+                                                            String 
installPath, long virtualSize,
+                                                            long physicalSize, 
String templateUniqueName) throws IOException {
+        String templateFilePath = String.format("%s%s", templateInstallFolder, 
"template.properties");
+        final File templateProp = new File(templateFilePath);
+        if (!templateProp.exists()) {
+            templateProp.createNewFile();
+        }
+        String pathPrefix = String.format("template/tmpl/%s/%s", accountId, 
templateId);
+        String relativePath = removeMountPointFromDiskPath(installPath, 
pathPrefix);
+
+        StringBuilder stringBuilder = new StringBuilder();
+        stringBuilder.append(String.format("%s=%s%s", "uniquename", 
templateUniqueName, System.getProperty("line.separator")));

Review Comment:
   can extract to a method 



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to