Bug 1484199

*** Bug 1468535 has been marked as a duplicate of this bug. ***

Created attachment 1317364 [details]
rhel7.4-import-kvm

It seems the type of the disk is changed from virtio scsi (at source) to virtio block in oVirt. VDSM sends a hint to Engine how the disk is connected (vda for virtio block, sda for virtio scsi). Engine should take this under advisement when constructing new VM during import.

(In reply to Tomáš Golembiovský from comment #3)
> It seems the type of the disk is changed from virtio scsi (at source) to
> virtio block in oVirt. VDSM sends a hint to Engine how the disk is connected
> (vda for virtio block, sda for virtio scsi). Engine should take this under
> advisement when constructing new VM during import.

That's not going to fly. The disks are completely different, their /dev/disk/by-id/... looks completely different and of course /dev/sdX vs. /dev/vdX .

Let's make sure the disk type is not changed.

> Let's make sure the disk type is not changed.

That's exactly what I meant.

Actually it is hardcoded in ImportVmFromExternalProviderCommand.createDisk for all the providers, not only for the kvm one.

The good news is that it can be changed manually after the import finishes.

But since this happens for all the providers I think we should consider it for 4.2

(In reply to Tomas Jelinek from comment #6)
> Actually it is hardcoded in ImportVmFromExternalProviderCommand.createDisk
> for all the providers, not only for the kvm one.

Hmm, for VMware and Xen sources, shouldn't we pay attention to the OVF generated by virt-v2v during import?

(In reply to Tomáš Golembiovský from comment #3)
> It seems the type of the disk is changed from virtio scsi (at source) to
> virtio block in oVirt. VDSM sends a hint to Engine how the disk is connected
> (vda for virtio block, sda for virtio scsi). Engine should take this under
> advisement when constructing new VM during import.

Regarding import from KVM source: the problem is that both scsi and sata interfaces are mapped and sent as 'sda' by vdsm. So for engine it is a problem to identify which interface to choose. Can vdsm send a more specific parameter value to distinguish?

You are right, this cannot be done without sending more info from VDSM. I remembered we have already discussed this issue in bug 1362186, comment 5.

 (In reply to Sharon Gratch from comment #9)
> Can vdsm send a more specific parameter value to distinguish?

To answer your question: yes that would be possible.

(In reply to Tomáš Golembiovský from comment #11)
>  (In reply to Sharon Gratch from comment #9)
> > Can vdsm send a more specific parameter value to distinguish?
> 
> To answer your question: yes that would be possible.

I suggest that for now we'll assume that if vdsm sends a hint of dev name set to 'sdx' then scsi interface will be chosen by engine (till now it was virtio by default so no regression will occur for windows vm's and this new behavior is still preferable on always setting virtio regardless to source setting).
I will also open a bug/requirement for reporting the bus name by vdsm to engine.

Sounds good enough to me.

(In reply to Tomáš Golembiovský from comment #8)
> Hmm, for VMware and Xen sources, shouldn't we pay attention to the OVF
> generated by virt-v2v during import?

Sure, and that's why the code Tomas.J referred to in comment 6 didn't matter until 4.0 - we had to provide some default value and that default value was changed later on after inspecting the OVF generated by virt-v2v.

@Sharon, I didn't reproduce it but by looking at the code it seems that this is broken for other provides as well (because of the introduction of DiskVmElement in 4.1) - seems that we clear the images at [1] and thus we don't fix those their elements with the right disk interface at [2], can you please check this?

[1] https://github.com/oVirt/ovirt-engine/blob/ovirt-engine-4.1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/ConvertVmCommand.java#L275
[2] https://github.com/oVirt/ovirt-engine/blob/ovirt-engine-4.1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/ConvertVmCommand.java#L279

(In reply to Tomáš Golembiovský from comment #11)
>  (In reply to Sharon Gratch from comment #9)
> > Can vdsm send a more specific parameter value to distinguish?
> 
> To answer your question: yes that would be possible.

Probably possible but I think there's a better approach. Sure, it would be great to have VDSM reporting the bus as well but it would be even better to extract that kvm2ovirt code from VDSM into an ansible task in the engine and by that to drop the middle-layer.

(In reply to Arik from comment #14)
> @Sharon, I didn't reproduce it but by looking at the code it seems that this
> is broken for other provides as well (because of the introduction of
> DiskVmElement in 4.1) - seems that we clear the images at [1] and thus we
> don't fix those their elements with the right disk interface at [2], can you
> please check this?
> 
> [1]
> https://github.com/oVirt/ovirt-engine/blob/ovirt-engine-4.1/backend/manager/
> modules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/
> ConvertVmCommand.java#L275
> [2]
> https://github.com/oVirt/ovirt-engine/blob/ovirt-engine-4.1/backend/manager/
> modules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/
> ConvertVmCommand.java#L279

I tested it in the past and it always worked for me and now I understand why.
In case there is only 1 disk (bootable one) then it works due to [3]. But you are right that it failed to update disk interface for all non-bootable disks.

I will fix this issue in a separate patch since it's not related to this BZ.

[3] https://github.com/oVirt/ovirt-engine/blob/ae96b03390ad88afacc2fac3ccb87b7bb46cbc5a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/exportimport/ConvertVmCommand.java#L269

(In reply to Arik from comment #15)
> (In reply to Tomáš Golembiovský from comment #11)
> >  (In reply to Sharon Gratch from comment #9)
> > > Can vdsm send a more specific parameter value to distinguish?
> > 
> > To answer your question: yes that would be possible.
> 
> Probably possible but I think there's a better approach. Sure, it would be
> great to have VDSM reporting the bus as well but it would be even better to
> extract that kvm2ovirt code from VDSM into an ansible task in the engine and
> by that to drop the middle-layer.

Are you implying we should be sending comlete domain XML to the engine?

(In reply to Tomáš Golembiovský from comment #17)
> Are you implying we should be sending comlete domain XML to the engine?

Ideally, yes - that would be even better.
But we currently don't have code that processes the whole domain-xml on the engine side (the engine only processes the devices + metadata sections). So it would be easier IMO to extract the python code that does list & dump the VMs using virsh + converts the output to the dictionary that is passed to the engine from VDSM and move that code to the engine - the engine will execute it with ansible on the selected host (that's how the new OVA support was implemented). The benefits would be:
1. the code resides in one place, thus easier to change.
2. no need to preserve backward compatibility.

This bug actually referred to all providers and not just to KVM external provider, as detailed below:
For KVM provider - the imported VM's disk interface type was always set to VIRTIO, regardless to source VM's disk interface types.

For XEN, VMware and 'OVA from VMware' - the imported VM's non bootable disk(s) interface type were always set to VIRTIO, regardless to virt-v2v generated OVA file data. Only Vm's bootable disk was set according to virt-v2v generated OVA data.

For oVirt OVA import -  the imported VM's disk interface type was always set to VIRTIO, regardless to OVA info for disk(s) interface types.

All of that was fixed in related patches.

Verify with:
Engine:
Software Version:4.2.1.6-0.1.el7
Host:
OS Version:RHEL - 7.4 - 18.el7
Kernel Version:3.10.0 - 693.17.1.el7.x86_64
KVM Version:2.9.0 - 16.el7_4.14
LIBVIRT Version:libvirt-3.9.0-8.el7
VDSM Version:vdsm-4.20.17-1.el7ev

Steps:
OVA:
Create VM for each interface:IDE, VIRTIO, VIRTIO-ISCSI
Export VM and import it, check that interface stay the same.
PASS OVA check
KVM:
Import VM with IDE, SATA and VIRTIO
IDE is mapped to IDE
VIRTIO mapped to VIRTIO
SATA mappaed to VIRTIO
PASS KVM check

This bugzilla is included in oVirt 4.2.1 release, published on Feb 12th 2018.

Since the problem described in this bug report should be
resolved in oVirt 4.2.1 release, it has been closed with a resolution of CURRENT RELEASE.

If the solution does not work for you, please open a new bug report.