cloudstack-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF subversion and git services (JIRA)" <>
Subject [jira] [Commented] (CLOUDSTACK-5823) Taking a VMware snapshot doesn't work
Date Fri, 10 Jan 2014 06:20:55 GMT


ASF subversion and git services commented on CLOUDSTACK-5823:

Commit a354d969ce316d065a4abf1405fce708955bdf0a in branch refs/heads/master from [~mike-tutkowski]
[;h=a354d96 ]

Merge from 4.3: CLOUDSTACK-5823: Taking a VMware snapshot doesn't work

> Taking a VMware snapshot doesn't work
> -------------------------------------
>                 Key: CLOUDSTACK-5823
>                 URL:
>             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 <>
> 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 <>
> 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 <>
> 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 <>
> 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
> 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:
> o: 303.746.7302
> Advancing the way the world uses the cloud™
> Mike Tutkowski <>
> 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:
> 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
> 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 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 <>
> 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 <>
> 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 <>
> 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],

This message was sent by Atlassian JIRA

View raw message