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>
*™*
