On Thu, Mar 02, 2023 at 11:31:14PM +0200, Andrey Drobyshev wrote:
> On 3/2/23 22:59, Richard W.M. Jones wrote:
> > On Thu, Mar 02, 2023 at 08:52:33PM +0200, Andrey Drobyshev wrote:
> >> On 3/2/23 20:36, Richard W.M. Jones wrote:
> >>> I can probably do an outline of a patch if you need it, but
> >>> not today.
> >>
> >> Yes, sure. But speaking of the ways to influence the behaviour of a
> >> command line tool: there're also env variables, which are being used by
> >> v2v as well (for instance, for setting the tools ISO location). So what
> >> if we introduce an env variable here instead? I presume it would be
> >> much easier to implement. Smth like:
> >
> > The environment variable is a bit of a mistake too. It'd be better to
> > specify that through the command line; and in fact that's what I went
> > with for the new virt-customize --inject-virtio-win option:
> >
> > --inject-virtio-win METHOD
> > Inject virtio-win drivers into a Windows guest. These drivers
> > add
> > virtio accelerated drivers suitable when running on top of a
> > hypervisor that supports virtio (such as qemu/KVM). The
> > operation
> > also adjusts the Windows Registry so that the drivers are
> > installed
> > when the guest boots.
> >
> > The parameter can be one of:
> >
> > ISO The path to the ISO image containing the virtio-win drivers
> > (eg. /usr/share/virtio-win/virtio-win.iso).
> > [etc]
> >
> >>> diff --git a/mlcustomize/inject_virtio_win.ml
> >>> b/mlcustomize/inject_virtio_win.ml
> >>> index 62f7710..562f055 100644
> >>> --- a/mlcustomize/inject_virtio_win.ml
> >>> +++ b/mlcustomize/inject_virtio_win.ml
> >>> @@ -177,6 +177,11 @@ let rec inject_virtio_win_drivers ({ g } as t) reg =
> >>> (* Can we install the block driver? *)
> >>> let block : block_type =
> >>> let filenames = ["virtio_blk"; "vrtioblk"; "viostor"] in
> >>> + let viostor_drv_env = Sys.getenv_opt "VIOSTOR_DRIVER" in
> >>> + let filenames =
> >>> + match viostor_drv_env with
> >>> + | None -> filenames
> >>> + | Some str -> str :: filenames in
> >>> let viostor_driver = try (
> >>> Some (
> >>> List.find (
> >>
> >> As you can see here the default doesn't change if the variable isn't
> >> provided at all (or it is, but is set to a non-existing driver).
> >> What do you think of such a solution?
> >
> > I prefer the command line option even though it's a little bit more
> > work. Offer above still stands if you can't work out the complexities
> > of OCaml command line parsing.
>
> It's not about the command line parsing complexities, but rather about
> the proper way to pass that option from the top level of the call stack
> (convert ()) down to the inject_virtio_win_drivers (). That'd require
> adding one more extra argument to all the calls along this call stack,
> which might look a bit clumsy. Isn't there a more elegant way?
>
> So having a sketch in that regard would be indeed much appreciated.
OK I'll come up with something, but may not be til Monday.
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
