Actually, the more I look at this code, the more I think perhaps VMware
snapshots are broken because the newPath field should probably not be
assigned null after creating a new VMware snapshot (I'm thinking the intend
is to replace the other path with a new path that refers to the delta file
that was just created).
Does anyone know who worked on VMware snapshots? I've love to ask these
questions to him soon as we are approaching the end of 4.3.
Thanks!
On Mon, Jan 6, 2014 at 4:35 PM, Mike Tutkowski <mike.tutkow...@solidfire.com
> wrote:
> In short, I believe we can remove mapNewDisk and just assign null to
> newPath. This will keep the existing path for the volume in question (in
> the volumes table) in the same state as it was before we created a VMware
> snapshot, which I believe is the intent anyways.
>
> Thoughts on that?
>
>
> On Mon, Jan 6, 2014 at 4:10 PM, Mike Tutkowski <
> mike.tutkow...@solidfire.com> wrote:
>
>> Hi,
>>
>> I was wondering about the following code in VmwareStorageManagerImpl. It
>> is in the CreateVMSnapshotAnswer execute(VmwareHostService hostService,
>> CreateVMSnapshotCommand cmd) method.
>>
>> The part I wonder about is in populating the mapNewDisk map. For disks
>> like the following:
>>
>> i-2-9-VM/fksjfaklsjdgflajs.vmdk, the key for the map ends up being i-2.
>>
>> When we call this:
>>
>> String baseName = extractSnapshotBaseFileName(volumeTO.getPath());
>>
>> It uses a path such as the following:
>>
>> fksjfaklsjdgflajs
>>
>> There is no i-2-9-VM/ preceding the name, so the key we search on ends up
>> being the following:
>>
>> fksjfaklsjdgflajs
>>
>> This leads to a newPath being equal to null.
>>
>> As it turns out, I believe null is actually correct, but - if that's the
>> case - why do we have all this logic if - in the end - we are just going to
>> assign null to newPath in every case when creating a VM snapshot for
>> VMware? As it turns out, null is later interpreted to mean, "don't replace
>> the path field of this volume in the volumes table," which is, I think,
>> what we want.
>>
>> Thanks!
>>
>> VirtualDisk[] vdisks = vmMo.getAllDiskDevice();
>>
>> for (int i = 0; i < vdisks.length; i ++){
>>
>> List<Pair<String, ManagedObjectReference>> vmdkFiles
>> = vmMo.getDiskDatastorePathChain(vdisks[i], false);
>>
>> for(Pair<String, ManagedObjectReference> fileItem :
>> vmdkFiles) {
>>
>> String vmdkName = fileItem.first().split(" ")[1];
>>
>> if (vmdkName.endsWith(".vmdk")){
>>
>> vmdkName = vmdkName.substring(0,
>> vmdkName.length() - (".vmdk").length());
>>
>> }
>>
>> String baseName =
>> extractSnapshotBaseFileName(vmdkName);
>>
>> mapNewDisk.put(baseName, vmdkName);
>>
>> }
>>
>> }
>>
>> for (VolumeObjectTO volumeTO : volumeTOs) {
>>
>> String baseName =
>> extractSnapshotBaseFileName(volumeTO.getPath());
>>
>> String newPath = mapNewDisk.get(baseName);
>>
>> // get volume's chain size for this VM snapshot,
>> exclude current volume vdisk
>>
>> DataStoreTO store = volumeTO.getDataStore();
>>
>> long size =
>> getVMSnapshotChainSize(context,hyperHost,baseName + "*.vmdk",
>>
>> store.getUuid(), newPath);
>>
>>
>> if(volumeTO.getVolumeType()== Volume.Type.ROOT){
>>
>> // add memory snapshot size
>>
>> size = size +
>> getVMSnapshotChainSize(context,hyperHost,cmd.getVmName()+"*.vmsn"
>> ,store.getUuid(),null);
>>
>> }
>>
>>
>> volumeTO.setSize(size);
>>
>> volumeTO.setPath(newPath);
>>
>> }
>>
>> --
>> *Mike Tutkowski*
>> *Senior CloudStack Developer, SolidFire Inc.*
>> e: mike.tutkow...@solidfire.com
>> o: 303.746.7302
>> Advancing the way the world uses the
>> cloud<http://solidfire.com/solution/overview/?video=play>
>> *™*
>>
>
>
>
> --
> *Mike Tutkowski*
> *Senior CloudStack Developer, SolidFire Inc.*
> e: mike.tutkow...@solidfire.com
> o: 303.746.7302
> Advancing the way the world uses the
> cloud<http://solidfire.com/solution/overview/?video=play>
> *™*
>
--
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkow...@solidfire.com
o: 303.746.7302
Advancing the way the world uses the
cloud<http://solidfire.com/solution/overview/?video=play>
*™*
