+++ This bug was initially created as a clone of Bug #1746699 +++

Description of problem:
Can't import guest from export domain to data domain on rhv4.3 due to error "Invalid parameter: 'DiskType=1'"

Version-Release number of selected component (if applicable):
vdsm-4.30.27-1.el7ev.x86_64
RHV:4.3.4.3-0.1.el7

Steps to Reproduce:
1.Convert a guests from VMware to RHV's export domain by virt-v2v
# virt-v2v -i ova esx6_7-rhel7.7-x86_64 -o rhv -os 10.73.224.199:/home/p2v_export -of qcow2 -b ovirtmgmt
[   0.0] Opening the source -i ova esx6_7-rhel7.7-x86_64
[   8.7] Creating an overlay to protect the source from being modified
[   8.9] Opening the overlay
[  13.2] Inspecting the overlay
[  37.9] Checking for sufficient free disk space in the guest
[  37.9] Estimating space required on target for each disk
[  37.9] Converting Red Hat Enterprise Linux Server 7.7 Beta (Maipo) to run on KVM
virt-v2v: warning: guest tools directory ‘linux/el7’ is missing from 
the virtio-win directory or ISO.

Guest tools are only provided in the RHV Guest Tools ISO, so this can 
happen if you are using the version of virtio-win which contains just the 
virtio drivers.  In this case only virtio drivers can be installed in the 
guest, and installation of Guest Tools will be skipped.
virt-v2v: This guest has virtio drivers installed.
[ 184.2] Mapping filesystem data to avoid copying unused and blank areas
[ 184.9] Closing the overlay
[ 185.0] Assigning disks to buses
[ 185.0] Checking if the guest needs BIOS or UEFI to boot
[ 185.0] Initializing the target -o rhv -os 10.73.224.199:/home/p2v_export
[ 185.4] Copying disk 1/2 to /tmp/v2v.43WPcK/e7cd32d9-6b7d-4be9-ad0f-3fb7cfeeea3b/images/c2b64a63-85ca-402f-a775-391849776152/4344f61d-5a07-45ec-a3c6-e0b5041f9b8e (qcow2)
    (100.00/100%)
[ 438.6] Copying disk 2/2 to /tmp/v2v.43WPcK/e7cd32d9-6b7d-4be9-ad0f-3fb7cfeeea3b/images/0569bfe8-3857-4997-9c06-93248e809ab3/e8f2ad4d-adc4-4d10-bd46-92cd545e1b12 (qcow2)
    (100.00/100%)
[ 439.4] Creating output metadata
[ 439.5] Finishing off


2.Try to import the guest from export domain to data domain, but failed to import guest with below error

VDSM p2v command HSMGetAllTasksStatusesVDS failed: low level Image copy failed: (u"Destination volume 4344f61d-5a07-45ec-a3c6-e0b5041f9b8e error: Invalid parameter: 'DiskType=1'",)

Additional info:
Can't reproduce the bug with vdsm-4.30.12-1.el7ev.x86_64

--- Additional comment from Nir Soffer on 2019-09-02 16:34:31 CEST ---

(In reply to mxie from comment #0)
...
> 1.Convert a guests from VMware to RHV's export domain by virt-v2v
> # virt-v2v -i ova esx6_7-rhel7.7-x86_64 -o rhv -os

OK, the bug seems to be in virt-v2 then.

Looking in v2v/create_ovf.ml

 497       let buf = Buffer.create 256 in
 498       let bpf fs = bprintf buf fs in
 499       bpf "DOMAIN=%s\n" sd_uuid; (* "Domain" as in Storage Domain *)
 500       bpf "VOLTYPE=LEAF\n";
 501       bpf "CTIME=%.0f\n" time;
 502       bpf "MTIME=%.0f\n" time;
 503       bpf "IMAGE=%s\n" image_uuid;
 504       bpf "DISKTYPE=1\n";
 505       bpf "PUUID=00000000-0000-0000-0000-000000000000\n";
 506       bpf "LEGALITY=LEGAL\n";
 507       bpf "POOL_UUID=\n";
 508       bpf "SIZE=%Ld\n" size_in_sectors;
 509       bpf "FORMAT=%s\n" format_for_rhv;
 510       bpf "TYPE=%s\n" output_alloc_for_rhv;
 511       bpf "DESCRIPTION=%s\n" (String.replace generated_by "=" "_");
 512       bpf "EOF\n";

Looks like the .meta file is generated by virt-v2v. This usage is not supported
by RHV since the .meta files are not part of RHV API.

This means also that rhv output does not support block storage (no .meta file)
and will create incorrect .meta files when using storage format v5.

I think this bug should move to virt-v2v. RHV should not support corrupted meta
data created by external tools bypassing RHV API.

Richard, what do you think?

--- Additional comment from Richard W.M. Jones on 2019-09-02 17:49:08 CEST ---

This is the old -o rhv mode which doesn't do via the RHV API at all.  It's also
a deprecated mode in virt-v2v.  And AIUI the Export Storage Domain which it uses
is also deprecated in RHV.

As for why this error has suddenly appeared, I'm not sure why but it has
to be because of some change in RHV to do with handling of ESDs.

--- Additional comment from Richard W.M. Jones on 2019-09-02 17:52:01 CEST ---

Of historical note, the DISKTYPE=1 was copied from the old Perl virt-v2v.
I've no idea what that did since I didn't write it.

That git repo is not actually online any longer but the code was:

lib/Sys/VirtConvert/Connection/RHEVTarget.pm:    print $meta "DISKTYPE=1\n";

--- Additional comment from Nir Soffer on 2019-09-02 18:09:47 CEST ---

(In reply to Richard W.M. Jones from comment #23)
> This is the old -o rhv mode which doesn't do via the RHV API at all.  It's
> also
> a deprecated mode in virt-v2v.  And AIUI the Export Storage Domain which it
> uses
> is also deprecated in RHV.

I guess there is no point in fixing this code to use the correct value at
this point.

> As for why this error has suddenly appeared, I'm not sure why but it has
> to be because of some change in RHV to do with handling of ESDs.

The error was exposed in 4.3 since we started to validate the disk type 
when creating new volumes. Older versions of vdsm were writing the value
as is to storage without any validation.

Since we have corrupted metadata files in existing export domains, I think
we can workaround this issue by accepting also DISKTYPE=1.

--- Additional comment from Nir Soffer on 2019-09-02 18:13:03 CEST ---

Tal, this can be fixed with a trivial patch, targeting to 4.3.6.

--- Additional comment from Richard W.M. Jones on 2019-09-02 18:18:29 CEST ---

(In reply to Nir Soffer from comment #30)
> Since we have corrupted metadata files in existing export domains, I think
> we can workaround this issue by accepting also DISKTYPE=1.

I should say that the way -o rhv works is it copies the disks to
the ESD, and then you're supposed to soon afterwards import them
into RHV.  (This of course long predates RHV even having an API).

So the disks shouldn't exist in the ESD for very long.  It may
therefore not be necessary to work around this in RHV.

My question is what should the DISKTYPE field actually contain?  Maybe
we can put the proper data into the .meta file or remove this field
entirely?

--- Additional comment from Nir Soffer on 2019-09-02 18:38:18 CEST ---

(In reply to Richard W.M. Jones from comment #32)
> (In reply to Nir Soffer from comment #30)
> > Since we have corrupted metadata files in existing export domains, I think
> > we can workaround this issue by accepting also DISKTYPE=1.
> 
> I should say that the way -o rhv works is it copies the disks to
> the ESD, and then you're supposed to soon afterwards import them
> into RHV.  (This of course long predates RHV even having an API).
> 
> So the disks shouldn't exist in the ESD for very long.  It may
> therefore not be necessary to work around this in RHV.

It depends on engine, if it deletes the exported vm right after the import,
but based on reports from other users I suspect that the vms are not deleted.
 
> My question is what should the DISKTYPE field actually contain?  Maybe
> we can put the proper data into the .meta file or remove this field
> entirely?

The correct value is "DISKTYPE=2", so this should fix the issue:

diff --git a/v2v/create_ovf.ml b/v2v/create_ovf.ml
index 91ff5198d..9aad5dd15 100644
--- a/v2v/create_ovf.ml
+++ b/v2v/create_ovf.ml
@@ -501,7 +501,7 @@ let create_meta_files output_alloc sd_uuid image_uuids overlays =
       bpf "CTIME=%.0f\n" time;
       bpf "MTIME=%.0f\n" time;
       bpf "IMAGE=%s\n" image_uuid;
-      bpf "DISKTYPE=1\n";
+      bpf "DISKTYPE=2\n";
       bpf "PUUID=00000000-0000-0000-0000-000000000000\n";
       bpf "LEGALITY=LEGAL\n";
       bpf "POOL_UUID=\n";

But it will not help with existing images, or with engine database containing
the invalid value "1" for imported disks.

--- Additional comment from Richard W.M. Jones on 2019-09-02 22:22:08 CEST ---

Thanks.  Whether or not we also need a fix in RHV, this is now fixed in
virt-v2v in commit fcfdbc9420b07e3003df38481afb9ccd22045e1a (virt-v2v >= 1.41.5).