liron aravot has posted comments on this change.
Change subject: backend,webadmin: Enable virtual drive size extension
......................................................................
Patch Set 13: (4 inline comments)
partial review..will continue.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExtendImageSizeCommand.java
Line 41: }
Line 42:
Line 43: @Override
Line 44: protected boolean canDoAction() {
Line 45: return isDiskExists() && isNewSizeValid() && isVMStatusValid();
we need to propagate the CDA messages to the caller.
Line 46: }
Line 47:
Line 48: protected boolean isDiskExists() {
Line 49: if (getImage() == null) {
Line 105:
Line 106: setSucceeded(true);
Line 107: }
Line 108:
Line 109: private void updateRelevantVms() {
I still don't get this method,
what is the disk isn't plugged to any VM at the moment? what if we have more
then 1 vm that the disk is plugged to and they are DOWN? it won't be executed..
don't we need to sync between the qcow header and the metadata then? it's not
clear
Line 110: List<VM> vms = getVmsDiskPluggedTo();
Line 111:
Line 112: for (VM vm : vms) {
Line 113: // All running VMs must be updated that their virtual
disk is grown, otherwise the change will be
Line 111:
Line 112: for (VM vm : vms) {
Line 113: // All running VMs must be updated that their virtual
disk is grown, otherwise the change will be
Line 114: // visible after next restart. If disk is plugged to the
single VM, this step must be executed anyway -
Line 115: // for QCOW disk format it must sync between QCOW header
and volume metadata.
what if we fail to sync, the disk is extended but when we will run vm we will
never see it? or will it somehow get synced?
Line 116: if (vm.getStatus().isUpOrPaused() || vms.size() == 1) {
Line 117: try {
Line 118: VDSReturnValue ret = extendVmDiskSize(vm,
getParameters().getNewSize());
Line 119: if (!ret.getSucceeded()) {
Line 223: private List<VM> getVmsDiskPluggedTo() {
Line 224: if (vmsDiskPluggedTo == null) {
Line 225: vmsDiskPluggedTo =
getVmDAO().getForDisk(getImage().getId()).get(Boolean.TRUE);
Line 226:
Line 227: if (vmsDiskPluggedTo == null) {
the dao will return you empty list in .
Line 228: vmsDiskPluggedTo = Collections.emptyList();
Line 229: }
Line 230: }
Line 231: return vmsDiskPluggedTo;
--
To view, visit http://gerrit.ovirt.org/14975
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie702348a68a26ac02a01f66aaa1ea42c2c675ebb
Gerrit-PatchSet: 13
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: A Burden <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Cheryn Tan <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: liron aravot <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
