[Libguestfs] [PATCH] convert_linux: translate the first CD-ROM's references in boot conf files

[Laszlo Ersek]

On 12/14/21 21:51, Richard W.M. Jones wrote:
> On Tue, Dec 14, 2021 at 08:46:45PM +0000, Richard W.M. Jones wrote:
>> On Tue, Dec 14, 2021 at 04:17:49PM +0100, Laszlo Ersek wrote:
>>> +    match cdroms with
>>> +    | _ :: _ :: _ -> warning (f_"multiple CD-ROMs found; translation of \
>>> +                                 CD-ROM references may be inexact")
>>> +    | _ -> ();
>>> +
>>> +    let map = map @
>>
>> This part is either wrong or indented incorrectly.  The "let map ..."
>> part is part of the second branch of the match statement and doesn't
>> run if the warning is printed.
>>
>> In any case it's probably better to replace the confusing first match
>> with something simpler such as:
>>
>>   if List.length cdroms > 2 then
>>     warning (f_"multiple CD-ROMs found; translation of CD-ROM references may be inexact");
>>
>> The \ is also not necessary in the original since OCaml lets you have
>> arbitrary length strings over multiple lines (a good thing), plus we
>> automatically wrap warning and error strings on whitespace.
>>
>>> +      (match cdroms with
>>> +       | cdrom :: _ ->
>>> +           (match (cdrom.s_removable_controller, cdrom.s_removable_slot,
>>> +                   family, inspect.i_major_version) with
>>> +            | Some Source_IDE, Some slot, `RHEL_family, v when v <= 5 ->
>>
>> You can just match to arbitrary depth in single match statements, so
>> the whole match would become something like:
>>
>>   let map = map @
>>     match cdroms with
>>     | { Some Source_IDE, Some slot, `RHEL_family v } :: _ when v <= 5 ->
> 
> This line should have been:
> 
>  | { s_removable_slot = Some Source_IDE; s_removable_slot = Some slot } :: _
>       when family = `RHEL_family && inspect.i_major_version <= 5 ->

Yes, in an earlier version, I used some structure field matching, and
just found it impossible to remember the syntax for that. Had to look it
up in the book... So, now I'm even less sure that this is more readable
:) Please advise, I'll heed it.

Thanks,
Laszlo

> 
> Rich.
> 
>>            [("hd" ^ drive_name slot, "cdrom")]
>>     | _ -> [] in
>>
>> Rich.
>>
>> -- 
>> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
>> Read my programming and virtualization blog: http://rwmj.wordpress.com
>> libguestfs lets you edit virtual machines.  Supports shell scripting,
>> bindings from many languages.  http://libguestfs.org
>