Deepti – Please find answers inline. Only thing you need to fix is #2 below.
From: Deepti Dohare <deepti.doh...@citrix.com<mailto:deepti.doh...@citrix.com>> Reply-To: Deepti Dohare <deepti.doh...@citrix.com<mailto:deepti.doh...@citrix.com>> To: Anthony Xu <xuefei...@citrix.com<mailto:xuefei...@citrix.com>> Cc: "cloudstack-dev@incubator.apache.org<mailto:cloudstack-dev@incubator.apache.org>" <cloudstack-dev@incubator.apache.org<mailto:cloudstack-dev@incubator.apache.org>>, Deepti Dohare <deepti.doh...@citrix.com<mailto:deepti.doh...@citrix.com>>, Nitin Mehta <nitin.me...@citrix.com<mailto:nitin.me...@citrix.com>> Subject: Re: Review Request: CLOUDSTACK-356 Snapshot errors with multiple secondary storage in one zone. This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7594/ On February 15th, 2013, 7:08 a.m., Nitin Mehta wrote: #1 Fixing the path will potentially break the migration path. So you need to correct the sec. storage id for snapshots otherwise the user wont be able to restore the vm from the snapshot which is not good. This can be a very tricky problem to solve. But you need to fix it. #2 I would advise you not to include the dc_id in the path. Since sec host id would be unique for the cloud. #3 I am not completely convinced that this fix solves the problem. I know there is a bug of multiple sec. storage not having paths on the host and should be fixed but your example doesn't seem right to me. 1. I tested the patch for migration of VMs and it was working fine. Secondary mount point was successfully created on the destination host. Can you tell me the scenario where migration will fail, since the explanation is not clear to me. Or Do you mean migration from an older version of cloudstack to newer version. If so, this patch will work, since CloudStack will mount the secondary storage in the new mount point, and will get the parent vhd for snapshots. I was talking CS migration and I think you need to paste the example of first snapshot being a different path and 2nd snapshot having a different path mounted. That makes it clear so this is not an issue here. 2. dc_id is there in the existing code of CloudStack. Are you saying instead of mount path:/var/run/cloud_mount/dc_id/snapshots/secHostid, /var/run/cloud_mount/secHostid/snapshots/ should be used. How will removing dc_id will help? dc_id with secondary storage id is also unique How will keeping dc_id help ? To me this is a useless value with no significance bcz secondary id is unique by itself. 3. Can you please elaborate more on the 3rd point. #3 is the same is #1 so that is clear. Just fix #2 - deepti On February 15th, 2013, 6:31 a.m., deepti dohare wrote: Review request for cloudstack and anthony xu. By deepti dohare. Updated Feb. 15, 2013, 6:31 a.m. Description Fixed CLOUDSTACK-356 The issue is happening because of the multiple secondary storages, the secondary storage which is used first is getting mounted on the host resulting in failure of other snapshot using different secondary storage as it is not getting mounted on the host. For eg: 1. When first incremental snapshot (eg Ssec1-1) is taken, secondary storage (sec1) is mounted on local path “/var/run/sr-mount/ on the host. 2. Second incremental snapshot (Ssec1-2) is created, to get the parent vhd, the secondary storage is mounted at mount path “/var/run/cloud_mount/dc_id/snapshots”. 3. If other snapshot (for different volume) is created on different secondary storage (sec2), sec2 is not getting mounted on "/var/run/cloud_mount/dc_id/snapshots" path. The fix is to mount other secondary storages as well on the host. The local mount path in vmopsSnapshot.py is modified, instead of mounting on “/var/run/cloud_mount/dc_id/snapshots”, “/var/run/cloud_mount/dc_id/snapshots/secHostid” mount path is used. Testing Tested for following cases: 1. Snapshot Creation: Host 1 has a vm i-1, volume ROOT-1 DATA-1, Host 2 has a vm i-2, volume ROOT-2 DATA-2, Taken recurring snapshot for 4 volumes. Verified: multiple secondary storages are getting mounted on host. Snapshots creation successful 2. Template Creation: Verified: templates successfully created from the above snapshots 3. Volume creation: Verified: volumes successfully created from the above snapshots 4. VM Migration Migrate i-1 from Host-1 to Host-2. Verified: Secondary mount point successfully created on Host-2. Bugs: CLOUDSTACK-356 Diffs * api/src/com/cloud/agent/api/BackupSnapshotCommand.java (a0ac8d7) * api/test/org/apache/cloudstack/api/agent/test/BackupSnapshotAnswerTest.java (ede86e9) * api/test/org/apache/cloudstack/api/agent/test/BackupSnapshotCommandTest.java (7836b6d) * plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java (22f4ba9) * scripts/vm/hypervisor/xenserver/vmopsSnapshot (6fb1b18) * scripts/vm/hypervisor/xenserver/xcposs/vmopsSnapshot (f7b2e0e) * server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java (e06da75) View Diff<https://reviews.apache.org/r/7594/diff/>
