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.
>