[virt-tools-list] 答复： Re: [virt-manager PATCH 1/7] details: Disallow removing the scsi controller if disks attached to it

[Lin Ma]

>>> Cole Robinson <crobinso at redhat.com> 2017/11/22 星期三 上午 6:18 >>>
>On 11/06/2017 07:52 AM, Lin Ma wrote:
>> Through virt-manager, After we removed a virtio-scsi controller which
>> virtual disks still attach to it, Libvirt will add a LSI scsi controller
>> for this guest automatically and trigger a lifecycle event, virt-manager
>> updates and shows this new scsi controller in details panel once it got
>> the lifecycle event.
>> 
>> It may confuse user that a LSI scsi controller occurs while one removes
>> the virtio-scsi controller.
>> 
>> This patch prevents removing a scsi controller if any disks attaching to
>> it.
>> 
>> Signed-off-by: Lin Ma <lma at suse.com>
>> ---
>>  virtManager/details.py | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/virtManager/details.py b/virtManager/details.py
>> index a51f5623..0923f4b0 100644
>> --- a/virtManager/details.py
>> +++ b/virtManager/details.py
>> @@ -3000,6 +3000,12 @@ class vmmDetails(vmmGObjectUI):
>>			  can_remove = False
>>		  if dev.type == "pci":
>>			  can_remove = False
>> +	    if dev.type == "scsi":
>> +		    for disk in self.vm.get_disk_devices(inactive=True):
>> +			    if (dev.type == disk.bus and
>> +				    dev.index == disk.address.controller):
>> +				    can_remove = False
>> +				    break
>>		  self.widget("config-remove").set_sensitive(can_remove)
>>  
>>		  type_label = dev.pretty_desc()
>> 
>
>Good idea, we should do this for other controller types too I think.
No problem, I'd like to do it for other controller types(say SATA,
VirtIO Serial and Floppy in a new patch after this patch got merged.

>(Another nice addition would be setting a tooltip on config-remove in
>these cases to at least give the chance of informing the user _why_ the
>device can't be removed. but again not required)
ok, I'll try to add them after these patches got merged.

>Why inactive=True here though? If you drop it, it will use the current
>state of the VM active or inactive which seems the best option

Yes, dropping it is the best option, I'll do it.
 
Thanks!
Lin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20171122/89177751/attachment.htm>