Maor Lipchuk has posted comments on this change.

Change subject: core:Replace vm id with number of vms in views.
......................................................................


Patch Set 5: (9 inline comments)

Thanks for the comments, some will be changed some were answered.
Tested most of the flows, will verify it again after ack (might open a pre 
integration ticket)

....................................................
File backend/manager/dbscripts/create_views.sql
Line 24: FROM vm_static JOIN vm_device ON vm_static.vm_guid = vm_device.vm_id
agreed

....................................................
File backend/manager/dbscripts/storages_sp.sql
Line 522:          from images_storage_domain_view JOIN vm_device ON 
vm_device.device_id = images_storage_domain_view.disk_id
done.

Line 527:          from images_storage_domain_view JOIN vm_device ON 
vm_device.device_id = images_storage_domain_view.disk_id
done

Line 537:                                          JOIN vm_device ON 
vm_device.device_id = vm_images_view.disk_id
done.
Bummer the formatter can't be configured to do so.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddImageFromScratchCommand.java
Line 50
no, since we only add the disk to the DB, and vm count is calculated field.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java
Line 143
It is already being done when I call DiskImage.copyOf(srcDiskImage)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
Line 86:     private final List<Guid> imageGuidList = new ArrayList<Guid>();
automatic added, agreed, can be removed.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
Line 158:     private void setVmTemplateIdParameter() {
For locking purposes.

Since before, the locking mechanism, used the vm id(or template id) which was 
already set when the disk was fetched, to lock it. It is now not fetched any 
more.

So I need to fetch the template id so I can lock it.
This method fetching the template id which the disk is related to.

....................................................
File 
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java
Line 134:         assertEquals("There should be only three attachable disks", 
3, disks.size());
Will do.

--
To view, visit http://gerrit.ovirt.org/5216
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id306d8322245780ea200c10f9e96254cddf3bc76
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to