Allon Mureinik has posted comments on this change.
Change subject: core: GetAllVmSnapshotsFromConfigurationByVmIdQuery
......................................................................
Patch Set 2: Code-Review-1
(3 comments)
1. See comments about the implementation inline.
2. Where is this query used? The patch, as it currently is, just introduced
dead code that is not used anywhere.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetAllVmSnapshotsFromConfigurationByVmIdQuery.java
Line 5: import org.ovirt.engine.core.common.queries.IdQueryParameters;
Line 6: import org.ovirt.engine.core.common.queries.VdcQueryReturnValue;
Line 7: import org.ovirt.engine.core.common.queries.VdcQueryType;
Line 8:
Line 9: import java.util.List;
by our coding conventions, java.* imports should be on the top.
Line 10:
Line 11: /**
Line 12: * Return a list of all the snapshots for the given VM id.<br>
Line 13: * The snapshots are sorted by their creation date.<br>who
Line 9: import java.util.List;
Line 10:
Line 11: /**
Line 12: * Return a list of all the snapshots for the given VM id.<br>
Line 13: * The snapshots are sorted by their creation date.<br>who
who?
Line 14: */
Line 15: public class GetAllVmSnapshotsFromConfigurationByVmIdQuery<P extends
IdQueryParameters> extends QueriesCommandBase<P> {
Line 16: public GetAllVmSnapshotsFromConfigurationByVmIdQuery(P parameters)
{
Line 17: super(parameters);
Line 23: .getAll(getParameters().getId(), getUserID(),
getParameters().isFiltered());
Line 24: for (Snapshot snapshot : snapshotsList) {
Line 25: VdcQueryReturnValue queryReturnValue =
Line 26:
Backend.getInstance().runInternalQuery(VdcQueryType.GetVmConfigurationBySnapshot,
Line 27: new IdQueryParameters(snapshot.getId()));
Yikes.
I really don't like going through N snapshots and executing a new query for
each one.
Basically, you should be calling SnapshotDAO.getAll() and iterate through all
of them. This probably needs to share some code with
GetVmConfigurationBySnapshotQuery, but not like this.
Line 28: if (queryReturnValue.getSucceeded()) {
Line 29: VM vm = queryReturnValue.getReturnValue();
Line 30: snapshot.setDiskImages(vm.getImages());
Line 31: }
--
To view, visit http://gerrit.ovirt.org/22774
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I23aa4de4d233fade33d2a5ea174a9a1802b49370
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Tal Nisan <[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
