Liron Aravot has posted comments on this change.

Change subject: core: ensure unique unit for VirtIO_SCSI and SPAR_VSCSI disks
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/39471/2/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HotPlugDiskVDSCommand.java:

Line 175:                 VmDevice vmDevice = getParameters().getVmDevice();
Line 176:                 vmDevice.setAddress(address);
Line 177:                 
DbFacade.getInstance().getVmDeviceDao().update(vmDevice);
Line 178:                 return null;
Line 179:             }
> I see. HotPlugDiskToVmCommand is non-transactive, can't we set this command
VDS commands don't start transactions.
I'd say that this command shouldn't rely on the caller implementation and to be 
implemented "correctly"..but on the other hand, VDS commands shouldn't be 
called from transaction so i'd remove this new transaction.
Line 180:         });
Line 181:     }
Line 182: 
Line 183:     protected void lockVmWithWait() {


-- 
To view, visit https://gerrit.ovirt.org/39471
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5e621618e8891fca52b48bff437cc4c2a1f695db
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Vitor de Lima <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to