[
https://issues.apache.org/jira/browse/CLOUDSTACK-5823?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Mike Tutkowski reassigned CLOUDSTACK-5823:
------------------------------------------
Assignee: Mike Tutkowski
> Taking a VMware snapshot doesn't work
> -------------------------------------
>
> Key: CLOUDSTACK-5823
> URL: https://issues.apache.org/jira/browse/CLOUDSTACK-5823
> Project: CloudStack
> Issue Type: Bug
> Security Level: Public(Anyone can view this level - this is the
> default.)
> Components: VMware
> Affects Versions: 4.3.0
> Environment: ESXi 5.1
> Reporter: Mike Tutkowski
> Assignee: Mike Tutkowski
> Fix For: 4.3.0
>
>
> Per a discussion on the mailing list:
> VMware snapshot question
> Mike Tutkowski <[email protected]>
> 4:10 PM (18 hours ago)
> to dev
> 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 <[email protected]>
> 4:35 PM (17 hours ago)
> to dev
> 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?
> Mike Tutkowski <[email protected]>
> 6:33 PM (15 hours ago)
> to Kelven, dev
> 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!
> Kelven Yang 10:02 PM (12 hours ago)
> Yes, your guess is correct, the intent to update with a new path is to
> reflec...
> Mike Tutkowski <[email protected]>
> 10:13 PM (12 hours ago)
> to dev, Kelven
> 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!
> --
> Mike Tutkowski
> Senior CloudStack Developer, SolidFire Inc.
> e: [email protected]
> o: 303.746.7302
> Advancing the way the world uses the cloud™
> Mike Tutkowski <[email protected]>
> 10:43 PM (11 hours ago)
> to dev, Kelven
> 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!!
> Mike Tutkowski <[email protected]>
> 10:47 PM (11 hours ago)
> to dev, Kelven
> 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?
> Mike Tutkowski <[email protected]>
> 12:19 AM (10 hours ago)
> to dev, Kelven
> 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!
> Mike Tutkowski <[email protected]>
> 12:20 AM (10 hours ago)
> to dev, Kelven
> 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);
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
