On Thu, Mar 09, 2023 at 04:38:44PM +0200, Andrey Drobyshev wrote:
> On 3/8/23 22:45, Richard W.M. Jones wrote:
> > On Tue, Mar 07, 2023 at 09:40:26PM +0200, Andrey Drobyshev wrote:
> >> In case when we are injecting virtio-scsi device driver into the guest
> >> (rather than the default virtio-blk), make sure we write the right PCI ID
> >> value into the Windows guest registry. This is essential for the guest
> >> to be bootable afterwards.
> >>
> >> Originally-by: Roman Kagan <rka...@virtuozzo.com>
> >> Signed-off-by: Andrey Drobyshev <andrey.drobys...@virtuozzo.com>
> >> ---
> >> mlcustomize/inject_virtio_win.ml | 12 +++++++++---
> >> 1 file changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/mlcustomize/inject_virtio_win.ml
> >> b/mlcustomize/inject_virtio_win.ml
> >> index 345fe32..922c1ab 100644
> >> --- a/mlcustomize/inject_virtio_win.ml
> >> +++ b/mlcustomize/inject_virtio_win.ml
> >> @@ -207,10 +207,16 @@ let rec inject_virtio_win_drivers ({ g } as t) reg =
> >> let target = sprintf "%s/system32/drivers/%s.sys"
> >> t.i_windows_systemroot driver_name in
> >> let target = g#case_sensitive_path target in
> >> + let installed_block_type, legacy_pciid, modern_pciid = (
> >> + if driver_name = "vioscsi" then
> >> + Virtio_SCSI, vioscsi_legacy_pciid, vioscsi_modern_pciid
> >> + else
> >> + Virtio_blk, viostor_legacy_pciid, viostor_modern_pciid
> >> + ) in
> >> g#cp source target;
> >> - add_guestor_to_registry t reg driver_name viostor_legacy_pciid;
> >> - add_guestor_to_registry t reg driver_name viostor_modern_pciid;
> >> - Virtio_blk in
> >> + add_guestor_to_registry t reg driver_name legacy_pciid;
> >> + add_guestor_to_registry t reg driver_name modern_pciid;
> >> + installed_block_type in
> >>
> >> (* Can we install the virtio-net driver? *)
> >> let net : net_type =
> >
> > Could we make this look more like the code before it was reverted
> > here?
> >
> > https://github.com/libguestfs/virt-v2v/commit/b28cd1dcfeb40e7002e8d0b0ce9dcc4ce86beb6c
> >
> > See the lines starting:
> > | Some Virtio_SCSI, _, true ->
> > (* Block driver needs tweaks to allow booting; the rest is set up
> > by PnP
> > ...
> >
> > So the change would look like:
> >
> > - | Some driver_name ->
> > + | Some "viostor" ->
> > (* Block driver needs tweaks to allow booting;
> > * the rest is set up by PnP manager.
> > *)
> > let source = driverdir // (driver_name ^ ".sys") in
> > let target = sprintf "%s/system32/drivers/%s.sys"
> > t.i_windows_systemroot driver_name in
> > let target = g#case_sensitive_path target in
> > g#cp source target;
> > add_guestor_to_registry t reg driver_name viostor_legacy_pciid;
> > add_guestor_to_registry t reg driver_name viostor_modern_pciid;
> > Virtio_blk in
> >
> > + | Some "vioscsi" ->
> > + [... new code for the virtio-scsi case ...]
> >
> >
> > The old code was a bit clearer don't you think? Even if there's a
> > little bit more duplication.
>
> IMO evaluating driver path and adding registry value separately for each
> and every case seems like too much duplication. If you think string
> comparison looks a bit clumsy here, how about nesting another level of
> matching, like so:
>
>
> > | Some driver_name ->
> >
> > (* Block driver needs tweaks to allow booting;
> >
> > * the rest is set up by PnP manager.
> >
> > *)
> >
> > let source = driverdir // (driver_name ^ ".sys") in
> >
> > let target = sprintf "%s/system32/drivers/%s.sys"
> >
> > t.i_windows_systemroot driver_name in
> >
> > let target = g#case_sensitive_path target in
> >
> > let installed_block_type, legacy_pciid, modern_pciid =
> >
> > match driver_name with
> >
> > | "vioscsi" -> Virtio_SCSI, vioscsi_legacy_pciid,
> > vioscsi_modern_pciid
> > | _ -> Virtio_blk, viostor_legacy_pciid, viostor_modern_pciid
> >
> >
> > in
> >
> > g#cp source target;
> >
> > add_guestor_to_registry t reg driver_name legacy_pciid;
> >
> > add_guestor_to_registry t reg driver_name modern_pciid;
> >
> > installed_block_type in
Seems about the same as the original patch really.
But yes, your choice here. You could leave it as in the original version.
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
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
