Copilot commented on code in PR #12261:
URL: https://github.com/apache/cloudstack/pull/12261#discussion_r2619642621
##########
ui/src/views/network/VpnDetails.vue:
##########
@@ -16,40 +16,86 @@
// under the License.
<template>
- <div v-if="remoteAccessVpn">
- <div>
- <p>{{ $t('message.enabled.vpn') }} <strong>{{ remoteAccessVpn.publicip
}}</strong></p>
- <p>{{ $t('message.enabled.vpn.ip.sec') }} <strong>{{
remoteAccessVpn.presharedkey }}</strong></p>
- <a-divider/>
- <a-button
- style="margin-left: 10px"
- type="primary"
- danger
- @click="disableVpn = true"
- :disabled="!('deleteRemoteAccessVpn' in $store.getters.apis)">
- {{ $t('label.disable.vpn') }}
- </a-button>
- <a-button><router-link :to="{ path: '/vpnuser'}">{{
$t('label.manage.vpn.user') }}</router-link></a-button>
+ <div class="vpn-details">
+ <div v-if="remoteAccessVpn">
+ <div>
+ <p>{{ $t('message.enabled.vpn') }} <strong>{{ remoteAccessVpn.publicip
}}</strong></p>
+ <p>{{ $t('message.enabled.vpn.ip.sec') }} <strong>{{
remoteAccessVpn.presharedkey }}</strong></p>
+ <p>{{ $t('message.enabled.vpn.ip.range') }} <strong>{{
remoteAccessVpn.iprange }}</strong></p>
+ <a-divider/>
+ <a-button><router-link :to="{ path: '/vpnuser'}">{{
$t('label.manage.vpn.user') }}</router-link></a-button>
+ <a-button
+ style="margin-left: 10px"
+ type="primary"
+ danger
+ @click="disableVpn = true"
+ :disabled="!('deleteRemoteAccessVpn' in $store.getters.apis)">
+ {{ $t('label.disable.vpn') }}
+ </a-button>
+ </div>
+
+ <a-modal
+ :visible="disableVpn"
+ :footer="null"
+ :title="$t('label.disable.vpn')"
+ :closable="true"
+ :maskClosable="false"
+ @cancel="disableVpn = false">
+ <div v-ctrl-enter="handleDisableVpn">
+ <p>{{ $t('message.disable.vpn') }}</p>
+
+ <a-divider />
+
+ <div class="actions">
+ <a-button @click="() => disableVpn = false">{{ $t('label.cancel')
}}</a-button>
+ <a-button type="primary" @click="handleDisableVpn">{{
$t('label.yes') }}</a-button>
+ </div>
+ </div>
+ </a-modal>
+
</div>
+ <div v-else>
+ <a-button :disabled="!('createRemoteAccessVpn' in $store.getters.apis)"
type="primary" @click="enableVpn = true">
+ {{ $t('label.enable.vpn') }}
+ </a-button>
- <a-modal
- :visible="disableVpn"
- :footer="null"
- :title="$t('label.disable.vpn')"
- :closable="true"
- :maskClosable="false"
- @cancel="disableVpn = false">
- <div v-ctrl-enter="handleDisableVpn">
- <p>{{ $t('message.disable.vpn') }}</p>
+ <a-modal
+ :visible="enableVpn"
+ :footer="null"
+ :title="$t('label.enable.vpn')"
+ :maskClosable="false"
+ :closable="true"
+ @cancel="enableVpn = false">
+ <div v-ctrl-enter="handleCreateVpn">
+ <p>{{ $t('message.enable.vpn') }}</p>
+ <a-form-item>
+ <a-checkbox v-model:checked="specifyIpRange">
+ {{ $t('label.remote.access.vpn.specify.iprange') }}
+ </a-checkbox>
+ </a-form-item>
+ <a-form-item
+ v-if="specifyIpRange"
+ name="iprange"
+ :colon="false"
+ ref="iprange">
+ <template #label>
+ <tooltip-label :title="$t('label.ip.range')"
:tooltip="$t('message.remote.access.vpn.iprange.description')"/>
+ </template>
+ <a-input
+ v-model:value="vpnIpRange"
+ :placeholder="'10.1.2.1-10.1.2.8'"
+ />
+ </a-form-item>
- <a-divider />
+ <a-divider />
- <div class="actions">
- <a-button @click="() => disableVpn = false">{{ $t('label.cancel')
}}</a-button>
- <a-button type="primary" @click="handleDisableVpn">{{
$t('label.yes') }}</a-button>
+ <div class="actions">
+ <a-button @click="() => enableVpn = false">{{ $t('label.cancel')
}}</a-button>
+ <a-button type="primary" ref="submit" @click="handleCreateVpn">{{
$t('label.yes') }}</a-button>
+ </div>
</div>
- </div>
- </a-modal>
+ </a-modal>
+ </div>
</div>
<div v-else>
Review Comment:
There is a duplicate closing div tag followed by a `v-else` that seems to be
orphaned. Line 100 has a closing `</div>` tag, and line 101 has another closing
`</div>` with `v-else` attribute. This `v-else` appears misplaced as it's on a
closing tag rather than an opening tag. This could cause rendering issues or
break the component's template structure.
##########
ui/src/views/network/NicsTab.vue:
##########
@@ -104,14 +110,28 @@
</span>
</a-select-option>
</a-select>
- <p class="modal-form__label">{{ $t('label.publicip') }}:</p>
- <a-input v-model:value="addNetworkData.ip"></a-input>
- <br>
+ </a-form-item>
+ <a-form-item name="ip" ref="ip">
+ <template #label>
+ <tooltip-label :title="$t('label.ipaddress')"
:tooltip="addNetworkData.apiParams.ipaddress.description"/>
+ </template>
+ <a-input
+ :placeholder="addNetworkData.apiParams.ipaddress.description"
+ v-model:value="addNetworkData.ipaddress" />
Review Comment:
The variable name changed from `ip` to `ipaddress` but this inconsistency
with the previous usage suggests this could be incomplete. The field is named
`ip` in the data structure on line 364, but the v-model binds to
`addNetworkData.ipaddress` on line 120. This mismatch will cause the IP address
value not to be properly stored or cleared.
```suggestion
v-model:value="addNetworkData.ip" />
```
##########
ui/src/views/infra/network/IpRangesTabPublic.vue:
##########
@@ -220,6 +220,20 @@
<a-select-option v-for="pod in pods" :key="pod.id" :value="pod.id"
:label="pod.name">{{ pod.name }}</a-select-option>
</a-select>
</a-form-item>
+ <a-form-item name="isolationmethod" ref="isolationmethod"
class="form__item" v-if="!basicGuestNetwork">
+ <tooltip-label :title="$t('label.isolation.method')"
:tooltip="$t('label.choose.isolation.method.public.ip.range')"
class="tooltip-label-wrapper"/>
+ <a-select
+ v-model:value="form.isolationmethod"
+ showSearch
+ optionFilterProp="label"
+ :filterOption="(input, option) => {
+ return option.label.toLowerCase().indexOf(input.toLowerCase())
>= 0
+ }" >
+ <a-select-option value="">{{ }}</a-select-option>
Review Comment:
The empty string value for the first select option will display as blank,
making it unclear to users what this option represents. Consider adding a
meaningful label like "Default" or "None" to improve user experience.
##########
server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java:
##########
@@ -372,8 +411,6 @@ List<Ternary<VirtualMachine, Host, Host>>
getDrsPlan(Cluster cluster,
ServiceOffering serviceOffering =
vmIdServiceOfferingMap.get(vm.getId());
migrationPlan.add(new Ternary<>(vm, hostMap.get(vm.getHostId()),
hostMap.get(destHost.getId())));
- hostVmMap.get(vm.getHostId()).remove(vm);
- hostVmMap.get(destHost.getId()).add(vm);
hostVmMap.get(vm.getHostId()).remove(vm);
hostVmMap.get(destHost.getId()).add(vm);
Review Comment:
The duplicate lines 414-415 suggest an incomplete merge. Line 414 and 415
both perform the same operation: removing vm from source host and adding to
destination host. One of these duplicate blocks should be removed.
##########
server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java:
##########
@@ -118,240 +113,283 @@ public boolean maintain(DataStore store) {
@Override
public boolean maintain(DataStore store, Map<String,String> details) {
- Long userId = CallContext.current().getCallingUserId();
- User user = _userDao.findById(userId);
- Account account = CallContext.current().getCallingAccount();
StoragePoolVO pool = primaryDataStoreDao.findById(store.getId());
try {
- List<StoragePoolVO> spes = null;
- // Handling Zone and Cluster wide storage scopes.
- // if the storage is ZONE wide then we pass podid and cluster id
as null as they will be empty for ZWPS
- if (pool.getScope() == ScopeType.ZONE) {
- spes = primaryDataStoreDao.listBy(pool.getDataCenterId(),
null, null, ScopeType.ZONE);
- } else {
- spes = primaryDataStoreDao.listBy(pool.getDataCenterId(),
pool.getPodId(), pool.getClusterId(), ScopeType.CLUSTER);
- }
- for (StoragePoolVO sp : spes) {
- if (sp.getParent() != pool.getParent() && sp.getId() !=
pool.getParent()) { // If Datastore cluster is tried to prepare for maintenance
then child storage pools are also kept in PrepareForMaintenance mode
- if (sp.getStatus() ==
StoragePoolStatus.PrepareForMaintenance) {
- throw new CloudRuntimeException(String.format("Only
one storage pool in a cluster can be in PrepareForMaintenance mode, %s is
already in PrepareForMaintenance mode ", sp));
- }
- }
- }
- StoragePool storagePool = (StoragePool)store;
-
- //Handeling the Zone wide and cluster wide primay storage
- List<HostVO> hosts = new ArrayList<HostVO>();
- // if the storage scope is ZONE wide, then get all the hosts for
which hypervisor ZWSP created to send Modifystoragepoolcommand
- //TODO: if it's zone wide, this code will list a lot of hosts in
the zone, which may cause performance/OOM issue.
- if (pool.getScope().equals(ScopeType.ZONE)) {
- if (HypervisorType.Any.equals(pool.getHypervisor())) {
- hosts =
_resourceMgr.listAllUpAndEnabledHostsInOneZone(pool.getDataCenterId());
- }
- else {
- hosts =
_resourceMgr.listAllUpAndEnabledHostsInOneZoneByHypervisor(pool.getHypervisor(),
pool.getDataCenterId());
- }
- } else {
- hosts =
_resourceMgr.listHostsInClusterByStatus(pool.getClusterId(), Status.Up);
+ getStoragePoolForSpecification(pool);
+
+ List<HostVO> hosts = getHostsForStoragePool(pool);
+
+ if (setNextStateForMaintenance(hosts, pool) ==
StoragePoolStatus.PrepareForMaintenance) {
+ removeHeartbeatForHostsFromPool(hosts, pool);
+ // check to see if other ps exist
+ // if they do, then we can migrate over the system vms to them
+ // if they don't, then just stop all vms on this one
+ List<StoragePoolVO> upPools =
primaryDataStoreDao.listByStatusInZone(pool.getDataCenterId(),
StoragePoolStatus.Up);
+ boolean restart = !CollectionUtils.isEmpty(upPools);
+
+ // 2. Get a list of all the ROOT volumes within this storage
pool
+ List<VolumeVO> allVolumes =
volumeDao.findByPoolId(pool.getId());
+ // 3. Enqueue to the work queue
+ enqueueMigrationsForVolumes(allVolumes, pool);
+ // 4. Process the queue
+ processMigrationWorkloads(pool, restart);
}
+ } catch (Exception e) {
+ logger.error("Exception in enabling primary storage maintenance:",
e);
+ pool.setStatus(StoragePoolStatus.ErrorInMaintenance);
+ primaryDataStoreDao.update(pool.getId(), pool);
+ // TODO decide on what recovery is possible
+ throw new CloudRuntimeException(e.getMessage());
+ }
+ return true;
+ }
- if (hosts == null || hosts.size() == 0) {
- pool.setStatus(StoragePoolStatus.Maintenance);
- primaryDataStoreDao.update(pool.getId(), pool);
- return true;
- } else {
- // set the pool state to prepare for maintenance
- pool.setStatus(StoragePoolStatus.PrepareForMaintenance);
- primaryDataStoreDao.update(pool.getId(), pool);
- }
- // remove heartbeat
- for (HostVO host : hosts) {
- ModifyStoragePoolCommand cmd = new
ModifyStoragePoolCommand(false, storagePool);
- if (MapUtils.isNotEmpty(details) &&
storageManager.canDisconnectHostFromStoragePool(host, storagePool)) {
- cmd.setDetails(details);
- }
- final Answer answer = agentMgr.easySend(host.getId(), cmd);
- if (answer == null || !answer.getResult()) {
- if (logger.isDebugEnabled()) {
- logger.debug("ModifyStoragePool false failed due to "
+ ((answer == null) ? "answer null" : answer.getDetails()));
- }
- } else {
- if (logger.isDebugEnabled()) {
- logger.debug("ModifyStoragePool false succeeded");
- }
- if (pool.getPoolType() ==
Storage.StoragePoolType.DatastoreCluster) {
- logger.debug("Started synchronising datastore cluster
storage pool {} with vCenter", pool);
-
storageManager.syncDatastoreClusterStoragePool(pool.getId(),
((ModifyStoragePoolAnswer) answer).getDatastoreClusterChildren(), host.getId());
- }
- }
- }
- // check to see if other ps exist
- // if they do, then we can migrate over the system vms to them
- // if they don't, then just stop all vms on this one
- List<StoragePoolVO> upPools =
primaryDataStoreDao.listByStatusInZone(pool.getDataCenterId(),
StoragePoolStatus.Up);
- boolean restart = true;
- if (upPools == null || upPools.size() == 0) {
- restart = false;
- }
+ @Override
+ public boolean cancelMaintain(DataStore store) {
+ return cancelMaintain(store, null);
+ }
- // 2. Get a list of all the ROOT volumes within this storage pool
- List<VolumeVO> allVolumes = volumeDao.findByPoolId(pool.getId());
+ @Override
+ public boolean cancelMaintain(DataStore store, Map<String,String> details)
{
+ // Change the storage state back to up
+ StoragePoolVO poolVO = primaryDataStoreDao.findById(store.getId());
+ StoragePool pool = (StoragePool)store;
- // 3. Enqueue to the work queue
- for (VolumeVO volume : allVolumes) {
- VMInstanceVO vmInstance =
vmDao.findById(volume.getInstanceId());
+ List<HostVO> hosts = getHostsForStoragePool(poolVO);
- if (vmInstance == null) {
- continue;
- }
+ if (CollectionUtils.isEmpty(hosts)) {
+ return true;
+ }
- // enqueue sp work
- if (vmInstance.getState().equals(State.Running) ||
vmInstance.getState().equals(State.Starting) ||
vmInstance.getState().equals(State.Stopping)) {
-
- try {
- StoragePoolWorkVO work = new
StoragePoolWorkVO(vmInstance.getId(), pool.getId(), false, false,
server.getId());
- _storagePoolWorkDao.persist(work);
- } catch (Exception e) {
- if (logger.isDebugEnabled()) {
- logger.debug("Work record already exists, re-using
by re-setting values");
- }
- StoragePoolWorkVO work =
_storagePoolWorkDao.findByPoolIdAndVmId(pool.getId(), vmInstance.getId());
- work.setStartedAfterMaintenance(false);
- work.setStoppedForMaintenance(false);
- work.setManagementServerId(server.getId());
- _storagePoolWorkDao.update(work.getId(), work);
- }
- }
+ Pair<Map<String, String>, Boolean> nfsMountOpts =
storageManager.getStoragePoolNFSMountOpts(pool, null);
+ addHeartbeatToHostsInPool(hosts, pool, nfsMountOpts);
+
+ // 2. Get a list of pending work for this queue
+ List<StoragePoolWorkVO> pendingWork =
_storagePoolWorkDao.listPendingWorkForCancelMaintenanceByPoolId(poolVO.getId());
+
+ // 3. work through the queue
+ cancelMigrationWorkloads(pendingWork);
+ return false;
+ }
+
+ private StoragePoolStatus setNextStateForMaintenance(List<HostVO> hosts,
StoragePoolVO pool) {
+ if (CollectionUtils.isEmpty(hosts)) {
+ pool.setStatus(StoragePoolStatus.Maintenance);
+ primaryDataStoreDao.update(pool.getId(), pool);
+ return StoragePoolStatus.Maintenance;
+ } else {
+ // set the pool state to prepare for maintenance
+ pool.setStatus(StoragePoolStatus.PrepareForMaintenance);
+ primaryDataStoreDao.update(pool.getId(), pool);
+ return StoragePoolStatus.PrepareForMaintenance;
+ }
+ }
+
+ private void processMigrationWorkloads(StoragePoolVO pool, boolean
restart) throws ResourceUnavailableException, OperationTimedoutException,
InsufficientCapacityException {
+ List<StoragePoolWorkVO> pendingWork =
_storagePoolWorkDao.listPendingWorkForPrepareForMaintenanceByPoolId(pool.getId());
+
+ for (StoragePoolWorkVO work : pendingWork) {
+ // shut down the running vms
+ VMInstanceVO vmInstance = vmDao.findById(work.getVmId());
+
+ if (vmInstance == null) {
+ continue;
}
- // 4. Process the queue
- List<StoragePoolWorkVO> pendingWork =
_storagePoolWorkDao.listPendingWorkForPrepareForMaintenanceByPoolId(pool.getId());
+ switch (vmInstance.getType()) {
+ case ConsoleProxy:
+ case SecondaryStorageVm:
+ case DomainRouter:
+ handleVmMigration(restart, work, vmInstance);
+ break;
+ case User:
+ handleStopVmForMigration(work, vmInstance);
+ break;
+ }
+ }
+ }
- for (StoragePoolWorkVO work : pendingWork) {
- // shut down the running vms
+ private void cancelMigrationWorkloads(List<StoragePoolWorkVO> pendingWork)
{
+ for (StoragePoolWorkVO work : pendingWork) {
+ try {
VMInstanceVO vmInstance = vmDao.findById(work.getVmId());
if (vmInstance == null) {
continue;
}
- // if the instance is of type consoleproxy, call the console
- // proxy
- if
(vmInstance.getType().equals(VirtualMachine.Type.ConsoleProxy)) {
- // call the consoleproxymanager
- ConsoleProxyVO consoleProxy =
_consoleProxyDao.findById(vmInstance.getId());
- vmMgr.advanceStop(consoleProxy.getUuid(), false);
- // update work status
- work.setStoppedForMaintenance(true);
- _storagePoolWorkDao.update(work.getId(), work);
+ switch (vmInstance.getType()) {
+ case ConsoleProxy:
+ case SecondaryStorageVm:
+ case DomainRouter:
+ handleVmStart(work, vmInstance);
+ break;
+ case User:
+ handleUserVmStart(work, vmInstance);
+ break;
+ }
+ } catch (Exception e) {
+ logger.debug("Failed start vm", e);
+ throw new CloudRuntimeException(e.toString());
+ }
+ }
+ }
- if (restart) {
+ private void handleStopVmForMigration(StoragePoolWorkVO work, VMInstanceVO
vmInstance) throws ResourceUnavailableException, OperationTimedoutException {
+ vmMgr.advanceStop(vmInstance.getUuid(), false);
+ // update work status
+ work.setStoppedForMaintenance(true);
+ _storagePoolWorkDao.update(work.getId(), work);
+ }
- vmMgr.advanceStart(consoleProxy.getUuid(), null, null);
- // update work status
- work.setStartedAfterMaintenance(true);
- _storagePoolWorkDao.update(work.getId(), work);
- }
- }
+ private void handleVmMigration(boolean restart, StoragePoolWorkVO work,
VMInstanceVO vmInstance) throws ResourceUnavailableException,
OperationTimedoutException, InsufficientCapacityException {
+ handleStopVmForMigration(work, vmInstance);
- // if the instance is of type uservm, call the user vm manager
- if (vmInstance.getType() == VirtualMachine.Type.User) {
- UserVmVO userVm = userVmDao.findById(vmInstance.getId());
- vmMgr.advanceStop(userVm.getUuid(), false);
- // update work status
- work.setStoppedForMaintenance(true);
- _storagePoolWorkDao.update(work.getId(), work);
- }
+ if (restart) {
+ handleVmStart(work, vmInstance);
+ }
+ }
- // if the instance is of type secondary storage vm, call the
- // secondary storage vm manager
- if
(vmInstance.getType().equals(VirtualMachine.Type.SecondaryStorageVm)) {
- SecondaryStorageVmVO secStrgVm =
_secStrgDao.findById(vmInstance.getId());
- vmMgr.advanceStop(secStrgVm.getUuid(), false);
- // update work status
- work.setStoppedForMaintenance(true);
- _storagePoolWorkDao.update(work.getId(), work);
+ private void handleVmStart(StoragePoolWorkVO work, VMInstanceVO
vmInstance) throws InsufficientCapacityException, ResourceUnavailableException,
OperationTimedoutException {
+ vmMgr.advanceStart(vmInstance.getUuid(), null, null);
+ // update work queue
+ work.setStartedAfterMaintenance(true);
+ _storagePoolWorkDao.update(work.getId(), work);
+ }
- if (restart) {
- vmMgr.advanceStart(secStrgVm.getUuid(), null, null);
- // update work status
- work.setStartedAfterMaintenance(true);
- _storagePoolWorkDao.update(work.getId(), work);
- }
- }
+ private void enqueueMigrationsForVolumes(List<VolumeVO> allVolumes,
StoragePoolVO pool) {
+ for (VolumeVO volume : allVolumes) {
+ VMInstanceVO vmInstance = vmDao.findById(volume.getInstanceId());
- // if the instance is of type domain router vm, call the
network
- // manager
- if
(vmInstance.getType().equals(VirtualMachine.Type.DomainRouter)) {
- DomainRouterVO domR =
_domrDao.findById(vmInstance.getId());
- vmMgr.advanceStop(domR.getUuid(), false);
- // update work status
- work.setStoppedForMaintenance(true);
- _storagePoolWorkDao.update(work.getId(), work);
+ if (vmInstance == null) {
+ continue;
+ }
- if (restart) {
- vmMgr.advanceStart(domR.getUuid(), null, null);
- // update work status
- work.setStartedAfterMaintenance(true);
- _storagePoolWorkDao.update(work.getId(), work);
+ // enqueue sp work
+ if (vmInstance.getState().equals(State.Running) ||
vmInstance.getState().equals(State.Starting) ||
vmInstance.getState().equals(State.Stopping)) {
+
+ try {
+ StoragePoolWorkVO work = new
StoragePoolWorkVO(vmInstance.getId(), pool.getId(), false, false,
server.getId());
+ _storagePoolWorkDao.persist(work);
+ } catch (Exception e) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("Work record already exists, re-using by
re-setting values");
}
+ StoragePoolWorkVO work =
_storagePoolWorkDao.findByPoolIdAndVmId(pool.getId(), vmInstance.getId());
+ work.setStartedAfterMaintenance(false);
+ work.setStoppedForMaintenance(false);
+ work.setManagementServerId(server.getId());
+ _storagePoolWorkDao.update(work.getId(), work);
}
}
- } catch (Exception e) {
- logger.error("Exception in enabling primary storage maintenance:",
e);
- pool.setStatus(StoragePoolStatus.ErrorInMaintenance);
- primaryDataStoreDao.update(pool.getId(), pool);
- throw new CloudRuntimeException(e.getMessage());
}
- return true;
}
- @Override
- public boolean cancelMaintain(DataStore store) {
- return cancelMaintain(store, null);
+ private void removeHeartbeatForHostsFromPool(List<HostVO> hosts,
StoragePool storagePool) {
+ // remove heartbeat
+ for (HostVO host : hosts) {
+ ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(false,
storagePool);
+ final Answer answer = agentMgr.easySend(host.getId(), cmd);
+ if (answer == null || !answer.getResult()) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("ModifyStoragePool false failed due to {}",
((answer == null) ? "answer null" : answer.getDetails()));
+ }
+ } else {
+ reportSucceededModifyStorePool(storagePool,
(ModifyStoragePoolAnswer) answer, host, false);
+ }
+ }
}
- @Override
- public boolean cancelMaintain(DataStore store, Map<String,String> details)
{
- // Change the storage state back to up
- Long userId = CallContext.current().getCallingUserId();
- User user = _userDao.findById(userId);
- Account account = CallContext.current().getCallingAccount();
- StoragePoolVO poolVO = primaryDataStoreDao.findById(store.getId());
- StoragePool pool = (StoragePool)store;
+ private void reportSucceededModifyStorePool(StoragePool storagePool,
ModifyStoragePoolAnswer answer, HostVO host, boolean add) {
+ if (logger.isDebugEnabled()) {
+ logger.debug("ModifyStoragePool succeeded for {}", add ? "adding"
: "removing");
+ }
+ if (storagePool.getPoolType() ==
Storage.StoragePoolType.DatastoreCluster) {
+ logger.debug("Started synchronising datastore cluster storage pool
{} with vCenter", storagePool);
+
storageManager.syncDatastoreClusterStoragePool(storagePool.getId(),
answer.getDatastoreClusterChildren(), host.getId());
+ }
+ }
- //Handeling the Zone wide and cluster wide primay storage
- List<HostVO> hosts = new ArrayList<HostVO>();
- // if the storage scope is ZONE wide, then get all the hosts for which
hypervisor ZWSP created to send Modifystoragepoolcommand
- if (poolVO.getScope().equals(ScopeType.ZONE)) {
+ /**
+ * Handling the Zone wide and cluster wide primary storage
+ * if the storage scope is ZONE wide, then get all the hosts for which
hypervisor ZoneWideStoragePools created to send ModifyStoragePoolCommand
+ * TODO: if it's zone wide, this code will list a lot of hosts in the
zone, which may cause performance/OOM issue.
+ * @param pool pool to check for connected hosts
+ * @return a list of connected hosts
+ */
+ private List<HostVO> getHostsForStoragePool(StoragePoolVO pool) {
+ List<HostVO> hosts;
+ if (pool.getScope().equals(ScopeType.ZONE)) {
if (HypervisorType.Any.equals(pool.getHypervisor())) {
hosts =
_resourceMgr.listAllUpAndEnabledHostsInOneZone(pool.getDataCenterId());
}
else {
- hosts =
_resourceMgr.listAllUpAndEnabledHostsInOneZoneByHypervisor(poolVO.getHypervisor(),
pool.getDataCenterId());
+ hosts =
_resourceMgr.listAllUpAndEnabledHostsInOneZoneByHypervisor(pool.getHypervisor(),
pool.getDataCenterId());
}
} else {
hosts =
_resourceMgr.listHostsInClusterByStatus(pool.getClusterId(), Status.Up);
}
+ return hosts;
+ }
- if (hosts == null || hosts.size() == 0) {
- return true;
+ /**
+ * Handling Zone and Cluster wide storage scopes. Depending on the scope
of the pool, check for other storage pools in the same scope
+ * If the storage is ZONE wide then we pass podId and cluster id as null
as they will be empty for Zone wide storage
+ *
+ * @param pool pool to check for other pools in the same scope
+ */
+ private void getStoragePoolForSpecification(StoragePoolVO pool) {
+ List<StoragePoolVO> storagePools;
+ if (pool.getScope() == ScopeType.ZONE) {
+ storagePools = primaryDataStoreDao.listBy(pool.getDataCenterId(),
null, null, ScopeType.ZONE);
+ } else {
+ storagePools = primaryDataStoreDao.listBy(pool.getDataCenterId(),
pool.getPodId(), pool.getClusterId(), ScopeType.CLUSTER);
}
+ checkHierarchyForPreparingForMaintenance(pool, storagePools);
+ }
- Pair<Map<String, String>, Boolean> nfsMountOpts =
storageManager.getStoragePoolNFSMountOpts(pool, null);
- // add heartbeat
- for (HostVO host : hosts) {
- ModifyStoragePoolCommand msPoolCmd = new
ModifyStoragePoolCommand(true, pool);
- if (MapUtils.isNotEmpty(details)) {
- msPoolCmd.setDetails(details);
+ /**
+ * If Datastore cluster is tried to prepare for maintenance then child
storage pools are also kept in PrepareForMaintenance mode
+ * @param pool target to put in maintenance
+ * @param storagePools list of possible peers/parents/children
+ */
+ private static void checkHierarchyForPreparingForMaintenance(StoragePoolVO
pool, List<StoragePoolVO> storagePools) {
+ for (StoragePoolVO storagePool : storagePools) {
+ if (!(storagePool.getParent().equals(pool.getParent()) ||
!pool.getParent().equals(storagePool.getId())) &&
Review Comment:
The logic for checking hierarchy in PrepareForMaintenance mode appears
inverted. The condition `!(storagePool.getParent().equals(pool.getParent()) ||
!pool.getParent().equals(storagePool.getId()))` uses double negation which is
confusing. The expression `!(A || !B)` is equivalent to `!A && B` by De
Morgan's law, but this doesn't clearly express the intended logic. This should
be simplified for clarity and correctness.
```suggestion
if (!storagePool.getParent().equals(pool.getParent()) &&
pool.getParent().equals(storagePool.getId()) &&
```
--
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]
