Liron Aravot has posted comments on this change.
Change subject: core : create template from VM with no snappable disks (#841534)
......................................................................
Patch Set 8: (8 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java
Line 41: getVm().setvmt_guid(VmTemplateHandler.BlankVmTemplateId);
Line 42:
getVm().getStaticData().setQuotaId(getParameters().getVmStaticData().getQuotaId());
Line 43:
DbFacade.getInstance().getVmStaticDAO().update(getVm().getStaticData());
Line 44: // if there are no tasks, we can end the command right away.
Line 45: if (getTaskIdList().isEmpty()) {
i think that if there are no tasks we should end the command right way, either
with a failure or with success end command methods - which one should run
should be taken care of while trying to create tasks.
At the moment we are under the assumption that if we get to this stage we have
the
tasks that we need. we work under the same assumption that when we create
tasks and theoretically could fail in creating one needed task out of N and we
will proceed regularly. I could change this IF to be one the number of disks
instead, but i'm not sure that it will be correct the tie the end sync
operation to the number of disks exclusively.
Line 46: EndSuccessfully();
Line 47: }
Line 48: }
Line 49:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
Line 125: getCompensationContext().stateChanged();
Line 126: return null;
Line 127: }
Line 128: });
Line 129:
Done
Line 130: TransactionSupport.executeInNewTransaction(new
TransactionMethod<Void>() {
Line 131:
Line 132: @Override
Line 133: public Void runInTransaction() {
Line 363:
Line 364: for (VdcActionParametersBase p :
getParameters().getImagesParameters()) {
Line 365:
Backend.getInstance().EndAction(VdcActionType.CreateImageTemplate, p);
Line 366: }
Line 367: if (reloadVmTemplateFromDB() != null) {
basically, it shouldn't ever be null and i'm pretty sure that this check can be
removed (as the template is being unlocked here) along all other null checks
when the object is locked on DB and cannot be removed - but it's a change
unrelated to this patch, though it should be done.
Line 368: endDefaultOperations();
Line 369: }
Line 370: setSucceeded(true);
Line 371: }
Line 366: }
Line 367: if (reloadVmTemplateFromDB() != null) {
Line 368: endDefaultOperations();
Line 369: }
Line 370: setSucceeded(true);
I don't want to tie between EndSuccesffuly (which is end asynch actions method)
to endSuccessfullySynchronous(), i was asked to separate this into two
functions and not just to call EndSuccessfully.
Line 371: }
Line 372:
Line 373: private void endSuccessfullySynchronous() {
Line 374: if (reloadVmTemplateFromDB() != null) {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmTemplateCommand.java
Line 87: retVal = false;
Line 88: }
Line 89: }
Line 90:
Line 91: if (!getVmTemplate().getDiskList().isEmpty()) {
unnecessary IF, removed. thanks.
Line 92: retVal = retVal && super.canDoAction();
Line 93: }
Line 94:
Line 95: // check if template (with no override option)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java
Line 253: return null;
Line 254: }
Line 255: });
Line 256:
Line 257: boolean isVmTemplateContainImages =
!getParameters().getImages().isEmpty();
Done
Line 258: if (isVmTemplateContainImages) {
Line 259: MoveOrCopyAllImageGroups(getVmTemplateId(),
getParameters().getImages());
Line 260: }
Line 261:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateCommand.java
Line 180: // Set VM to lock status immediately, for reducing race
condition.
Line 181:
VmTemplateHandler.lockVmTemplateInTransaction(getVmTemplateId(),
getCompensationContext());
Line 182: // if for some reason template doesn't have images,
remove it now and not in end action
Line 183: final boolean hasImages = imageTemplates.size() > 0;
Line 184: if
(RemoveTemplateInSpm(getVmTemplate().getstorage_pool_id().getValue(),
getVmTemplateId())) {
yep, thats for the ovf operation on vdsm.
Line 185: if (hasImages) {
Line 186: TransactionSupport.executeInNewTransaction(new
TransactionMethod<Void>() {
Line 187:
Line 188: @Override
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewTemplateVmModelBehavior.java
Line 120
Line 121
Line 122
Line 123
Line 124
Done
--
To view, visit http://gerrit.ovirt.org/6593
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie825ea39fee6d6d5c86f9c82f3008fa314392ed2
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
