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

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

Laszlo

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.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit