You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by deepti dohare <de...@citrix.com> on 2013/02/01 08:37:05 UTC
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/
-----------------------------------------------------------
(Updated Feb. 1, 2013, 7:37 a.m.)
Review request for cloudstack.
Changes
-------
Resubmitting the patch with new changes.
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.
This addresses bug CLOUDSTACK-357.
Diffs (updated)
-----
api/src/org/apache/cloudstack/api/command/user/iso/DeleteIsoCmd.java 4c370c7
server/src/com/cloud/template/TemplateManagerImpl.java 42106b3
server/src/com/cloud/vm/dao/UserVmDao.java 9fbcde3
server/src/com/cloud/vm/dao/UserVmDaoImpl.java db1d877
ui/scripts/templates.js 040ce4a
Diff: https://reviews.apache.org/r/7660/diff/
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
Thanks,
deepti dohare
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
Posted by Nitin Mehta <Ni...@citrix.com>.
Code committed with 4d573ddd1bcd4ab27edfb91791a830308236521b
From: Deepti Dohare <de...@citrix.com>>
Reply-To: Deepti Dohare <de...@citrix.com>>
To: Min Chen <mi...@citrix.com>>
Cc: Nitin Mehta <ni...@citrix.com>>, Wei Zhou <w....@leaseweb.com>>, Deepti Dohare <de...@citrix.com>>, Sateesh Chodapuneedi <sa...@citrix.com>>, "cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>" <cl...@incubator.apache.org>>
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/
Review request for cloudstack and Min Chen.
By deepti dohare.
Updated Feb. 25, 2013, 8:55 a.m.
Changes
Resolved Conflicts.
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 (updated)
* server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java (4d19bfc)
* server/src/com/cloud/storage/download/DownloadMonitorImpl.java (e12bc32)
* server/src/com/cloud/template/HyervisorTemplateAdapter.java (c1177f4)
* server/src/com/cloud/template/TemplateAdapterBase.java (247ce63)
* server/src/com/cloud/template/TemplateManagerImpl.java (101c3d9)
* server/src/com/cloud/vm/dao/UserVmDao.java (9fbcde3)
* server/src/com/cloud/vm/dao/UserVmDaoImpl.java (f2fc10b)
View Diff<https://reviews.apache.org/r/7660/diff/>
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
Posted by deepti dohare <de...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/
-----------------------------------------------------------
(Updated Feb. 25, 2013, 8:55 a.m.)
Review request for cloudstack and Min Chen.
Changes
-------
Resolved Conflicts.
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.
This addresses bug CLOUDSTACK-357.
Diffs (updated)
-----
server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java 4d19bfc
server/src/com/cloud/storage/download/DownloadMonitorImpl.java e12bc32
server/src/com/cloud/template/HyervisorTemplateAdapter.java c1177f4
server/src/com/cloud/template/TemplateAdapterBase.java 247ce63
server/src/com/cloud/template/TemplateManagerImpl.java 101c3d9
server/src/com/cloud/vm/dao/UserVmDao.java 9fbcde3
server/src/com/cloud/vm/dao/UserVmDaoImpl.java f2fc10b
Diff: https://reviews.apache.org/r/7660/diff/
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
Thanks,
deepti dohare
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
Posted by Nitin Mehta <Ni...@citrix.com>.
Your patch doesn't apply
Nitins-MacBook-Air:incubator-cloudstack nitinmehta$ git apply --whitespace=fix ../0001-CS-357-ISO-cannot-be-detached-when-it-is-deleted-fro.patch
error: patch failed: server/src/com/cloud/storage/download/DownloadMonitorImpl.java:78
error: server/src/com/cloud/storage/download/DownloadMonitorImpl.java: patch does not apply
error: patch failed: server/src/com/cloud/template/TemplateAdapterBase.java:76
error: server/src/com/cloud/template/TemplateAdapterBase.java: patch does not apply
From: Nitin Mehta <ni...@citrix.com>>
Reply-To: Nitin Mehta <ni...@citrix.com>>
To: Min Chen <mi...@citrix.com>>
Cc: "cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>" <cl...@incubator.apache.org>>, Nitin Mehta <ni...@citrix.com>>, Deepti Dohare <de...@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/
Ship it!
Great work. Thanks for all the detailed information.
- Nitin
On February 21st, 2013, 10:01 a.m., deepti dohare wrote:
Review request for cloudstack and Min Chen.
By deepti dohare.
Updated Feb. 21, 2013, 10:01 a.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
* server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java (4d1ac02)
* server/src/com/cloud/storage/download/DownloadMonitorImpl.java (6d3cf2a)
* server/src/com/cloud/template/HyervisorTemplateAdapter.java (fe6bc2a)
* server/src/com/cloud/template/TemplateAdapterBase.java (fa677ac)
* 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)
View Diff<https://reviews.apache.org/r/7660/diff/>
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
Posted by Nitin Mehta <ni...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review17017
-----------------------------------------------------------
Ship it!
Great work. Thanks for all the detailed information.
- Nitin Mehta
On Feb. 21, 2013, 10:01 a.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 21, 2013, 10:01 a.m.)
>
>
> Review request for cloudstack and Min Chen.
>
>
> 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.
>
>
> This addresses bug CLOUDSTACK-357.
>
>
> Diffs
> -----
>
> server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java 4d1ac02
> server/src/com/cloud/storage/download/DownloadMonitorImpl.java 6d3cf2a
> server/src/com/cloud/template/HyervisorTemplateAdapter.java fe6bc2a
> server/src/com/cloud/template/TemplateAdapterBase.java fa677ac
> 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
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>
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
Posted by deepti dohare <de...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/
-----------------------------------------------------------
(Updated Feb. 21, 2013, 10:01 a.m.)
Review request for cloudstack and Min Chen.
Changes
-------
Updated the patch for template sync during MS restart.
Manually tested the following:
setup: upload ISO1 and ISO 2
Attach ISO1 to VM1 and VM2
Attach ISO2 to VM3
set storage.cleanup.interval to 300
test cases:
1. delete ISO1 from UI, gets deleted
2. In VM Details of VM1 and VM2, can see detach ISO option
3. ISO1 exists in secondary storage
4. detach ISO1 from VM1, successful
5. ISO1 still exists in secondary storage.
6. Restart MS, template sync will not delete ISO1.
7. Detach ISO1 from VM2, successfull detached.
8. Wait for storage cleanup thread to execute, ISO1 gets deleted from Secondary storage.
9. Detach ISO2 from VM3
10.ISO2 exists in secondary storage, Delete ISO2 form UI, get deleted from secondary storage.
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.
This addresses bug CLOUDSTACK-357.
Diffs (updated)
-----
server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java 4d1ac02
server/src/com/cloud/storage/download/DownloadMonitorImpl.java 6d3cf2a
server/src/com/cloud/template/HyervisorTemplateAdapter.java fe6bc2a
server/src/com/cloud/template/TemplateAdapterBase.java fa677ac
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
Diff: https://reviews.apache.org/r/7660/diff/
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
Thanks,
deepti dohare
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
Posted by Nitin Mehta <ni...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review16837
-----------------------------------------------------------
Thanks Deepti for the work. Really appreciate the hard work.
Can you please confirm me that this iso wont get deleted during template sync which gets triggered during stop start ssvm or MS restart ?
Also can you please put in all the test cases u used for my benefit ?
- Nitin Mehta
On Feb. 19, 2013, 5:21 p.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2013, 5:21 p.m.)
>
>
> Review request for cloudstack and Min Chen.
>
>
> 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.
>
>
> This addresses bug CLOUDSTACK-357.
>
>
> Diffs
> -----
>
> server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java 4d1ac02
> server/src/com/cloud/template/HyervisorTemplateAdapter.java fe6bc2a
> server/src/com/cloud/template/TemplateAdapterBase.java fa677ac
> 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
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>
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
Posted by deepti dohare <de...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/
-----------------------------------------------------------
(Updated Feb. 19, 2013, 5:21 p.m.)
Review request for cloudstack and Min Chen.
Changes
-------
Thanks for the review comments Nitin.
I made the changes to make sure that:
1. ISO will be deleted from the UI, but it is not deleted from the secondary storage as long as it is attached to a VM.
2. The storage cleanup thread will check whether the iso is attached to any vm, if not, it removes the ISO from the secondary storage.
3. Detach operation is now working which was failing before for the vms having attached iso(deleted).
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.
This addresses bug CLOUDSTACK-357.
Diffs (updated)
-----
server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java 4d1ac02
server/src/com/cloud/template/HyervisorTemplateAdapter.java fe6bc2a
server/src/com/cloud/template/TemplateAdapterBase.java fa677ac
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
Diff: https://reviews.apache.org/r/7660/diff/
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
Thanks,
deepti dohare
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
Posted by deepti dohare <de...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/
-----------------------------------------------------------
(Updated Feb. 18, 2013, 1:59 p.m.)
Review request for cloudstack and Min Chen.
Changes
-------
Updated the patch with the suggested changes.
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.
This addresses bug CLOUDSTACK-357.
Diffs (updated)
-----
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
Diff: https://reviews.apache.org/r/7660/diff/
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
Thanks,
deepti dohare
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
Posted by Wei Zhou <w....@leaseweb.com>.
> On Feb. 15, 2013, 9:51 a.m., Wei Zhou wrote:
> > I just see this issue. I had some test about DetachISO at the beginn of this year. I did not add a jira ticket because I think it is not the bug of CloudStack.
> >
> > I was using Ubuntu 12.04, and the qemu version is 1.0+noroms-0ubuntu14.3. Here is the result of my test.
> >
> > (1) If the Guest OS is CentOS 5.5, it works well!
> > ISO can be Attached successfully.
> > ISO can be detached successfully if it is not mounted in Guest OS.
> > ISO cannot be detached if it is mounted in Guest OS (even if DetachISO many times). (mount /dev/cdrom /mnt)
> > ISO can be detached successfully after “umount” the ISO in Guest OS (umount /mnt)
> > ISO will be detached successfully when I add -- force flag, even if it is mounted in Guest OS. (it may be a problem)
> >
> > (2) If the Guest OS is Ubuntu 12.04
> > I create two XML files to attach and eject ISO.
> >
> > attach.xml
> > <disk type='file' device='cdrom'>
> > <source file='/root/KNOPPIX_V7.0.4bootonly-2012-08-20-EN.iso'/>
> > <target dev='hdc' bus='ide'/>
> > </disk>
> >
> > eject.xml
> > <disk type='file' device='cdrom'>
> > <target dev='hdc' bus='ide'/>
> > <readonly/>
> > </disk>
> >
> > a) the result of attachISO looks like random. But it does not matter, as attachISO always works well after the ISO is ejected successfully.
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > error: Failed to update device from attach.xml
> > error: internal error unable to execute QEMU command 'change': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > error: Failed to update device from attach.xml
> > error: internal error unable to execute QEMU command 'change': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > error: Failed to update device from attach.xml
> > error: internal error unable to execute QEMU command 'change': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > b) the ISO can be detached after being ejected some times( sometimes 1, sometimes 2, sometimes N…), no matter whether ISO is mounted or not in Guest OS.
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml
> > error: Failed to update device from eject.xml
> > error: internal error unable to execute QEMU command 'eject': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml
> > error: Failed to update device from eject.xml
> > error: internal error unable to execute QEMU command 'eject': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml
> > Device updated successfully
> >
> > c) if I add --force flag when DetachISO, it works.
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml --force
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml --force
> > Device updated successfully
> >
> >
> >
>
> Wei Zhou wrote:
> I think the ideal result should be
> (1) We can detach ISO when the virtual machine is stopped.
> (2) We can detach ISO when the ISO is attached to but not used(such as mounted/read) in the running virtual machine.
> (3) We cannot detach ISO when the ISO is attach and also used in the running virtual machine.
>
> For a solution, I have upgraded “libvirt” on host from 0.9.8(default) to 1.0.1(the latest version). However, this problem still exists.
>
>
> I also had tested the --force flag, although I do think it is not a good way (!).
> I have changed com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.attachOrDetachDevice(Connect, boolean, String, String).
>
> dm.attachDevice(xml); -> dm.attachDeviceFlags(xml, 4);
> dm.detachDevice(xml); -> dm.detachDeviceFlags(xml, 4);
>
> from http://www.libvirt.org/html/libvirt-libvirt.html#virDomainDeviceModifyFlags
> VIR_DOMAIN_DEVICE_MODIFY_FORCE = 4 Forcibly modify device (ex. force eject a cdrom)
>
> from the /var/log/cloud/management/management-server.log, I can see that the libvirt do not support this flag.
> 2013-01-09 16:30:43,988 DEBUG [agent.transport.Request] (AgentManager-Handler-13:null) Seq 6-1083047954: Processing: { Ans: , MgmtId: 345051234346, via: 6, Ver: v1, Flags: 110, [{"Answer":{"result":false,"details":"org.libvirt.LibvirtException: invalid argument: qemuDomainModifyDeviceFlags: unsupported flags (0x4)","wait":0}}] }
>
I think this issue is because of the hypervisor, not cloudstack. A good way is to tell the user to follow when DetachISO fails
(1) umount ISO inside the virtual machine, and try again.
(2) try again (and again...)
(3) shutdown the virtual machine and detach iso.
- Wei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review16639
-----------------------------------------------------------
On Feb. 15, 2013, 5:02 a.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 5:02 a.m.)
>
>
> Review request for cloudstack.
>
>
> 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.
>
>
> This addresses bug 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
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>
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
Posted by Wei Zhou <w....@leaseweb.com>.
> On Feb. 15, 2013, 9:51 a.m., Wei Zhou wrote:
> > I just see this issue. I had some test about DetachISO at the beginn of this year. I did not add a jira ticket because I think it is not the bug of CloudStack.
> >
> > I was using Ubuntu 12.04, and the qemu version is 1.0+noroms-0ubuntu14.3. Here is the result of my test.
> >
> > (1) If the Guest OS is CentOS 5.5, it works well!
> > ISO can be Attached successfully.
> > ISO can be detached successfully if it is not mounted in Guest OS.
> > ISO cannot be detached if it is mounted in Guest OS (even if DetachISO many times). (mount /dev/cdrom /mnt)
> > ISO can be detached successfully after “umount” the ISO in Guest OS (umount /mnt)
> > ISO will be detached successfully when I add -- force flag, even if it is mounted in Guest OS. (it may be a problem)
> >
> > (2) If the Guest OS is Ubuntu 12.04
> > I create two XML files to attach and eject ISO.
> >
> > attach.xml
> > <disk type='file' device='cdrom'>
> > <source file='/root/KNOPPIX_V7.0.4bootonly-2012-08-20-EN.iso'/>
> > <target dev='hdc' bus='ide'/>
> > </disk>
> >
> > eject.xml
> > <disk type='file' device='cdrom'>
> > <target dev='hdc' bus='ide'/>
> > <readonly/>
> > </disk>
> >
> > a) the result of attachISO looks like random. But it does not matter, as attachISO always works well after the ISO is ejected successfully.
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > error: Failed to update device from attach.xml
> > error: internal error unable to execute QEMU command 'change': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > error: Failed to update device from attach.xml
> > error: internal error unable to execute QEMU command 'change': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > error: Failed to update device from attach.xml
> > error: internal error unable to execute QEMU command 'change': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > b) the ISO can be detached after being ejected some times( sometimes 1, sometimes 2, sometimes N…), no matter whether ISO is mounted or not in Guest OS.
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml
> > error: Failed to update device from eject.xml
> > error: internal error unable to execute QEMU command 'eject': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml
> > error: Failed to update device from eject.xml
> > error: internal error unable to execute QEMU command 'eject': Device 'drive-ide0-1-0' is locked
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml
> > Device updated successfully
> >
> > c) if I add --force flag when DetachISO, it works.
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml --force
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
> > Device updated successfully
> >
> > root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml --force
> > Device updated successfully
> >
> >
> >
I think the ideal result should be
(1) We can detach ISO when the virtual machine is stopped.
(2) We can detach ISO when the ISO is attached to but not used(such as mounted/read) in the running virtual machine.
(3) We cannot detach ISO when the ISO is attach and also used in the running virtual machine.
For a solution, I have upgraded “libvirt” on host from 0.9.8(default) to 1.0.1(the latest version). However, this problem still exists.
I also had tested the --force flag, although I do think it is not a good way (!).
I have changed com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.attachOrDetachDevice(Connect, boolean, String, String).
dm.attachDevice(xml); -> dm.attachDeviceFlags(xml, 4);
dm.detachDevice(xml); -> dm.detachDeviceFlags(xml, 4);
from http://www.libvirt.org/html/libvirt-libvirt.html#virDomainDeviceModifyFlags
VIR_DOMAIN_DEVICE_MODIFY_FORCE = 4 Forcibly modify device (ex. force eject a cdrom)
from the /var/log/cloud/management/management-server.log, I can see that the libvirt do not support this flag.
2013-01-09 16:30:43,988 DEBUG [agent.transport.Request] (AgentManager-Handler-13:null) Seq 6-1083047954: Processing: { Ans: , MgmtId: 345051234346, via: 6, Ver: v1, Flags: 110, [{"Answer":{"result":false,"details":"org.libvirt.LibvirtException: invalid argument: qemuDomainModifyDeviceFlags: unsupported flags (0x4)","wait":0}}] }
- Wei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review16639
-----------------------------------------------------------
On Feb. 15, 2013, 5:02 a.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 5:02 a.m.)
>
>
> Review request for cloudstack.
>
>
> 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.
>
>
> This addresses bug 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
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>
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
Posted by Wei Zhou <w....@leaseweb.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review16639
-----------------------------------------------------------
I just see this issue. I had some test about DetachISO at the beginn of this year. I did not add a jira ticket because I think it is not the bug of CloudStack.
I was using Ubuntu 12.04, and the qemu version is 1.0+noroms-0ubuntu14.3. Here is the result of my test.
(1) If the Guest OS is CentOS 5.5, it works well!
ISO can be Attached successfully.
ISO can be detached successfully if it is not mounted in Guest OS.
ISO cannot be detached if it is mounted in Guest OS (even if DetachISO many times). (mount /dev/cdrom /mnt)
ISO can be detached successfully after “umount” the ISO in Guest OS (umount /mnt)
ISO will be detached successfully when I add -- force flag, even if it is mounted in Guest OS. (it may be a problem)
(2) If the Guest OS is Ubuntu 12.04
I create two XML files to attach and eject ISO.
attach.xml
<disk type='file' device='cdrom'>
<source file='/root/KNOPPIX_V7.0.4bootonly-2012-08-20-EN.iso'/>
<target dev='hdc' bus='ide'/>
</disk>
eject.xml
<disk type='file' device='cdrom'>
<target dev='hdc' bus='ide'/>
<readonly/>
</disk>
a) the result of attachISO looks like random. But it does not matter, as attachISO always works well after the ISO is ejected successfully.
root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
Device updated successfully
root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
error: Failed to update device from attach.xml
error: internal error unable to execute QEMU command 'change': Device 'drive-ide0-1-0' is locked
root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
error: Failed to update device from attach.xml
error: internal error unable to execute QEMU command 'change': Device 'drive-ide0-1-0' is locked
root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
error: Failed to update device from attach.xml
error: internal error unable to execute QEMU command 'change': Device 'drive-ide0-1-0' is locked
root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
Device updated successfully
b) the ISO can be detached after being ejected some times( sometimes 1, sometimes 2, sometimes N…), no matter whether ISO is mounted or not in Guest OS.
root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
Device updated successfully
root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml
error: Failed to update device from eject.xml
error: internal error unable to execute QEMU command 'eject': Device 'drive-ide0-1-0' is locked
root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml
error: Failed to update device from eject.xml
error: internal error unable to execute QEMU command 'eject': Device 'drive-ide0-1-0' is locked
root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml
Device updated successfully
c) if I add --force flag when DetachISO, it works.
root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
Device updated successfully
root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml --force
Device updated successfully
root@cs-kvm002:~# virsh update-device i-2-17-VM attach.xml
Device updated successfully
root@cs-kvm002:~# virsh update-device i-2-17-VM eject.xml --force
Device updated successfully
- Wei Zhou
On Feb. 15, 2013, 5:02 a.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 5:02 a.m.)
>
>
> Review request for cloudstack.
>
>
> 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.
>
>
> This addresses bug 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
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>
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
Posted by Sateesh Chodapuneedi <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review16627
-----------------------------------------------------------
api/src/org/apache/cloudstack/api/command/user/iso/DeleteIsoCmd.java
<https://reviews.apache.org/r/7660/#comment35294>
prefix the variable with _ as it is private.
server/src/com/cloud/template/TemplateManagerImpl.java
<https://reviews.apache.org/r/7660/#comment35295>
Use camel casing for variable uservms.
server/src/com/cloud/template/TemplateManagerImpl.java
<https://reviews.apache.org/r/7660/#comment35296>
Check for null before attempting to invoke a method onver this object.
server/src/com/cloud/template/TemplateManagerImpl.java
<https://reviews.apache.org/r/7660/#comment35297>
Use Camel casing for variable uservm.
server/src/com/cloud/template/TemplateManagerImpl.java
<https://reviews.apache.org/r/7660/#comment35300>
Could include the vm in question.
server/src/com/cloud/template/TemplateManagerImpl.java
<https://reviews.apache.org/r/7660/#comment35299>
The message could be more informative by providing the virtual machines that are having this iso attached.
- Sateesh Chodapuneedi
On Feb. 15, 2013, 5:02 a.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 5:02 a.m.)
>
>
> Review request for cloudstack.
>
>
> 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.
>
>
> This addresses bug 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
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>
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
Posted by Nitin Mehta <Ni...@citrix.com>.
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 <de...@citrix.com>>
Reply-To: Deepti Dohare <de...@citrix.com>>
To: Min Chen <mi...@citrix.com>>
Cc: "cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>" <cl...@incubator.apache.org>>, Deepti Dohare <de...@citrix.com>>, Nitin Mehta <ni...@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/>
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
Posted by deepti dohare <de...@citrix.com>.
> On Feb. 15, 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.
>
> 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
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review16634
-----------------------------------------------------------
On Feb. 18, 2013, 1:59 p.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2013, 1:59 p.m.)
>
>
> Review request for cloudstack and Min Chen.
>
>
> 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.
>
>
> This addresses bug 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
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>
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
Posted by Nitin Mehta <Ni...@citrix.com>.
Great thanks Deepti.
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.
-Nitin
From: Deepti Dohare <de...@citrix.com>>
Reply-To: Deepti Dohare <de...@citrix.com>>
To: "cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>" <cl...@incubator.apache.org>>, Nitin Mehta <ni...@citrix.com>>, Deepti Dohare <de...@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.
Nitin, I agree with you and will update the patch with the suggestions you gave.
- deepti
On February 15th, 2013, 5:02 a.m., deepti dohare wrote:
Review request for cloudstack.
By deepti dohare.
Updated Feb. 15, 2013, 5:02 a.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/>
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
Posted by deepti dohare <de...@citrix.com>.
> On Feb. 15, 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.
Nitin, I agree with you and will update the patch with the suggestions you gave.
- deepti
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review16634
-----------------------------------------------------------
On Feb. 15, 2013, 5:02 a.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 5:02 a.m.)
>
>
> Review request for cloudstack.
>
>
> 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.
>
>
> This addresses bug 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
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>
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
Posted by Nitin Mehta <ni...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review16634
-----------------------------------------------------------
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.
- Nitin Mehta
On Feb. 15, 2013, 5:02 a.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2013, 5:02 a.m.)
>
>
> Review request for cloudstack.
>
>
> 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.
>
>
> This addresses bug 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
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>
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
Posted by deepti dohare <de...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/
-----------------------------------------------------------
(Updated Feb. 15, 2013, 5:02 a.m.)
Review request for cloudstack.
Changes
-------
Removed conflicts.
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.
This addresses bug CLOUDSTACK-357.
Diffs (updated)
-----
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
Diff: https://reviews.apache.org/r/7660/diff/
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
Thanks,
deepti dohare
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
Posted by Min Chen <mi...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7660/#review16518
-----------------------------------------------------------
This patch looks good, but I cannot apply this to latest master, here is the error:
Mins-MacBook-Pro:cloud-asf-patch minc$ git am 0001-CLOUDSTACK-357-ISOs-can-be-deleted-while-still-attac.patch
Applying: CLOUDSTACK-357 ISOs can be deleted while still attached to a running VM
error: patch failed: server/src/com/cloud/vm/dao/UserVmDaoImpl.java:61
error: server/src/com/cloud/vm/dao/UserVmDaoImpl.java: patch does not apply
Patch failed at 0001 CLOUDSTACK-357 ISOs can be deleted while still attached to a running VM
Please address this ASAP, and upload a new diff, I will commit to master.
- Min Chen
On Feb. 1, 2013, 7:37 a.m., deepti dohare wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7660/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2013, 7:37 a.m.)
>
>
> Review request for cloudstack.
>
>
> 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.
>
>
> This addresses bug CLOUDSTACK-357.
>
>
> Diffs
> -----
>
> api/src/org/apache/cloudstack/api/command/user/iso/DeleteIsoCmd.java 4c370c7
> server/src/com/cloud/template/TemplateManagerImpl.java 42106b3
> server/src/com/cloud/vm/dao/UserVmDao.java 9fbcde3
> server/src/com/cloud/vm/dao/UserVmDaoImpl.java db1d877
> ui/scripts/templates.js 040ce4a
>
> Diff: https://reviews.apache.org/r/7660/diff/
>
>
> 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
>
>
> Thanks,
>
> deepti dohare
>
>