Maor Lipchuk has posted comments on this change. Change subject: core: replace vm id with number of vms in views. ......................................................................
Patch Set 1: (9 inline comments) .................................................... File backend/manager/dbscripts/create_views.sql Line 69: LEFT OUTER JOIN base_disks ON images.image_group_id = base_disks.disk_id I'm not sure I understand you comment here. device id is not null, that is right, the count of device_id and count(*) should bring the same result. so if I will have a floating disk, then I will not be fetched it in this sub query (since the disk will not have devices) so the big query will have null value in the number_of_vms disregarding whether the sub select were counting device_id or * Line 232: LEFT JOIN I thought about it, but I prefer not to, because I think adding a new view for these specific field (number_of_vms) will be an over head, we already have many views I think adding a new view will not make it more organize, besides, these fields are only appearing in two views all_disks, and images_storage_domain_view which are actually based on each other, so logically the vm_static_device not really has that common use. .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DiskImage.java Line 14: Very good noticing it, no one is using that actually, this field has been used in the WPF era, this field should be removed from all the BE. but I think its better to handle it in a separate clean up patch. Line 81: VmEntityType vmEntityType, Same reason I wrote in the original BE .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Disk.java Line 21: private VmEntityType vmEntityType; Just more convenient, most of our BE using it that way, I don;t think it has a big influence in our performance and it handle usually handle more better with nulls. I can change it, if you think it could be better Line 118: same reason Line 122: same reason .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/LunDisk.java Line 24: VmEntityType vmEntityType, See my comment on Disk BE .................................................... File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java Line 351: funny, it should not be here an experiment went wrong. Will fix, thanks -- 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: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
