Re: [Libguestfs] [V2V PATCH 0/5] Bring support for virtio-scsi back to Windows

On Tue, Feb 28, 2023 at 03:24:46PM +0100, Laszlo Ersek wrote: > On 2/28/23 14:01, Richard W.M. Jones wrote: >> On Wed, Feb 22, 2023 at 08:20:43PM +0200, Andrey Drobyshev wrote: >>> Since commits b28cd1dc ("Remove requested_guestcaps / rcaps"), f0afc439 >>> ("Remove guestcaps_block_type Virtio_SCSI") support for installing >>> virtio-scsi driver is missing in virt-v2v. AFAIU plans and demands for >>> bringing this feature back have been out there for a while. E.g. I've >>> found a corresponding issue which is still open [1]. >>> >>> The code in b28cd1dc, f0afc439 was removed due to removing the old in-place >>> support. However, having the new in-place support present and bringing >>> this same code (partially) back with several additions and improvements, >>> I'm able to successfully convert and boot a Win guest with a virtio-scsi >>> disk controller. So please consider the following implementation of >>> this feature. >>> >>> [1] https://github.com/libguestfs/virt-v2v/issues/12 >> >> So you're right that we ended up removing virtio-scsi support because >> in-place conversion was temporarily removed, and it wasn't restored >> when the new virt-v2v-in-place tool was added. >> >> I looked at the patches, and I don't have any objection to 1-4. >> >> Patch 5 is the tricky one because it changes the default, but we >> (Red Hat) are likely to stick with virtio-blk for better or worse. >> >> Could we make this configurable? We've traditionally shied away from >> adding virt-v2v command line options which are purely for fine-tuning >> conversions. The reason for this is that when you're doing bulk >> conversions of hundreds of VMs from VMware, you want virt-v2v to do >> "the best it can", and it's not really feasible to hand configure >> every conversion. (There are some historical exceptions to this, like >> --root, but don't look at those!) I know the bulk conversion case >> doesn't apply to Virtuozzo, but it does matter for us. >> >> It could be argued that this isn't the same, because if we're bulk >> converting, you're not fine-tuning virtio-scsi vs virtio-blk per VM, >> but per target hypervisor. So a command line option would be OK in >> this case. >> >> BTW previously we "configured" this preference through the input >> libvirt XML. That was a terrible idea, so I'd prefer to avoid it this >> time around. > > Ah, that's new information to me. I thought the "rcaps" (requested caps, > derived from the input XML) was exactly what was missing here. Not > because of the particular reason(s) that may have motivated rcaps in the > past, but because in the use case at hand, the tweaking of the domain > XML precedes the conversion (more like, the injection of the desired > virtio drivers). > > Why was it a terrible idea? It was a hack. For copying (normal) conversions -i libvirtxml describes the input metadata. For in-place conversions "input" is a fuzzy concept, so it was overloaded to describe the conversion preferences. We can do better with a command line parameter.

>> Anyway if you can think of a better version of patch 5 then I think we >> can accept this. > > ITYM "if you *cannot* think of a better version"... > > But anyway, if a command line option is acceptable to you, I'm perfectly > fine with it as well. I'd like to avoid changing the default. I quite > foresee tens of QE test runs breaking, resulting in as many bug reports, > and then us eventually reverting the change-of-the-default downstream. > I'd rather favor an experimental command line switch. Yes I'm sure we don't want to change the default; or at least we (meaning Red Hat, downstream) want to choose whether and when to change the default.

BTW/FYI we don't ship virt-v2v-in-place in RHEL (we do in Fedora). Thanks, Rich.