Mike Tutkowski created CLOUDSTACK-5823:
------------------------------------------
Summary: 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
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)
