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.

> 
> Rich.
> 

_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to