Address review comments

This commit is contained in:
nvazquez 2026-05-13 09:18:55 -03:00
parent b27991d645
commit fbe7ac61d3
No known key found for this signature in database
GPG Key ID: 656E1BCC8CB54F84
5 changed files with 15 additions and 40 deletions

View File

@ -1,23 +0,0 @@
//
// 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.agent.api;
public class CleanupConvertedInstanceDisksAnswer extends Answer {
}

View File

@ -51,7 +51,7 @@ import java.util.List;
import java.util.UUID;
import java.util.stream.Collectors;
public abstract class LibvirtBaseConvertCommandWrapper <T extends Command, A extends Answer, R extends ServerResource> extends CommandWrapper<Command, Answer, ServerResource> {
public abstract class LibvirtBaseConvertCommandWrapper <T extends Command, A extends Answer, R extends ServerResource> extends CommandWrapper<T, A, R> {
protected KVMStoragePool getTemporaryStoragePool(DataStoreTO conversionTemporaryLocation, KVMStoragePoolManager storagePoolMgr) {
if (conversionTemporaryLocation instanceof NfsTO) {
@ -237,8 +237,10 @@ public abstract class LibvirtBaseConvertCommandWrapper <T extends Command, A ext
logger.error(err);
throw new CloudRuntimeException(err);
}
InputStream is = new BufferedInputStream(new FileInputStream(xmlPath));
String xml = IOUtils.toString(is, Charset.defaultCharset());
String xml;
try (InputStream is = new BufferedInputStream(new FileInputStream(xmlPath))) {
xml = IOUtils.toString(is, Charset.defaultCharset());
}
final LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
try {
parser.parseDomainXML(xml);

View File

@ -20,7 +20,6 @@ package com.cloud.hypervisor.kvm.resource.wrapper;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.CleanupConvertedInstanceDisksCommand;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.to.DataStoreTO;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser;
@ -28,7 +27,6 @@ import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk;
import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
import com.cloud.resource.ResourceWrapper;
import com.cloud.resource.ServerResource;
import java.io.File;
import java.util.List;
@ -37,11 +35,9 @@ import java.util.List;
public class LibvirtCleanupConvertedInstanceDisksCommandWrapper extends LibvirtBaseConvertCommandWrapper<CleanupConvertedInstanceDisksCommand, Answer, LibvirtComputingResource> {
@Override
public Answer execute(Command command, ServerResource resource) {
CleanupConvertedInstanceDisksCommand cmd = (CleanupConvertedInstanceDisksCommand) command;
LibvirtComputingResource serverResource = (LibvirtComputingResource) resource;
DataStoreTO vmVolumesStore = cmd.getVmVolumesStore();
String vmVolumesPrefix = cmd.getVmVolumesPrefix();
public Answer execute(CleanupConvertedInstanceDisksCommand command, LibvirtComputingResource serverResource) {
DataStoreTO vmVolumesStore = command.getVmVolumesStore();
String vmVolumesPrefix = command.getVmVolumesPrefix();
final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
KVMStoragePool conversionPool = getTemporaryStoragePool(vmVolumesStore, storagePoolMgr);
@ -53,7 +49,7 @@ public class LibvirtCleanupConvertedInstanceDisksCommandWrapper extends LibvirtB
boolean xmlExists = new File(xmlPath).exists();
LibvirtDomainXMLParser xmlParser = xmlExists ? parseMigratedVMXmlDomain(volumesBasePath) : null;
List<KVMPhysicalDisk> temporaryDisks = xmlExists ?
List<KVMPhysicalDisk> temporaryDisks = xmlExists && xmlParser != null ?
getTemporaryDisksFromParsedXml(conversionPool, xmlParser, volumesBasePath) :
getTemporaryDisksWithPrefixFromTemporaryPool(conversionPool, conversionPoolPath, vmVolumesPrefix);

View File

@ -20,8 +20,6 @@ package com.cloud.hypervisor.kvm.resource.wrapper;
import java.util.List;
import com.cloud.agent.api.Command;
import com.cloud.resource.ServerResource;
import org.apache.cloudstack.vm.UnmanagedInstanceTO;
import com.cloud.agent.api.Answer;
@ -42,9 +40,7 @@ import com.cloud.resource.ResourceWrapper;
public class LibvirtImportConvertedInstanceCommandWrapper extends LibvirtBaseConvertCommandWrapper<ImportConvertedInstanceCommand, Answer, LibvirtComputingResource> {
@Override
public Answer execute(Command command, ServerResource resource) {
ImportConvertedInstanceCommand cmd = (ImportConvertedInstanceCommand) command;
LibvirtComputingResource serverResource = (LibvirtComputingResource) resource;
public Answer execute(ImportConvertedInstanceCommand cmd, LibvirtComputingResource serverResource) {
RemoteInstanceTO sourceInstance = cmd.getSourceInstance();
Hypervisor.HypervisorType sourceHypervisorType = sourceInstance.getHypervisorType();
String sourceInstanceName = sourceInstance.getInstanceName();

View File

@ -2248,7 +2248,11 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
CleanupConvertedInstanceDisksCommand cleanupCommand =
new CleanupConvertedInstanceDisksCommand(temporaryConvertLocation, convertedDisksPrefix);
try {
agentManager.send(convertHost.getId(), cleanupCommand);
Answer cleanupAnswer = agentManager.send(convertHost.getId(), cleanupCommand);
if (!cleanupAnswer.getResult()) {
logger.warn("Failed to cleanup the converted disks for the VM {} through " +
"the conversion host {}: {}", sourceVM, convertHost.getName(), cleanupAnswer.getDetails());
}
} catch (AgentUnavailableException | OperationTimedoutException e) {
logger.error("Error cleaning up converted disks for VM {} through the conversion host {}",
sourceVM, convertHost.getName(), e);