Skip to content

Fix local storage deletion cases #10231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
harikrishna-patnala merged 4 commits into from
Jan 23, 2025
Merged

Fix local storage deletion cases #10231

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e82435
Delete local storage properties in agent.properties during delete pool
harikrishna-patnala Jan 21, 2025
7e4860a
Fix stale entry when add local storage failed
harikrishna-patnala Jan 21, 2025
4246ab3
Smaller methods
harikrishna-patnala Jan 22, 2025
dcea74e
Comment added
harikrishna-patnala Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...rvisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv

protected static final String LOCAL_STORAGE_PATH = "local.storage.path";
protected static final String LOCAL_STORAGE_UUID = "local.storage.uuid";
protected static final String DEFAULT_LOCAL_STORAGE_PATH = "/var/lib/libvirt/images/";
public static final String DEFAULT_LOCAL_STORAGE_PATH = "/var/lib/libvirt/images";

protected List<String> localStoragePaths = new ArrayList<>();
protected List<String> localStorageUUIDs = new ArrayList<>();
Expand Down
36 changes: 36 additions & 0 deletions ...ava/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteStoragePoolCommandWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,20 @@
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.DeleteStoragePoolCommand;
import com.cloud.agent.api.to.StorageFilerTO;
import com.cloud.agent.dao.impl.PropertiesStorage;
import com.cloud.agent.properties.AgentProperties;
import com.cloud.agent.properties.AgentPropertiesFileHandler;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
import com.cloud.resource.CommandWrapper;
import com.cloud.resource.ResourceWrapper;
import com.cloud.storage.Storage;
import com.cloud.utils.exception.CloudRuntimeException;

import java.util.Arrays;
import java.util.HashMap;
import java.util.stream.Collectors;

@ResourceWrapper(handles = DeleteStoragePoolCommand.class)
public final class LibvirtDeleteStoragePoolCommandWrapper extends CommandWrapper<DeleteStoragePoolCommand, Answer, LibvirtComputingResource> {
@Override
Expand All @@ -39,6 +47,34 @@ public Answer execute(final DeleteStoragePoolCommand command, final LibvirtCompu
final KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr();

storagePoolMgr.deleteStoragePool(pool.getType(), pool.getUuid());
if (Storage.StoragePoolType.Filesystem.equals(pool.getType()) && !libvirtComputingResource.DEFAULT_LOCAL_STORAGE_PATH.equals(pool.getPath())) {
String localStoragePath = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.LOCAL_STORAGE_PATH);
String localStorageUuid = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.LOCAL_STORAGE_UUID);

String uuidToRemove = pool.getUuid();
String pathToRemove = pool.getPath();

if (localStorageUuid != null && uuidToRemove != null) {
localStorageUuid = Arrays.stream(localStorageUuid.split(","))
.filter(uuid -> !uuid.equals(uuidToRemove))
.collect(Collectors.joining(","));
}

if (localStoragePath != null && pathToRemove != null) {
localStoragePath = Arrays.stream(localStoragePath.split(","))
.filter(path -> !path.equals(pathToRemove))
.collect(Collectors.joining(","));
}

PropertiesStorage agentProperties = new PropertiesStorage();
agentProperties.configure("AgentProperties", new HashMap<String, Object>());
if (localStorageUuid != null) {
agentProperties.persist(AgentProperties.LOCAL_STORAGE_UUID.getName(), localStorageUuid);
}
if (localStoragePath != null) {
agentProperties.persist(AgentProperties.LOCAL_STORAGE_PATH.getName(), localStoragePath);
}
}
Comment thread
harikrishna-patnala marked this conversation as resolved.
Outdated
}

return new Answer(command);
Expand Down
14 changes: 11 additions & 3 deletions server/src/main/java/com/cloud/storage/StorageManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,9 @@ public DataStore createLocalStorage(Host host, StoragePoolInfo pInfo) throws Con
if (!(dc.isLocalStorageEnabled() || useLocalStorageForSystemVM)) {
return null;
}
DataStore store;
DataStore store = null;
DataStoreProvider provider = _dataStoreProviderMgr.getDefaultPrimaryDataStoreProvider();
DataStoreLifeCycle lifeCycle = provider.getDataStoreLifeCycle();
try {
String hostAddress = pInfo.getHost();
if (host.getHypervisorType() == Hypervisor.HypervisorType.VMware) {
Expand All @@ -829,8 +831,6 @@ public DataStore createLocalStorage(Host host, StoragePoolInfo pInfo) throws Con
}
}

DataStoreProvider provider = _dataStoreProviderMgr.getDefaultPrimaryDataStoreProvider();
DataStoreLifeCycle lifeCycle = provider.getDataStoreLifeCycle();
if (pool == null) {
Map<String, Object> params = new HashMap<String, Object>();
String name = pInfo.getName() != null ? pInfo.getName() : createLocalStoragePoolName(host, pInfo);
Expand Down Expand Up @@ -860,6 +860,14 @@ public DataStore createLocalStorage(Host host, StoragePoolInfo pInfo) throws Con

} catch (Exception e) {
s_logger.warn("Unable to setup the local storage pool for " + host, e);
try {
if (store != null) {
s_logger.debug(String.format("Trying to delete storage pool entry if exists %s", store));
lifeCycle.deleteDataStore(store);
}
} catch (Exception ex) {
s_logger.debug(String.format("Failed to clean up local storage pool: %s", ex.getMessage()));
}
throw new ConnectionException(true, "Unable to setup the local storage pool for " + host, e);
}

Expand Down