Mice thanks for taking care of this Animesh
> -----Original Message----- > From: Mice Xia [mailto:mice_...@tcloudcomputing.com] > Sent: Wednesday, April 10, 2013 1:57 AM > To: Animesh Chaturvedi; cloudstack-...@incubator.apache.org > Cc: Chandan Purushothama; Anthony Xu; Hari Kannan > Subject: RE: [MERGE] Support VM Snapshot > > Update in master: > > Add vm_snapshot enable/disable by hypervisor_capabilities, Add a new > column ` vm_snapshot_enabled` in table `hypervisor_capabilities`, and > change related VO/Dao Set vm_snapshot_enabled = 1 for > VMware/Xenserver Check hypervisor_capabilities when createVMSnapshot > > UI part was not implemented for hypervisor_capabilities because I noticed > 'maxHostPerCluster' is also added to hypervisor_capabilities and not sure if > there are more items will be there. > > -Mice > > -----Original Message----- > From: Animesh Chaturvedi [mailto:animesh.chaturv...@citrix.com] > Sent: Friday, March 22, 2013 5:09 AM > To: Mice Xia; cloudstack-...@incubator.apache.org > Cc: Chandan Purushothama; Anthony Xu; Hari Kannan > Subject: RE: [MERGE] Support VM Snapshot > > That's good, thanks for your response. > > > -----Original Message----- > > From: Mice Xia [mailto:mice_...@tcloudcomputing.com] > > Sent: Wednesday, March 20, 2013 11:29 PM > > To: cloudstack-...@incubator.apache.org; Animesh Chaturvedi > > Cc: Chandan Purushothama; Anthony Xu; Mice Xia > > Subject: RE: [MERGE] Support VM Snapshot > > > > I'd like to a hypervisor_capabilities based solution. > > The earliest time that I can start changing it is next week. I haven’t > > investigated the solution, let's roughly suppose this could be done > > before mid April. > > > > -Mice > > > > -----Original Message----- > > From: Animesh Chaturvedi [mailto:animesh.chaturv...@citrix.com] > > Sent: Thursday, March 21, 2013 12:58 AM > > To: cloudstack-...@incubator.apache.org > > Cc: Chandan Purushothama; Anthony Xu > > Subject: RE: [MERGE] Support VM Snapshot > > > > Mice any updates, when can you provide a global enable/disable flag > > for the feature? > > > > Animesh > > > > > -----Original Message----- > > > From: Animesh Chaturvedi [mailto:animesh.chaturv...@citrix.com] > > > Sent: Monday, March 18, 2013 10:44 PM > > > To: cloudstack-...@incubator.apache.org > > > Cc: Chandan Purushothama; Anthony Xu > > > Subject: RE: [MERGE] Support VM Snapshot > > > > > > It would be best to enable/disable this feature on a per hypervisor > > > basis but if that's complicated adding a top level flag to > > > enable/disable the functionality is reasonable. > > > > > > > -----Original Message----- > > > > From: Mice Xia [mailto:mice_...@tcloudcomputing.com] > > > > Sent: Monday, March 18, 2013 9:59 PM > > > > To: cloudstack-...@incubator.apache.org > > > > Cc: Chandan Purushothama; Anthony Xu; Mice Xia > > > > Subject: RE: [MERGE] Support VM Snapshot > > > > > > > > Animesh, > > > > > > > > Do you mean limit the capability by adding global configurations > > > > or adding in [global-setting--> hypervisor capabilities] ? > > > > > > > > Mice > > > > > > > > -----Original Message----- > > > > From: Animesh Chaturvedi [mailto:animesh.chaturv...@citrix.com] > > > > Sent: Tuesday, March 19, 2013 9:47 AM > > > > To: cloudstack-...@incubator.apache.org; Mice Xia; Chandan > > > > Purushothama; Anthony Xu > > > > Subject: RE: [MERGE] Support VM Snapshot > > > > > > > > Mice any comments to my email below. > > > > > > > > Animesh > > > > > > > > > -----Original Message----- > > > > > From: Animesh Chaturvedi [mailto:animesh.chaturv...@citrix.com] > > > > > Sent: Wednesday, March 13, 2013 6:03 PM > > > > > To: Mice Xia; cloudstack-...@incubator.apache.org; Chandan > > > > > Purushothama; Anthony Xu > > > > > Subject: RE: [MERGE] Support VM Snapshot > > > > > > > > > > Mice please see concerns from Anthony on Xenserver support., I > > > > > guess they can only be proved conclusively by testing with xenserver. > > > > > Chandan will test with xenserver. > > > > > > > > > > But given that there is some doubt on full support for xenserver > > > > > and kvm dependency on libvirt-java binding and I assume the > > > > > effort in making the capability configurable is trivial. In my > > > > > opinion we should make it configurable and turn on for xenserver > > > > > and kvm when support is > > > > clear. > > > > > > > > > > > > > > > Thanks > > > > > Animesh > > > > > > -----Original Message----- > > > > > > From: Mice Xia [mailto:weiran.x...@gmail.com] > > > > > > Sent: Monday, March 11, 2013 2:49 AM > > > > > > To: cloudstack-...@incubator.apache.org > > > > > > Cc: Animesh Chaturvedi > > > > > > Subject: Re: [MERGE] Support VM Snapshot > > > > > > > > > > > > Animesh, > > > > > > > > > > > > sorry for the late reply. > > > > > > > > > > > > 1. currently in master branch it supports both xenserver and > > > > > > vmware, for KVM it needs a new-versioned libvirt java binding, > > > > > > considering it's for 4.2, still three months away from the > > > > > > release date, we have big chance to ship the new libvirt > > > > > > binding. I dont see a strong need to make it configurable for > > > > > > hypervisors unless some functions do not work > > > > > at all.. > > > > > > > > > > > > 2. now delta volume snapshot will become a full one. But I > > > > > > guess volume snapshot will be improved soon for xenserver. > > > > > > please correct me if we dont have this plan. > > > > > > > > > > > > Regards > > > > > > -Mice > > > > > > > > > > > > 2013/3/8 Animesh Chaturvedi <animesh.chaturv...@citrix.com>: > > > > > > > Folks following up on this thread looks like support for > > > > > > > XenServer is still not > > > > > > settled. > > > > > > > > > > > > > > 1. Mice can we make the feature configurable for each > > > > > > > hypervisor to enable/disable the feature 2. Test the feature > > > > > > > with XenServer thoroughly to check if Volume Snapshot is > > > > > > > affected / degraded > > > > > > > > > > > > > > Thanks > > > > > > > Animesh > > > > > > > > > > > > > > > > > > > > >> -----Original Message----- > > > > > > >> From: Anthony Xu [mailto:xuefei...@citrix.com] > > > > > > >> Sent: Friday, February 01, 2013 5:44 PM > > > > > > >> To: 'Mice Xia'; cloudstack-...@incubator.apache.org; Alex > > > > > > >> Huang; Mice Xia > > > > > > >> Subject: RE: [MERGE] Support VM Snapshot > > > > > > >> > > > > > > >> I see, snapshot manager detected the change in primary > > > > > > >> storage, and create a full snapshot instead, which is > > > > > > >> supposed to be a delta > > > > snapshot. > > > > > > >> > > > > > > >> It doesn’t break volume snapshot function, but this > > > > > > >> degrades the volume snapshot performance. > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> This is just a simple test, it cannot prove there is no > > > > > > >> impact to volume snapshot. > > > > > > >> > > > > > > >> I’m not sure what will happen if execute these two commands > > > > > > >> at the same time, is there any mechanism to sync/serialize > > > > > > >> these two > > > > > operation? > > > > > > >> > > > > > > >> I’m not sure if revert VM has impact to volume snapshot. > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> For now, it is better to have a global configuration to > > > > > > >> only choose > > > one. > > > > > > >> > > > > > > >> later, we may support both of them in one setup. > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> Anthony > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> From: Mice Xia [mailto:mice_...@tcloudcomputing.com] > > > > > > >> Sent: Friday, February 01, 2013 5:30 PM > > > > > > >> To: cloudstack-...@incubator.apache.org; Alex Huang; Mice > > > > > > >> Xia; Anthony Xu > > > > > > >> Subject: 答复: [MERGE] Support VM Snapshot > > > > > > >> > > > > > > >> > > > > > > >> Anthony, > > > > > > >> > > > > > > >> Thanks for your comments. > > > > > > >> > > > > > > >> Tested on a datadisk with steps you provide on xenserver, > > > > > > >> all the files (test1, test2, test3) are present, the > > > > > > >> function is not > > > affected. > > > > > > >> But as i have replied, volume snapshot (s2) is not a delta > > > > > > >> snapshot, it is a full one. Users need to be aware of this > > > > > > >> if they want to use both snapshots simultaneously. > > > > > > >> > > > > > > >> Regards > > > > > > >> Mice > > > > > > >> > > > > > > >> > > > > > > >> -----Original Message----- > > > > > > >> From: Anthony Xu [mailto:xuefei...@citrix.com] > > > > > > >> Sent: 2013-2-2 (星期六) 4:05 > > > > > > >> To: Alex Huang; Mice Xia; > > > > > > >> cloudstack-...@incubator.apache.org > > > > > > >> Subject: RE: [MERGE] Support VM Snapshot > > > > > > >> > > > > > > >> CS uses XenServer delta snapshot, snapshot manager records > > > > > > >> a VHD chain in snapshot DB for each volume. VM snapshot > > > > > > >> creation/revert also operate on volume snapshot, if > > > > > > >> snapshot manager doesn't know the VM snapshot , volume > > > > > > >> snapshot might > > > be broken. > > > > > > >> > > > > > > >> > > > > > > >> You can try following test, > > > > > > >> > > > > > > >> 1. create a VM. > > > > > > >> 2. create empty file test1 inside this VM. > > > > > > >> 3. create a volume snapshot(s1) 4. create empty file test2 > > > > > > >> inside this VM 5. create a VM snapshot > > > > > > >> (vm1) > > > > > 6. > > > > > > >> create empty file test3 inside this VM 7. create a volume > > > > > > >> snapshot (s2) > > > > > 8. > > > > > > >> create a volume from snapshot (s2) 9. attach this volume to > > > > > > >> a VM > > 10. > > > > > > >> if one of test1, test2, test3 is missing in this volume, > > > > > > >> might mean volume snapshot is broken. > > > > > > >> > > > > > > >> > > > > > > >> It might be difficult to support both VM snapshot and > > > > > > >> volume snapshot in the same time for hypervisor which > > > > > > >> supports delta > > > > > snapshot. > > > > > > >> Maybe we need to provide a zone level configuration for it, > > > > > > >> only one is supported in a zone, volume snapshot or vm > snapshot. > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> Anthony > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > -----Original Message----- > > > > > > >> > From: Alex Huang > > > > > > >> > Sent: Friday, February 01, 2013 10:54 AM > > > > > > >> > To: Mice Xia; cloudstack-...@incubator.apache.org > > > > > > >> > Cc: Anthony Xu > > > > > > >> > Subject: RE: [MERGE] Support VM Snapshot > > > > > > >> > > > > > > > >> > Mice, > > > > > > >> > > > > > > > >> > Thanks! > > > > > > >> > > > > > > > >> > Anthony, > > > > > > >> > > > > > > > >> > Can you comment on whether VM Snapshot breaks volume > > > > > snapshot? > > > > > > >> > > > > > > > >> > --Alex > > > > > > >> > > > > > > > >> > > -----Original Message----- > > > > > > >> > > From: Mice Xia [mailto:weiran.x...@gmail.com] > > > > > > >> > > Sent: Friday, February 01, 2013 8:53 AM > > > > > > >> > > To: cloudstack-...@incubator.apache.org; Alex Huang > > > > > > >> > > Subject: Re: [MERGE] Support VM Snapshot > > > > > > >> > > > > > > > > >> > > as Alex suggested > > > > > > >> > > updated vm-snapshot branch, commit ebca6890fd > > > > > > >> > > > > > > > > >> > > 1. remove snapshotting/revertting state from VM state > > > > > > >> > > machine > > > > > > >> > > 2 prevent VM state change if there are active vm > > > > > > >> > > snapshot tasks > > > > > > >> > > 3 change VMSnapshotService interface, except for > > > > > > >> > > ListVMSnapshotCmd, need some time to replace it in > > > > > > >> > > QueryService, maybe after merging to master > > > > > > >> > > 4 remove unused methods and fix some typos > > > > > > >> > > > > > > > > >> > > Regards > > > > > > >> > > Mice > > > > > > >> > > > > > > > > >> > > 2013/2/1 Mice Xia <mice_...@tcloudcomputing.com>: > > > > > > >> > > > Hi, Alex, > > > > > > >> > > > > > > > > > >> > > > Thanks for your feedbacks, please see my comments inline. > > > > > > >> > > > > > > > > > >> > > > - VM states is designed for VM lifecycle. Snapshot > > > > > > >> > > > is not part of > > > > > > >> > VM life > > > > > > >> > > cycle so therefore the state should not be there. I > > > > > > >> > > think it make > > > > > > >> > sense to add > > > > > > >> > > attributes to VM that says "Do Not Change State" and > > > > > > >> > > who changed the > > > > > > >> > VM > > > > > > >> > > to that state. Then virtualmachinemanager must obey > > > > > > >> > > that until the > > > > > > >> > external > > > > > > >> > > caller changes the attribute to now you can change state. > > > > > > >> > > The would > > > > > > >> > make > > > > > > >> > > more sense and decouples snapshotting from vm lifecycle > > > > > > management. > > > > > > >> > > Snapshotting then has it's own state (which I see it > > > > > > >> > > does > > > already). > > > > > > >> > If we want > > > > > > >> > > to reflect that a vm snapshot is being taken of a VM, > > > > > > >> > > that's a > > > > > > >> > function of the > > > > > > >> > > apiresponse module that gathers up everything about a > > > > > > >> > > vm but it > > > > > > >> > shouldn't > > > > > > >> > > be changed in the vm states. > > > > > > >> > > > > > > > > > >> > > > [mice] the reason that I added snapshotting/reverting > > > > > > >> > > > state is that > > > > > > >> > VM > > > > > > >> > > could be in suspend/pause state during > > > > > > >> > > snapshoting/reverting, which > > > > > > >> > is > > > > > > >> > > difficult to be categorized into existing states; and > > > > > > >> > > during the > > > > > > >> > process, VM > > > > > > >> > > should not be allowed to take any operations; and by > > > > > > >> > > adding new > > > > > > >> > states to > > > > > > >> > > VM, the implementation seems more 'natural' and only > > > > > > >> > > minimum codes > > > > > > >> > are > > > > > > >> > > changed to virtualmachinemanager. > > > > > > >> > > > Of course there are some other ways to prevent > > > > > > >> > > > operations, such as > > > > > > >> > check > > > > > > >> > > if associated snapshots are in snapshotting/reverting > > > > > > >> > > states either > > > > > > >> > in each > > > > > > >> > > method (start/stop/migrate/delete...) or hook > > > > > > >> > > stateTransitTo(), but > > > > > > >> > in this > > > > > > >> > > way, it does not reflect VM's real state in hypervisor > > > > > > >> > > and more > > > > > > >> > existing codes > > > > > > >> > > will be touched. > > > > > > >> > > > > > > > > > >> > > > - Does VM Revert operation work in the following way: > > > > > > >> > > > Stop VM, > > > > > > >> > restore > > > > > > >> > > to snapshot, and run VM? Shouldn't this be > > > > > > >> > > orchestration inside > > > > > > >> > snapshot > > > > > > >> > > manager? > > > > > > >> > > > > > > > > > >> > > > [mice] if a running VM is reverted to a memory > > > > > > >> > > > enabled snapshot, > > > > > > >> > current > > > > > > >> > > implementation is running--> reverting-->running > > > > > > >> > > > If a stopped VM is reverted to memory disabled snapshot: > > > > > > >> > > > stopped-- > > > > > > >> > > >reverting->stopped > > > > > > >> > > > If a running VM is reverted to a memory disabled snapshot: > > > > > > >> > > > running- > > > > > > >> > -(Stop > > > > > > >> > > VM)-->stopped-->reverting--> stopped > > > > > > >> > > > If a stopped VM is reverted to a memory enabled > snapshot: > > > > > > >> > > > stopped-- > > > > > > >> > > (Start VM)-->running->reverting-->running > > > > > > >> > > > > > > > > > >> > > > These logics are implemented in snapshot manager. > > > > > > >> > > > > > > > > > >> > > > - Does VM snapshot interfere with volume snapshot? > > > > > > >> > > > Volume snapshot > > > > > > >> > > today makes the assumption that it is the only code > > > > > > >> > > that's making > > > > > > >> > snapshots > > > > > > >> > > and can break if there are additional snapshots in between. > > > > > > >> > > This is > > > > > > >> > bad > > > > > > >> > > design in volume snapshot but unfortunately that's how > > > > > > >> > > it's > > > > design. > > > > > > >> > > > > > > > > > >> > > > [mice] about volume snapshot, for xensever, if parent > > > > > > >> > > > VHD cannot be > > > > > > >> > > found, it will take a full volume snapshot (this indeed > > > > > > >> > > break current semantics but it still works) > > > > > > >> > > > For vmware, the volume snapshot is always a full one. > > > > > > >> > > > > > > > > > >> > > > - VMSnapshotService follows the other services in > > > > > > >> > > > passing the cmds > > > > > > >> > to the > > > > > > >> > > service. That's really a bad practice that we should stop. > > > > > > >> > > Cmds are > > > > > > >> > really > > > > > > >> > > translations between over-the-wire api and java interface. > > > > > > >> > > They > > > > > > >> > shouldn't > > > > > > >> > > have been passed to down to the java interface. > > > > > > >> > > > [ > > > > > > >> > > > mice] I'll change it > > > > > > >> > > > > > > > > > >> > > > A small note: Would it be better to call it VM > > > > > > >> > > > restore than VM > > > > > > >> > revert? > > > > > > >> > > Revert really should be RevertTo which I think is in > > > > > > >> > > the code but is > > > > > > >> > not > > > > > > >> > > consistent. Some places it's just REVERT (for example, > > > > > > >> > > the event is > > > > > > >> > just > > > > > > >> > > revert). > > > > > > >> > > > > > > > > > >> > > > [mice] there is already RESTORE, which is restoring a > > > > > > >> > > > destroyed VM > > > > > > >> > to > > > > > > >> > > stopped. RevertTo is fine with me. > > > > > > >> > > > > > > > > > >> > > > -Mice > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > -----Original Message----- > > > > > > >> > > > From: Alex Huang [mailto:alex.hu...@citrix.com] > > > > > > >> > > > Sent: Friday, February 01, 2013 9:24 AM > > > > > > >> > > > To: CloudStack DeveloperList > > > > > > >> > > > Cc: Mice Xia > > > > > > >> > > > Subject: RE: [MERGE] Support VM Snapshot > > > > > > >> > > > > > > > > > >> > > > Hi Mice, > > > > > > >> > > > > > > > > > >> > > > Sorry it took so long to review this. Wanted to as > > > > > > >> > > > soon as I saw > > > > > > >> > it on the list > > > > > > >> > > but was sick and didn't get a chance. In general, I > > > > > > >> > > think the code > > > > > > >> > is excellent. > > > > > > >> > > I'm impressed how much Cloudstack internal code in > > > > > > >> > > touch and how comfortable the changes look. Nicely done! > > > > > > >> > > > > > > > > > >> > > > I have a few comments: > > > > > > >> > > > - VM states is designed for VM lifecycle. Snapshot > > > > > > >> > > > is not part of > > > > > > >> > VM life > > > > > > >> > > cycle so therefore the state should not be there. I > > > > > > >> > > think it make > > > > > > >> > sense to add > > > > > > >> > > attributes to VM that says "Do Not Change State" and > > > > > > >> > > who changed the > > > > > > >> > VM > > > > > > >> > > to that state. Then virtualmachinemanager must obey > > > > > > >> > > that until the > > > > > > >> > external > > > > > > >> > > caller changes the attribute to now you can change state. > > > > > > >> > > The would > > > > > > >> > make > > > > > > >> > > more sense and decouples snapshotting from vm lifecycle > > > > > > management. > > > > > > >> > > Snapshotting then has it's own state (which I see it > > > > > > >> > > does > > > already). > > > > > > >> > If we want > > > > > > >> > > to reflect that a vm snapshot is being taken of a VM, > > > > > > >> > > that's a > > > > > > >> > function of the > > > > > > >> > > apiresponse module that gathers up everything about a > > > > > > >> > > vm but it > > > > > > >> > shouldn't > > > > > > >> > > be changed in the vm states. > > > > > > >> > > > - Does VM Revert operation work in the following way: > > > > > > >> > > > Stop VM, > > > > > > >> > restore > > > > > > >> > > to snapshot, and run VM? Shouldn't this be > > > > > > >> > > orchestration inside > > > > > > >> > snapshot > > > > > > >> > > manager? > > > > > > >> > > > - Does VM snapshot interfere with volume snapshot? > > > > > > >> > > > Volume snapshot > > > > > > >> > > today makes the assumption that it is the only code > > > > > > >> > > that's making > > > > > > >> > snapshots > > > > > > >> > > and can break if there are additional snapshots in between. > > > > > > >> > > This is > > > > > > >> > bad > > > > > > >> > > design in volume snapshot but unfortunately that's how > > > > > > >> > > it's > > > > design. > > > > > > >> > > > - VMSnapshotService follows the other services in > > > > > > >> > > > passing the cmds > > > > > > >> > to the > > > > > > >> > > service. That's really a bad practice that we should stop. > > > > > > >> > > Cmds are > > > > > > >> > really > > > > > > >> > > translations between over-the-wire api and java interface. > > > > > > >> > > They > > > > > > >> > shouldn't > > > > > > >> > > have been passed to down to the java interface. > > > > > > >> > > > > > > > > > >> > > > A small note: Would it be better to call it VM > > > > > > >> > > > restore than VM > > > > > > >> > revert? > > > > > > >> > > Revert really should be RevertTo which I think is in > > > > > > >> > > the code but is > > > > > > >> > not > > > > > > >> > > consistent. Some places it's just REVERT (for example, > > > > > > >> > > the event is > > > > > > >> > just > > > > > > >> > > revert). > > > > > > >> > > > > > > > > > >> > > > --Alex > > > > > > >> > > > > > > > > > >> > > >> -----Original Message----- > > > > > > >> > > >> From: Chiradeep Vittal > > > > > > >> > > >> Sent: Wednesday, January 30, 2013 4:44 PM > > > > > > >> > > >> To: CloudStack DeveloperList > > > > > > >> > > >> Cc: Alex Huang > > > > > > >> > > >> Subject: Re: [MERGE] Support VM Snapshot > > > > > > >> > > >> > > > > > > >> > > >> Can we get Alex to review this? He is the designer > > > > > > >> > > >> of the state > > > > > > >> > machine. > > > > > > >> > > >> > > > > > > >> > > >> On 1/30/13 5:26 AM, "Murali Reddy" > > > > > > >> > > >> <murali.re...@citrix.com> > > > > > > >> wrote: > > > > > > >> > > >> > > > > > > >> > > >> >On 30/01/13 2:24 PM, "Mice Xia" > > > > > > >> > > >> ><mice_...@tcloudcomputing.com> > > > > > > >> > > wrote: > > > > > > >> > > >> > > > > > > > >> > > >> >>Agreed. > > > > > > >> > > >> >>Adding VM states are likely to have some > > > > > > >> > > >> >>side-effects, but for moveVMToUser case, does it > > > > > > >> > > >> >>explicitly reject other transient > > > > > > >> > states > > > > > > >> > > >> >>such as stating/stopping/migrating? > > > > > > >> > > >> >> > > > > > > >> > > >> >>-Mice > > > > > > >> > > >> > > > > > > > >> > > >> >No, it just accepts any state other than 'Running' > > > > > > >> > > >> >(though it > > > > > > >> > should > > > > > > >> > > >> >have checked for the valid states in which VM can > > > > > > >> > > >> >move to other > > > > > > >> > user). > > > > > > >> > > >> > > > > > > > >> > > >> >I am just saying, there could such VM state based > > > > > > >> > > >> >assumptions, > > > > > > >> > you > > > > > > >> > > >> >might want to check. > > > > > > >> > > >> > > > > > > > >> > > > > > > > > > >
