[virt-tools-list] [virt-manager PATCH] cloner: preserve the disk type in the cloned domain

[Cole Robinson]

On 03/03/2017 12:25 PM, Pavel Hrdina wrote:
> On Fri, Mar 03, 2017 at 12:02:37PM -0500, Cole Robinson wrote:
>> On 03/03/2017 02:19 AM, Pavel Hrdina wrote:
>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1420187
>>>
>>> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>>> ---
>>>  virtinst/cloner.py | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/virtinst/cloner.py b/virtinst/cloner.py
>>> index 5255acfe..ab1f036e 100644
>>> --- a/virtinst/cloner.py
>>> +++ b/virtinst/cloner.py
>>> @@ -409,7 +409,7 @@ class Cloner(object):
>>>  
>>>              # Change the XML
>>>              xmldisk.path = None
>>> -            xmldisk.type = clone_disk.type
>>> +            xmldisk.type = orig_disk.type
>>>              xmldisk.driver_name = orig_disk.driver_name
>>>              xmldisk.driver_type = orig_disk.driver_type
>>>              xmldisk.path = clone_disk.path
>>>
>>
>> This should have a test case. But I don't know if this is right... what if you
>> are cloning from a block device to a file? The type should change in that case
>> and not match the original type. Maybe the root issue of that bug is that
>> VirtualDisk isn't correctly detecting the new path as a block device?
> 
> Right, I thought that there is something that this patch would break.  Well
> I was trying to figure this out and the issue is that CloneStorageCreator
> that is used if we are cloning locally without libvirt change the type from
> "block" to "file" because in this case the the StorageCreator._vol_install
> is not set and StorageCreator.get_dev_type() is used as "default_cb"
> for VirtualDisk.type which is used on that line I was trying to change.
> 
> There might be some different hacky way how to fix it, I'll try to find it.
> 
> However I don't like that virt-manager/virt-clone supports cloning a disks
> locally.  IMHO everything should be done using Libvirt APIs in order to have
> the same functionality on local and remote connection and if there is no API
> to cover some specific task that is done locally we should try to implement
> a new API in Libvirt to do that task for us.
> 

Yeah the local thing is annoying, we basically have to keep that behavior
since it exists from before storage API days unfortunately, I too would like
to drop it.

- Cole