Ok I closed out the review as "submitted"
> -----Original Message-----
> From: Mike Tutkowski [mailto:nore...@reviews.apache.org] On Behalf Of
> Mike Tutkowski
> Sent: Thursday, November 07, 2013 1:03 PM
> To: Edison Su; Mike Tutkowski
> Cc: Marcus Sorensen; cloudstack
> Subject: Re: Review Request 14381: KVM: add connect/disconnect
> capabilities to StorageAdaptors so that external storage services can
> attach/detach devices on-demand
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14381/#review28443
> -----------------------------------------------------------
>
> Ship it!
>
>
> Ship It!
>
> - Mike Tutkowski
>
>
> On Sept. 30, 2013, 5:14 p.m., Marcus Sorensen wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/14381/
> > -----------------------------------------------------------
> >
> > (Updated Sept. 30, 2013, 5:14 p.m.)
> >
> >
> > Review request for cloudstack, edison su and Mike Tutkowski.
> >
> >
> > Repository: cloudstack-git
> >
> >
> > Description
> > -------
> >
> > With custom storage plugins comes the need to prep the KVM host prior
> to utilizing the disks. e.g. an iscsi initiator needs to log into the
> target and scan for the lun before it can be used on the host. This
> patch is an example I developed against 4.2, minor changes may be
> necessary to apply to master, but I want to share with others who are
> working on storage so they can ensure it works for them. Please tweak as
> you see fit.
> >
> > MigrateCommand: pass vmTO object so we can see which disks/storage
> pool types belong to the vm when migrating a VM. This facilitates being
> able to call disconnectPhysicalDisksViaVmSpec
> >
> > VirtualMachineManagerImpl: pass VirtualMachineTO when migrating so
> that we can see which disks belong to the VM and what storage
> pools/adaptors should be used
> >
> > LibvirtComputingResource: add calls KVMStoragePoolManager's
> connectPhysicalDiskViaVmSpec and disconnectPhysicalDiskViaVmSpec calls
> where appropriate (when starting a vm, migrating a vm). Ensure that we
> create 'raw' format XML disk definitions when the storage format is RAW.
> Move cleanupDisk logic to storage adaptors so that each adaptor type can
> clean up its disks in is own way.
> >
> > KVMStoragePoolManager: add connectPhysicalDisk,
> disconnectPhysicalDisk, connectPhysicalDiskViaVmSpec,
> disconnectPhysicalDiskViaVmSpec, disconnectPhysicalDiskByPath. These all
> call the specific StorageAdaptor's connectPhysicalDisk,
> disconnectPhysicalDisk, or disconnectPhysicalDiskByPath calls.
> >
> > KVMStorageProcessor: Call connectPhysicalDisk/disconnectPhysicalDisk
> on the storage adaptor. Whether or not this is implemented is up to the
> storage adaptor.
> >
> > LibvirtStorageAdaptor: implement dummy
> connectPhysicalDisk/disconnectPhysicalDisk, move cleanupDisk logic from
> LibvirtComputingResource to disconnectPhysicalDiskByPath
> >
> > StorageAdaptor: define
> connectPhysicalDisk/disconnectPhysicalDisk/disconnectPhysicalDiskByPath
> in the interface
> >
> >
> > Diffs
> > -----
> >
> > core/src/com/cloud/agent/api/MigrateCommand.java 5042b8c
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtCom
> putingResource.java 3ee811f
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageP
> oolManager.java e09c9ba
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageP
> rocessor.java c69f9b0
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/LibvirtStor
> ageAdaptor.java 123a9f1
> >
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdap
> tor.java 4956d8d
> > server/src/com/cloud/vm/VirtualMachineManagerImpl.java d46bbb0
> >
> > Diff: https://reviews.apache.org/r/14381/diff/
> >
> >
> > Testing
> > -------
> >
> > Basic testing with my storage adaptor
> >
> >
> > Thanks,
> >
> > Marcus Sorensen
> >
> >
