Chris Morrissey has posted comments on this change.

Change subject: restapi: Added "scan" to disks resource to import clones
......................................................................


Patch Set 1: (6 inline comments)

Responded to comments. Working on patch set 2 to be delivered shortly.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportImageCommand.java
Line 24:     @Override
Line 25:     protected void executeCommand() {
Line 26:         final DiskImage newDiskImage = getParameters().getDiskImage();
Line 27:         if (newDiskImage != null) {
Line 28:             TransactionSupport.executeInNewTransaction(new 
TransactionMethod<Void>() {
I had based the logic here on code from the AddDiskCommand which had used a new 
transaction when adding a disk. However, I guess since there's only a single 
change to the DB, there's no need for a transaction. I can remove it.
Line 29:                 @Override
Line 30:                 public Void runInTransaction() {
Line 31:                     addDiskImageToDb(newDiskImage, 
getCompensationContext());
Line 32:                     
getReturnValue().setActionReturnValue(newDiskImage.getId());


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ScanDomainQuery.java
Line 21:  * @author Ricky Hopper
Line 22:  *
Line 23:  * @param <P>
Line 24:  */
Line 25: public class ScanDomainQuery<P extends 
StorageDomainQueryParametersBase> extends QueriesCommandBase<P> {
I'll include a test in the next patch.
Line 26: 
Line 27:     private static final Log logger = 
LogFactory.getLog(ScanDomainQuery.class);
Line 28: 
Line 29:     public ScanDomainQuery(P parameters) {


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/ImportImageParameters.java
Line 6: public class ImportImageParameters extends ImagesActionsParametersBase {
Line 7: 
Line 8:     /**
Line 9:      *
Line 10:      */
Will remove in next patch set.
Line 11:     private static final long serialVersionUID = -2354567059991414220L;
Line 12: 
Line 13:     private DiskImage diskImage;
Line 14: 


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java
Line 106:     RemoveSnapshot(210, ActionGroup.MANIPULATE_VM_SNAPSHOTS, 
QuotaDependency.STORAGE),
Line 107:     RemoveImage(211, QuotaDependency.STORAGE),
Line 108:     RemoveAllVmImages(212, QuotaDependency.STORAGE),
Line 109:     AddImageFromScratch(213, QuotaDependency.STORAGE),
Line 110:     ImportImage(214, ActionGroup.CONFIGURE_VM_STORAGE, 
QuotaDependency.STORAGE),
It actually does not consume more disk space as it is simply importing the 
information about an existing image into the oVirt DB. I will change to 
QuotaDependency.NONE.
Line 111:     RemoveTemplateSnapshot(215, QuotaDependency.STORAGE),
Line 112:     RemoveAllVmTemplateImageTemplates(216, QuotaDependency.STORAGE),
Line 113:     TryBackToAllSnapshotsOfVm(223, 
ActionGroup.MANIPULATE_VM_SNAPSHOTS, QuotaDependency.NONE),
Line 114:     RestoreAllSnapshots(224, ActionGroup.MANIPULATE_VM_SNAPSHOTS, 
QuotaDependency.STORAGE),


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDisksResource.java
Line 140:             VdcQueryReturnValue newImagesReturn = 
runQuery(VdcQueryType.ScanDomain, scanQueryParameters);
Line 141:             if (newImagesReturn.getSucceeded()) {
Line 142: 
Line 143:                 // Get a reference to the VDS broker so we can run 
commands against the backend.
Line 144:                 VDSBrokerFrontend vdsBroker = 
getBackend().getResourceManager();
Good point. I had looked at this mainly from the point of need for our product, 
but it can be helpful in the UI as well. I'll split out this logic into a new 
ScanDomainCommand on the server side and just call that from here.
Line 145:                 @SuppressWarnings("unchecked")
Line 146:                 List<Guid> newImages = (List<Guid>) 
newImagesReturn.getReturnValue();
Line 147:                 for (Guid newImage : newImages) {
Line 148: 


....................................................
Commit Message
Line 3: AuthorDate: 2012-11-30 14:05:14 -0500
Line 4: Commit:     Chris Morrissey <cmorr...@netapp.com>
Line 5: CommitDate: 2012-11-30 15:24:46 -0500
Line 6: 
Line 7: restapi: Added "scan" to disks resource to import clones
Would be pretty tough to split these out. I'll change to core as you are 
correct in that most of the changes in the patch are core related.
Line 8: 
Line 9: I am working on a plugin for NetApp storage systems that will
Line 10: use its cloning capabilities to clone disks outside of oVirt.
Line 11: This new API will allow us to have those disks imported into


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If6236e563d38ee1a793bd94ff60587f19b3010d1
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Chris Morrissey <cmorr...@netapp.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Chris Morrissey <cmorr...@netapp.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: ofri masad <oma...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to