Eli Mesika has uploaded a new change for review. Change subject: userportal : Fix DiskForVmGuid high CPU ......................................................................
userportal : Fix DiskForVmGuid high CPU In this patch we remove the need to load all disk data and just load the required information for the user portal. This patch was first committed and merged as http://gerrit.ovirt.org/#/c/16657/ and then reverted due to DAO tests failure. this commit has exactly the same changes as the original patch, the only change is in the TEST class, the problem was that the test checked the description field for NULL while it was set by the fixtures.xml to "New Description" Change-Id: I1d7369cfbfe9b7c6901bd1bc2e9c16bdc31a9af9 Bug URL: https://bugzilla.redhat.com/show_bug.cgi?id=971237 Signed-off-by: Eli Mesika <emes...@redhat.com> --- A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllDisksPartialDataByVmIdQuery.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java M backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java M backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java M frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java M packaging/dbscripts/all_disks_sp.sql 7 files changed, 104 insertions(+), 11 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/68/18268/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllDisksPartialDataByVmIdQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllDisksPartialDataByVmIdQuery.java new file mode 100644 index 0000000..dd2ccf9 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllDisksPartialDataByVmIdQuery.java @@ -0,0 +1,20 @@ +package org.ovirt.engine.core.bll; + +import java.util.List; + +import org.ovirt.engine.core.common.businessentities.Disk; +import org.ovirt.engine.core.common.queries.IdQueryParameters; + +public class GetAllDisksPartialDataByVmIdQuery<P extends IdQueryParameters> extends QueriesCommandBase<P> { + public GetAllDisksPartialDataByVmIdQuery(P parameters) { + super(parameters); + } + + @Override + protected void executeQueryCommand() { + List<Disk> allDisks = + getDbFacade().getDiskDao().getAllForVmPartialData + (getParameters().getId(), false, getUserID(), getParameters().isFiltered()); + getQueryReturnValue().setReturnValue(allDisks); + } +} diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java index c3c2040..d62c496 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java @@ -284,6 +284,7 @@ GetClusterPolicyById, GetAllPolicyUnits, GetAttachedClustersByClusterPolicyId, + GetAllDisksPartialDataByVmId(VdcQueryAuthType.User), // Default type instead of having to null check Unknown(VdcQueryAuthType.User); diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java index a250f97..23bce56 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDao.java @@ -102,4 +102,19 @@ * @return the Disk */ Disk get(Guid id, Guid userID, boolean isFiltered); + + /** + * Retrieves all disks for the specified virtual machine id, with optional filtering. Only data for the UI basic + * screen is returned + * @param id + * the VM id + * @param onlyPluggedDisks + * whether to returned only the disks plugged to the VM or not + * @param userID + * the ID of the user requesting the information + * @param isFiltered + * Whether the results should be filtered according to the user's permissions + * @return the list of disks + */ + List<Disk> getAllForVmPartialData(Guid vmId, boolean onlyPluggedDisks, Guid userID, boolean isFiltered); } diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java index 01d150b..5360dd7 100644 --- a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java +++ b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DiskDaoDbFacadeImpl.java @@ -6,6 +6,7 @@ import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.Disk.DiskStorageType; +import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.LunDisk; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.DiskImageDAODbFacadeImpl.DiskImageRowMapper; @@ -48,6 +49,21 @@ .addValue("user_id", userID) .addValue("is_filtered", isFiltered); return getCallsHandler().executeReadList("GetDisksVmGuid", DiskRowMapper.instance, parameterSource); + } + + @Override + public List<Disk> getAllForVmPartialData(Guid vmId, + boolean onlyPluggedDisks, + Guid userID, + boolean isFiltered) { + MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() + .addValue("vm_guid", vmId) + .addValue("only_plugged", onlyPluggedDisks) + .addValue("user_id", userID) + .addValue("is_filtered", isFiltered); + return getCallsHandler().executeReadList("GetDisksVmGuidBasicView", + DiskBasicViewRowMapper.instance, + parameterSource); } @Override @@ -113,6 +129,24 @@ } } + private static class DiskBasicViewRowMapper implements RowMapper<Disk> { + + public static DiskBasicViewRowMapper instance = new DiskBasicViewRowMapper(); + + private DiskBasicViewRowMapper() { + } + + @Override + public Disk mapRow(ResultSet rs, int rowNum) throws SQLException { + DiskImage disk = new DiskImage(); + disk.setDiskAlias(rs.getString("disk_alias")); + disk.setSize(rs.getLong("size")); + disk.setId(getGuid(rs, "disk_id")); + + return disk; + } + } + private static class LunDiskRowMapper extends AbstractDiskRowMapper<LunDisk> { public static LunDiskRowMapper instance = new LunDiskRowMapper(); diff --git a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java index 1fed675..92c5125 100644 --- a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java +++ b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DiskDaoTest.java @@ -176,6 +176,14 @@ assertEquals("Wrong boot disk for VM", bootDisk.getId(), FixturesTool.BOOTABLE_DISK_ID); } + @Test + public void testGetVmPartialData() { + List<Disk> disks = dao.getAllForVm(FixturesTool.VM_RHEL5_POOL_57, PRIVILEGED_USER_ID, true); + assertFullGetAllForVMResult(disks); + assertEquals("New Description", disks.get(0).getDiskDescription()); + assertNotNull(disks.get(0).getDiskAlias()); + } + /** * Asserts the result of {@link DiskImageDAO#getAllForVm(Guid)} contains the correct disks. * @param disks diff --git a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java index b01f449..b4a525e 100644 --- a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java +++ b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/VmBasicDiskListModel.java @@ -1,6 +1,5 @@ package org.ovirt.engine.ui.uicommonweb.models.userportal; -import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -51,18 +50,16 @@ { List<DiskImage> disks = (List<DiskImage>) ((VdcQueryReturnValue) ReturnValue).getReturnValue(); - ArrayList<DiskImage> diskList = new ArrayList<DiskImage>(); - diskList.addAll(disks); - Collections.sort(diskList, new Linq.DiskByAliasComparer()); + Collections.sort(disks, new Linq.DiskByAliasComparer()); SearchableListModel searchableListModel = (SearchableListModel) model; - searchableListModel.setItems(diskList); + searchableListModel.setItems(disks); } }; IdQueryParameters queryParameters = new IdQueryParameters(vm.getId()); queryParameters.setRefresh(getIsQueryFirstTime()); - Frontend.RunQuery(VdcQueryType.GetAllDisksByVmId, queryParameters, + Frontend.RunQuery(VdcQueryType.GetAllDisksPartialDataByVmId, queryParameters, _asyncQuery); } else if (getEntity() instanceof VmPool) @@ -89,19 +86,18 @@ { List<DiskImage> disks = (List<DiskImage>) ((VdcQueryReturnValue) ReturnValue).getReturnValue(); - ArrayList<DiskImage> diskList = new ArrayList<DiskImage>(); - diskList.addAll(disks); - Collections.sort(diskList, new Linq.DiskByAliasComparer()); + Collections.sort(disks, new Linq.DiskByAliasComparer()); SearchableListModel searchableListModel = (SearchableListModel) model1; - searchableListModel.setItems(diskList); + searchableListModel.setItems(disks); } }; IdQueryParameters queryParameters = new IdQueryParameters(vm.getId()); queryParameters.setRefresh(getIsQueryFirstTime()); - Frontend.RunQuery(VdcQueryType.GetAllDisksByVmId, queryParameters, + Frontend.RunQuery(VdcQueryType.GetAllDisksPartialDataByVmId, queryParameters, _asyncQuery1); } + } }; diff --git a/packaging/dbscripts/all_disks_sp.sql b/packaging/dbscripts/all_disks_sp.sql index fbbd1ae..af0fec3 100644 --- a/packaging/dbscripts/all_disks_sp.sql +++ b/packaging/dbscripts/all_disks_sp.sql @@ -52,6 +52,25 @@ END; $procedure$ LANGUAGE plpgsql; +DROP TYPE IF EXISTS disks_basic_rs CASCADE; +CREATE TYPE disks_basic_rs AS (disk_id UUID,disk_alias varchar(255),size BIGINT); + +Create or replace FUNCTION GetDisksVmGuidBasicView(v_vm_guid UUID, v_only_plugged BOOLEAN, v_user_id UUID, v_is_filtered BOOLEAN) +RETURNS SETOF disks_basic_rs + AS $procedure$ +BEGIN + RETURN QUERY SELECT disk_id,disk_alias, size + FROM images + LEFT OUTER JOIN base_disks ON images.image_group_id = base_disks.disk_id + LEFT JOIN vm_device ON vm_device.device_id = image_group_id AND (NOT v_only_plugged OR is_plugged) + WHERE vm_device.vm_id = v_vm_guid + AND images.active = true + AND (NOT v_is_filtered OR EXISTS (SELECT 1 + FROM user_disk_permissions_view + WHERE user_id = v_user_id AND entity_id = disk_id)); + +END; $procedure$ +LANGUAGE plpgsql; Create or replace FUNCTION GetVmBootDisk(v_vm_guid UUID) RETURNS SETOF all_disks AS $procedure$ BEGIN -- To view, visit http://gerrit.ovirt.org/18268 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1d7369cfbfe9b7c6901bd1bc2e9c16bdc31a9af9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <emes...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
