2:46 a.m.
On 03/08/22 17:17, Richard W.M. Jones wrote:
On Tue, Mar 08, 2022 at 03:30:58PM +0100, Laszlo Ersek wrote:
> The "Firstboot.add_firstboot_script" function now takes an (optional)
> priority parameter. Because "install_firstboot_powershell" is a wrapper
> around "Firstboot.add_firstboot_script", enable callers of the former too
> to set the script run priority.
I had a double-take with the "too to" (but it is correct).
Maybe s/too/also/ or s/too//?
Yes, I'll drop "too".
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1788823
> Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
> ---
> convert/windows.mli | 6 +++---
> convert/windows.ml | 5 +++--
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/convert/windows.mli b/convert/windows.mli
> index d888e2db40b3..4c7f292832ac 100644
> --- a/convert/windows.mli
> +++ b/convert/windows.mli
> @@ -23,7 +23,7 @@ val detect_antivirus : Types.inspect -> bool
> this Windows guest. *)
>
> val install_firstboot_powershell : Guestfs.guestfs -> Types.inspect ->
> - string -> string list -> unit
> -(** [install_powershell_firstboot g inspect filename code] installs a
> + ?prio:int -> string -> string list ->
unit
> +(** [install_powershell_firstboot g inspect prio filename code] installs a
> Powershell script (the lines of code) as a firstboot script in
> - the Windows VM. *)
> + the Windows VM. If [prio] is omitted, [Firstboot.default_prio] is used. *)
> diff --git a/convert/windows.ml b/convert/windows.ml
> index fc0faef57249..8a30be10d21a 100644
> --- a/convert/windows.ml
> +++ b/convert/windows.ml
> @@ -52,7 +52,8 @@ and (=~) str rex = PCRE.matches rex str
> * a regular batch file.
> *)
> let install_firstboot_powershell g { Types.i_windows_systemroot; i_root }
> - filename code =
> + ?(prio = Firstboot.default_prio) filename
> + code =
> let tempdir = sprintf "%s/Temp" i_windows_systemroot in
> g#mkdir_p tempdir;
> let code = String.concat "\r\n" code ^ "\r\n" in
> @@ -67,4 +68,4 @@ let install_firstboot_powershell g { Types.i_windows_systemroot;
i_root }
> let ps_path = i_windows_systemroot ^ "\\Temp\\" ^ filename in
>
> let fb = sprintf "%s -ExecutionPolicy ByPass -file %s" ps_exe ps_path
in
> - Firstboot.add_firstboot_script g i_root filename fb
> + Firstboot.add_firstboot_script g i_root ~prio filename fb
So this is a bit convoluted. I note you exposed default_prio
in the Firstboot module which I was a little confused by, but
now I see why you did it.
However you didn't need to. You can instead do:
> let install_firstboot_powershell g { Types.i_windows_systemroot; i_root }
> - filename code =
> + ?prio filename code =
...
> - Firstboot.add_firstboot_script g i_root filename fb
> + Firstboot.add_firstboot_script g i_root ?prio filename fb
?prio has type 'int option'.
Huh, this syntax is not in the book I've been consulting!
So yes, this is exactly what I needed.
So ACK with that change, and drop default_prio from Firstboot too.
Yes.
When pushing this patch, don't forget to update the common submodule
in the same commit!
I figured I'd push a submodule update first, but fusing it with this
patch is more descriptive indeed.
Thanks!
Laszlo
Thanks,
Rich.