Return-Path: X-Original-To: apmail-incubator-cloudstack-dev-archive@minotaur.apache.org Delivered-To: apmail-incubator-cloudstack-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 699D1DAAE for ; Mon, 11 Mar 2013 09:49:38 +0000 (UTC) Received: (qmail 63006 invoked by uid 500); 11 Mar 2013 09:49:37 -0000 Delivered-To: apmail-incubator-cloudstack-dev-archive@incubator.apache.org Received: (qmail 62839 invoked by uid 500); 11 Mar 2013 09:49:37 -0000 Mailing-List: contact cloudstack-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: cloudstack-dev@incubator.apache.org Delivered-To: mailing list cloudstack-dev@incubator.apache.org Received: (qmail 62805 invoked by uid 99); 11 Mar 2013 09:49:36 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 11 Mar 2013 09:49:36 +0000 X-ASF-Spam-Status: No, hits=-0.5 required=5.0 tests=FREEMAIL_ENVFROM_END_DIGIT,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of weiran.xia1@gmail.com designates 209.85.223.181 as permitted sender) Received: from [209.85.223.181] (HELO mail-ie0-f181.google.com) (209.85.223.181) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 11 Mar 2013 09:49:32 +0000 Received: by mail-ie0-f181.google.com with SMTP id 17so4399377iea.40 for ; Mon, 11 Mar 2013 02:49:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:content-transfer-encoding; bh=jtUzTErPepFKKJhP7Oh3G5k+QXWaMzTp2qR4YXCNQ8k=; b=Ck8jocPx9/68ydb1yOS5jovBIWy+b5IrNhtcseVy1BpwOk91HuyVqPY29+QJ61SxAF 5CEhrqtkgwi+zMExBYa6Rl9YbKytaLoBIlNJlf/IJ5QyK4Jt1UB14WqGxsJ+VeNKyhAU WgkytIiq6UbF3XFmHIaGgyddOheBR/VXSHM9u94IfVK9DJUu2O0KhYQokiBg7jyh8iOe cknesaEuO6EcYl6+NHOI3/5WlMa8iGAvuU+0bbXkR71hezsDc/5Dwp48dnL+cECR7El1 /3Kka7dFYS+BVdebBiiMHcYOqETFlE5u4+LK+rSbqezFmZpc+Mrfxuz56rnM4Eu6KWLj zBcA== MIME-Version: 1.0 X-Received: by 10.50.108.235 with SMTP id hn11mr6335207igb.107.1362995352044; Mon, 11 Mar 2013 02:49:12 -0700 (PDT) Received: by 10.42.128.14 with HTTP; Mon, 11 Mar 2013 02:49:11 -0700 (PDT) In-Reply-To: <7A92FF96DF135843B4B608FB576BFC3E012F75182D84@SJCPMAILBOX01.citrite.net> References: <2C97F788CCC013428671BC3A5FC2F64D15AAB1@c-mail.cloud-valley.com.cn> <2C97F788CCC013428671BC3A5FC2F64D057376@c-mail.cloud-valley.com.cn> <7A92FF96DF135843B4B608FB576BFC3E012F75182D84@SJCPMAILBOX01.citrite.net> Date: Mon, 11 Mar 2013 17:49:11 +0800 Message-ID: Subject: Re: [MERGE] Support VM Snapshot From: Mice Xia To: cloudstack-dev@incubator.apache.org Cc: animesh.chaturvedi@citrix.com Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: quoted-printable X-Virus-Checked: Checked by ClamAV on apache.org 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 : > Folks following up on this thread looks like support for XenServer is sti= ll not settled. > > 1. Mice can we make the feature configurable for each hypervisor to enabl= e/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.Xu@citrix.com] >> Sent: Friday, February 01, 2013 5:44 PM >> To: 'Mice Xia'; cloudstack-dev@incubator.apache.org; Alex Huang; Mice Xi= a >> Subject: RE: [MERGE] Support VM Snapshot >> >> I see, snapshot manager detected the change in primary storage, and crea= te >> a full snapshot instead, which is supposed to be a delta snapshot. >> >> It doesn=A1=AFt break volume snapshot function, but this degrades the vo= lume >> snapshot performance. >> >> >> >> This is just a simple test, it cannot prove there is no impact to volume >> snapshot. >> >> I=A1=AFm 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=A1=AFm 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_xia@tcloudcomputing.com] >> Sent: Friday, February 01, 2013 5:30 PM >> To: cloudstack-dev@incubator.apache.org; Alex Huang; Mice Xia; Anthony X= u >> Subject: =B4=F0=B8=B4: [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.Xu@citrix.com] >> Sent: 2013-2-2 (=D0=C7=C6=DA=C1=F9) 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 i= n >> 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 snap= shot >> 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.xia1@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 : >> > > > 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.Huang@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" >> wrote: >> > > >> >> > > >> >On 30/01/13 2:24 PM, "Mice Xia" >> > > 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. >> > > >> > >> > > > >