Allon Mureinik has posted comments on this change.
Change subject: core: Migrate snapshots data & start using table
......................................................................
Patch Set 8: (4 inline comments)
Have some comments on the SQL part.
I glanced over the java in a level-0 way - looks OK, but someone else will have
to review the business logic, I don't think I understand in enough to review it.
....................................................
File
backend/manager/dbscripts/upgrade/03_01_0600_migrate_images_to_snapshots.sql
Line 7: BEGIN
I'd use "SELECT 1 FROM snapshots"
Theoretically it should has less I/O, although I doubt there's an actual
performance difference.
Line 11:
Why loop with cursor instead of INSERT-SELECT?
Line 17: SELECT *
same - SELECT 1
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
Line 257: private boolean statelessSnapshotExistsForVm() {
This pattern repeats quite a lot - I'd move it to SnapshotsManager or some
util/base class.
--
To view, visit http://gerrit.ovirt.org/2314
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0fa10e27736b05cf95fac22d088156b83de0747c
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Anonymous Coward #1000140
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Muli Salem <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
