[Libguestfs] virt-v2v: Virtio-Scsi patch

[Jean-Louis Dupond]

On 15/09/2020 21:08, Richard W.M. Jones wrote:
> On Tue, Sep 15, 2020 at 09:54:32AM +0200, Jean-Louis Dupond wrote:
>> Hi,
>>
>> I was trying to migrate some VM's to Virtio-SCSI block devices, as
>> this gives some advantages.
>>
>> While checking the virt-v2v code, I found out that it supported
>> Virtio-SCSI, but some bits were missing.
>> In attachment a small patch that adds the missing bits :)
> This isn't the patch I was expecting this morning :-)  I'm definitely
> looking forward also to the SMP changes.
The SMP 'change' is quite stupid atm :)
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 73edff2c..271e2b03 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -88,6 +88,7 @@ let rec main () =

    let g = open_guestfs ~identifier:"v2v" () in
    g#set_memsize (g#get_memsize () * 14 / 5);
+  g#set_smp 4;
    (* The network is only used by the unconfigure_vmware () function. *)
    g#set_network true;
    (match conversion_mode with


No commandline flag or so (for now).
>
> - - -
>
> I think this patch as it stands has two problems:
>
> (a) It should be split up into 3 patches.
>
> Patch #1 would contain the change to v2v/linux_kernels.mli and
> v2v/linux_kernels.ml.  IMHO this patch #1 would be completely
> uncontroversial and would go straight upstream.
>
> Patch #2 would contain the change to v2v/create_ovf.ml, and is also
> uncontroversial and ready to go upstream (but as a separate change
> from patch #1).
But then you end up in a situation like the code is now.
In some places Virtio_SCSI is added as a choise, but its never used 
because it misses code.

If we add Patch #1 and #2, we end up in the same situation no?
>
> (b) Now the problem is patch #3:
>
>> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
>> index a871d754..a36cc21d 100644
>> --- a/v2v/convert_linux.ml
>> +++ b/v2v/convert_linux.ml
>> @@ -109,7 +109,7 @@ let convert (g : G.guestfs) inspect source_disks output rcaps _ =
>>   
>>       let block_type =
>>         match rcaps.rcaps_block_bus with
>> -      | None -> if kernel.ki_supports_virtio_blk then Virtio_blk else IDE
>> +      | None -> if kernel.ki_supports_virtio_scsi then Virtio_SCSI else if kernel.ki_supports_virtio_blk then Virtio_blk else IDE
>>         | Some block_type -> block_type in
>>   
>>       let net_type =
> I believe this has the possibility of breaking some guests which use
> hard-coded device names in /etc/fstab and other files, because
> v2v/target_bus_assignment.ml is not very robust for Virtio_SCSI, and
> certainly not well tested (maybe Virtuozzo have tested this a lot more
> that Red Hat).
Could you elaborate why this would cause issues? Cause now it only uses 
virtio_scsi when there is a SCSI_VIRTIO driver in the kernel.
So this should be safe? In my tests it did not cause any issue.
>
> At the moment we're also looking at fixing Q35 support which kind of
> overlaps here (because old SATA devices get renamed to the same as
> virtio-scsi device names), so I would look at these two together.
>
> You can certainly submit patch #3 as part of the series, but I'll
> likely hang on to it before adding it so we can do some testing with
> the Q35 changes.
What needs to be done to support Q35? Cause the migrations I've been 
doing are added as Q35 on oVirt :)
>
> Rich.
>