virt-v2v: Virtio-Scsi patch
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 :) Best regards Jean-Louis
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. - - - 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). (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). 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. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
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.
On Wed, Sep 16, 2020 at 09:10:01AM +0200, Jean-Louis Dupond wrote:
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
OK, I have an improved patch which sets this to the number of processors on the host. Will post it shortly.
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?
It's up to you, but generally the smaller the delta between upstream and what you have, the better.
(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.
Simply because it needs massively more testing. This package will make it to hundreds of Red Hat customers who have thousands of configurations, and we need to make sure it doesn't break a single one. We'll do this testing, eventually, but I want to combine it with the Q35 changes we are also doing. Rich.
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.
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
On Wed, Sep 16, 2020 at 09:10:01AM +0200, Jean-Louis Dupond wrote:
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
FWIW this was what I put upstream: https://github.com/libguestfs/virt-v2v/commit/d2b64ecc67012d57e463c228f1ac5f... Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
participants (2)
-
Jean-Louis Dupond -
Richard W.M. Jones