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-dev@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-dev@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-dev@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-dev@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-dev@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-dev@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. > >> > > >> > > >> > > > > >
