Deepti – Unfortunately, that’s not the right fix as I have already said below because as a user you shouldn't be allowed to detach iso from other user's vm. That would be a big security hole. I also said you should keep a note of the two things below. Your bug is all about #1 so fix that instead.
Two things to note #1 You might want to be careful about is that when mark the iso removed…the detach operation should still work on the vm. #2 Template sync might come up and delete this iso from sec. storage. You need to be careful for ISOs in that logic. 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: Min Chen <min.c...@citrix.com<mailto:min.c...@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-357 ISOs can be deleted while still attached to a running VM, and they subsequently cannot be detached from a running VM This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7660/ On February 15th, 2013, 8:10 a.m., Nitin Mehta wrote: This gives the ability to the user to affect other user vms as well, which is not desirable. I would suggest you to do the following. When you delete iso, mark it removed in DB so that its not available for further use. In the storage cleanup thread check whether the iso is detached for the vms and if it is then delete it from the sec. storage. On February 15th, 2013, 11:52 a.m., deepti dohare wrote: Nitin, I agree with you and will update the patch with the suggestions you gave. When an iso is getting deleted, it is marked as removed in db. This functionality already exists in the current code select id, name, removed from vm_template where id > 200; +-----+------+---------------------+ | id | name | removed | +-----+------+---------------------+ | 201 | iso1 | 2013-02-18 13:50:31 | | 202 | iso2 | NULL | +-----+------+---------------------+ My change is to detach an iso from all the VMs, if attached, depending on the check option "forced", while deleting an iso. - deepti On February 18th, 2013, 1:59 p.m., deepti dohare wrote: Review request for cloudstack and Min Chen. By deepti dohare. Updated Feb. 18, 2013, 1:59 p.m. Description Currently no warning or confirmation is given or required to delete an iso which is attached to a virtual machine. Fixing this issue using “force” delete option. If an admin or a user deletes an ISO which is attached to a vm: 1. Without force option; gives a warning that ISO is attached to vm “Failed to delete iso. It is currently attached to virtual machine. Please detach it before deleting”. 2. With force option: first detach the ISO from all vms and then deletes the ISO. Testing Verified locally. Tested for some cases: 1. Deleting iso(not attached to any vm) 2. Deleting iso(attached to multiple vms in same domain) 3. Deleting iso(attached to multiple vms in different domains) For admin/user Bugs: CLOUDSTACK-357 Diffs * api/src/org/apache/cloudstack/api/command/user/iso/DeleteIsoCmd.java (c821775) * server/src/com/cloud/template/TemplateManagerImpl.java (f9cf277) * server/src/com/cloud/vm/dao/UserVmDao.java (9fbcde3) * server/src/com/cloud/vm/dao/UserVmDaoImpl.java (f2fc10b) * ui/scripts/templates.js (040ce4a) View Diff<https://reviews.apache.org/r/7660/diff/>
