incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mice Xia <weiran.x...@gmail.com>
Subject Re: [MERGE] Support VM Snapshot
Date Mon, 11 Mar 2013 09:49:11 GMT
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.chaturvedi@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.Xu@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_xia@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.Xu@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.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 <mice_xia@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.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" <Murali.Reddy@citrix.com>
>> wrote:
>> > > >>
>> > > >> >On 30/01/13 2:24 PM, "Mice Xia" <mice_xia@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.
>> > > >> >
>> > > >
>

Mime
View raw message