Alissa Bonas has posted comments on this change.
Change subject: core: check no running VMs use storage conn
......................................................................
Patch Set 4: (14 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
Line 102: }
Line 103:
Line 104: if (doDomainsUseConnection(newConnectionDetails) ||
doLunsUseConnection(newConnectionDetails)) {
Line 105: if (domains.size() == 1) {
Line 106: setStorageDomain(domains.get(0));
yep you're right. moved to use only the "domains" member for all cases.
Line 107: }
Line 108: else if (storageType.isFileDomain() && domains.size() >
0) {
Line 109: String domainNames = createDomainNamesList(domains);
Line 110: addCanDoActionMessage(String.format("$domainNames
%1$s", domainNames));
Line 138:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_CONNECTION_UNSUPPORTED_ACTION_FOR_DOMAINS_MAINTENANCE);
Line 139: }
Line 140: return isConnectionEditable;
Line 141: }
Line 142: else {
Done
Line 143: if (luns != null && !luns.isEmpty()) {
Line 144: List<String> vmNames = new ArrayList();
Line 145: List<String> domainNames = new ArrayList();
Line 146: for (LUNs lun : luns) {
Line 139: }
Line 140: return isConnectionEditable;
Line 141: }
Line 142: else {
Line 143: if (luns != null && !luns.isEmpty()) {
Unfortunately, this method was added in commons 3.2, while we are using 3.1
http://upstream-tracker.org/java/compat_reports/commons-collections/3.1_to_3.2/src_compat_report.html
Line 144: List<String> vmNames = new ArrayList();
Line 145: List<String> domainNames = new ArrayList();
Line 146: for (LUNs lun : luns) {
Line 147: NGuid diskId = lun.getDiskId();
Line 141: }
Line 142: else {
Line 143: if (luns != null && !luns.isEmpty()) {
Line 144: List<String> vmNames = new ArrayList();
Line 145: List<String> domainNames = new ArrayList();
Done
Line 146: for (LUNs lun : luns) {
Line 147: NGuid diskId = lun.getDiskId();
Line 148: if (diskId != null) {
Line 149: Map<Boolean, List<VM>> vmsMap =
getVmDAO().getForDisk(diskId.getValue());
Line 146: for (LUNs lun : luns) {
Line 147: NGuid diskId = lun.getDiskId();
Line 148: if (diskId != null) {
Line 149: Map<Boolean, List<VM>> vmsMap =
getVmDAO().getForDisk(diskId.getValue());
Line 150: List<VM> pluggedVms =
vmsMap.get(Boolean.TRUE);
nice idea, not in this patch IMO.
Line 151: if (pluggedVms != null &&
!pluggedVms.isEmpty()) {
Line 152: for (VM vm : pluggedVms) {
Line 153: if
(!vm.getStatus().equals(VMStatus.Down)) {
Line 154: vmNames.add(vm.getName());
Line 163: String domainName =
domain.getStorageName();
Line 164: domainNames.add(domainName);
Line 165: }
Line 166: else {
Line 167: if (domains == null) {
Done
Line 168: domains = new ArrayList();
Line 169: }
Line 170: domains.add(domain);
Line 171: }
Line 218: if (!hasConnectStorageSucceeded) {
Line 219: setSucceeded(false);
Line 220: VdcFault f = new VdcFault();
Line 221: f.setError(VdcBllErrors.StorageServerConnectionError);
Line 222: getReturnValue().setFault(f);
Done
Line 223: return;
Line 224: }
Line 225: for (StorageDomain domain : domains) {
Line 226: map = getStoragePoolIsoMap(domain);
Line 223: return;
Line 224: }
Line 225: for (StorageDomain domain : domains) {
Line 226: map = getStoragePoolIsoMap(domain);
Line 227: changeStorageDomainStatusInTransaction(map,
StorageDomainStatus.Locked);
Done
Line 228: // connect to the new path
Line 229: // update info such as free space - because we
switched to a different server
Line 230: returnValueUpdatedStorageDomain =
getStatsForDomain(domain);
Line 231: if (returnValueUpdatedStorageDomain.getSucceeded()) {
Line 224: }
Line 225: for (StorageDomain domain : domains) {
Line 226: map = getStoragePoolIsoMap(domain);
Line 227: changeStorageDomainStatusInTransaction(map,
StorageDomainStatus.Locked);
Line 228: // connect to the new path
Done
Line 229: // update info such as free space - because we
switched to a different server
Line 230: returnValueUpdatedStorageDomain =
getStatsForDomain(domain);
Line 231: if (returnValueUpdatedStorageDomain.getSucceeded()) {
Line 232: final StorageDomain updatedStorageDomain =
Line 233: (StorageDomain)
returnValueUpdatedStorageDomain.getReturnValue();
Line 234: executeInNewTransaction(new
TransactionMethod<Void>() {
Line 235: @Override
Line 236: public Void runInTransaction() {
Line 237:
getStorageDomainDynamicDao().update(updatedStorageDomain.getStorageDynamicData());
Done
Line 238: return null;
Line 239: }
Line 240: });
Line 241:
Line 241:
Line 242: }
Line 243: else {
Line 244: isUpdateSucceeded = false;
Line 245: break;
Done
Line 246: }
Line 247: }
Line 248: }
Line 249: if(isUpdateSucceeded) {
Line 271: }
Line 272:
Line 273: protected boolean doLunsUseConnection(StorageServerConnections
connection) {
Line 274: luns =
getLunDao().getAllForStorageServerConnection(connection.getid());
Line 275: return !luns.isEmpty();
Done
Line 276: }
Line 277:
Line 278: protected StoragePoolIsoMap getStoragePoolIsoMap(StorageDomain
storageDomain) {
Line 279: StoragePoolIsoMapId mapId = new
StoragePoolIsoMapId(storageDomain.getId(),
Line 354: return getDbFacade().getStoragePoolIsoMapDao();
Line 355: }
Line 356:
Line 357: @Override
Line 358: protected Map<String, Pair<String, String>> getExclusiveLocks() {
will do. next patchset locks domains and connection. will add lun disks and VMs
in the patchset after that one.
Line 359: Map<String, Pair<String, String>> locks = new HashMap<String,
Pair<String, String>>();
Line 360: domains =
getStorageDomainsByConnId(getParameters().getStorageServerConnection().getid());
Line 361: if (!domains.isEmpty() && domains.size() == 1) {
Line 362: setStorageDomain(domains.get(0));
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommandTest.java
Line 330: Guid storageDomainId = Guid.NewGuid();
Line 331: lun3.setStorageDomainId(storageDomainId);
Line 332: lun3.setvolume_group_id(Guid.NewGuid().toString());
Line 333: luns.add(lun3);
Line 334: Map<Boolean, List<VM>> vmsMap = new HashMap();
Done
Line 335: VM vm1 = new VM();
Line 336: vm1.setName("vm1");
Line 337: vm1.setStatus(VMStatus.Up);
Line 338: VM vm2 = new VM();
--
To view, visit http://gerrit.ovirt.org/16203
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I11fe22760952e1319654647eed661a7814675f64
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
