If it's true that only the last iteration "counts," couldn't we just grab
the last item in this list?:
List<Pair<String, ManagedObjectReference>> vmdkFiles = vmMo.
getDiskDatastorePathChain(vdisks[i], false);
On Tue, Jan 7, 2014 at 12:19 AM, Mike Tutkowski <
mike.tutkow...@solidfire.com> wrote:
> Hi Kelven,
>
> I've been playing around with some code to fix this VMware-snapshot issue.
> Probably won't have it done until tomorrow, but I wanted to ask you about
> this code:
>
> for (int i = 0; i < vdisks.length; i++) {
>
> List<Pair<String, ManagedObjectReference>> vmdkFiles =
> vmMo.getDiskDatastorePathChain(vdisks[i], false);
>
>
> for (Pair<String, ManagedObjectReference> fileItem :
> vmdkFiles) {
>
>
> Can you tell me why we iterate through all of the VMDK files of a virtual
> disk? It seems like only the last one "counts." Is that correct? Am I to
> assume the last one we iterate over is the most recent snapshot (the
> snapshot we just took)?
>
> Thanks!
>
>
> On Mon, Jan 6, 2014 at 10:47 PM, Mike Tutkowski <
> mike.tutkow...@solidfire.com> wrote:
>
>> Ignore my question about coming up with a baseName.
>>
>> I see now that volumeTO is not available in the first for loop.
>>
>> I do think the key and value we have in the map, though, is incorrect.
>>
>> What do you think?
>>
>>
>> On Mon, Jan 6, 2014 at 10:43 PM, Mike Tutkowski <
>> mike.tutkow...@solidfire.com> wrote:
>>
>>> 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>
>>> *™*
>>>
>>
>>
>>
>> --
>> *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>
*™*
