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

[Pavel Hrdina]

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.

Thanks for the review, I'll try to come up with better solution.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20170303/58b28682/attachment.sig>