Hi Kelven,

To give you an idea visually what I am referring to, please check out this
screen capture:

http://i.imgur.com/ma3FE9o.png

The key is "i-2" (part of the folder for the VMDK file).

The value contains the folder the VMDK file is in. Since the path column
for VMware volumes in the DB doesn't contain the folder the VMDK file is
in, I think this may be incorrect, as well.

I also noticed that we later try to retrieve from the map using
volumeTO.getPath() (ignore the getPath() method that enclosing
volumeTO.getPath() in the screen shot as this is related to new code...in
the standard case, the value of volumeTO.getPath() is just returned from
the getPath() method).

In the first line of code visible in the screen capture, why do we go to
the trouble of doing this:

String baseName = extractSnapshotBaseFileName(vmdkName);

It seems like this would have worked:

String baseName = extractSnapshotBaseFileName(volumTO.getPath());

Or am I missing something there?

Thanks!!


On Mon, Jan 6, 2014 at 10:13 PM, Mike Tutkowski <
mike.tutkow...@solidfire.com> wrote:

> Thanks for the info, Kelven.
>
> I believe we have a serious bug then as null is being assigned to newPath
> when a VMware snapshot is being taken (this is in 4.3, by the way).
>
> I was trying to fix an issue with VMware snapshots and managed storage and
> happened upon this.
>
> If you have a moment, you might want to set a breakpoint and step through
> and see what I mean first hand.
>
> I'm looking into it, as well.
>
> Thanks!
>
>
> On Mon, Jan 6, 2014 at 10:02 PM, Kelven Yang <kelven.y...@citrix.com>wrote:
>
>>
>>
>> On 1/6/14, 5:33 PM, "Mike Tutkowski" <mike.tutkow...@solidfire.com>
>> wrote:
>>
>> >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).
>>
>> Yes, your guess is correct, the intent to update with a new path is to
>> reflect the name change after a VM snapshot is taken. When VM snapshot is
>> involved, one CloudStack volume may have more than one disks be related
>> with it. So the information in path field only points to the top of the
>> disk chain, and it is not guaranteed that this information is in sync with
>> vCenter since there may exist out-of-band changes from vCenter (by taking
>> VM snapshot in vCenter).
>>
>> To gracefully work with vCenter, for existing disks that are attached to a
>> VM, CloudStack only uses the information stored in path field in volume
>> table as a search basis to connect the record with the real disk chain in
>> the storage, as soon as it has located the connection, it actually uses
>> the most recent information from vCenter datastore to setup the disk
>> device. In addition, CloudStack also updates the full disk-chain
>> information to a field called ³chainInfo². The full chain-info can be used
>> for recovery/copy-out purpose
>>
>> Kelven
>>
>>
>> >
>> >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>
>> >* *
>>
>>
>
>
> --
> *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>
*™*

Reply via email to