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