thr3ads.net - Libguestfs - [Libguestfs] [V2V PATCH v2 1/5] Revert "Remove guestcaps_block_type Virtio

If this information is useful, please help other people find it:
Share via:

Andrey Drobyshev

2023-Mar-13 08:22 UTC

[Libguestfs] [V2V PATCH v2 1/5] Revert "Remove guestcaps_block_type Virtio_SCSI"

On 3/13/23 10:13, Laszlo Ersek wrote:> On 3/7/23 20:40, Andrey Drobyshev wrote:
>> This code is needed to check whether virtio-scsi driver was installed.
>>
>> This reverts commit f0afc439524853508938b2bfc758896f053462e3.
>> ---
>>  convert/convert.ml                   | 2 +-
>>  convert/convert_linux.ml             | 9 +++++++--
>>  convert/target_bus_assignment.ml     | 1 +
>>  lib/create_ovf.ml                    | 1 +
>>  lib/types.ml                         | 3 ++-
>>  lib/types.mli                        | 2 +-
>>  output/openstack_image_properties.ml | 7 +++++++
>>  7 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/convert/convert.ml b/convert/convert.ml
>> index 0aa0e5cd..084619c8 100644
>> --- a/convert/convert.ml
>> +++ b/convert/convert.ml
>> @@ -252,7 +252,7 @@ and do_convert g source inspect i_firmware
keep_serial_console interfaces >>    (* Did we manage to install virtio
drivers? *)
>>    if not (quiet ()) then (
>>      match guestcaps.gcaps_block_bus with
>> -    | Virtio_blk ->
>> +    | Virtio_blk | Virtio_SCSI ->
>>          info (f_"This guest has virtio drivers installed.")
>>      | IDE ->
>>          info (f_"This guest does not have virtio drivers
installed.")
>> diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
>> index d5c0f24d..dab4f36d 100644
>> --- a/convert/convert_linux.ml
>> +++ b/convert/convert_linux.ml
>> @@ -1068,8 +1068,12 @@ let convert (g : G.guestfs) source inspect
i_firmware keep_serial_console _ >>      (* Update 'alias
scsi_hostadapter ...' *)
>>      let paths = augeas_modprobe ". =~
regexp('scsi_hostadapter.*')" in
>>      (match block_type with
>> -    | Virtio_blk ->
>> -      let block_module = "virtio_blk" in
>> +    | Virtio_blk | Virtio_SCSI ->
>> +      let block_module >> +        match block_type with
>> +        | Virtio_blk -> "virtio_blk"
>> +        | Virtio_SCSI -> "virtio_scsi"
>> +        | IDE -> assert false in
>>  
>>        if paths <> [] then (
>>          (* There's only 1 scsi controller in the converted guest.
>> @@ -1142,6 +1146,7 @@ let convert (g : G.guestfs) source inspect
i_firmware keep_serial_console _ >>      let block_prefix_after_conversion
>>        match block_type with
>>        | Virtio_blk -> "vd"
>> +      | Virtio_SCSI -> "sd"
>>        | IDE -> ide_block_prefix in
>>  
>>      let map >> diff --git a/convert/target_bus_assignment.ml
b/convert/target_bus_assignment.ml
>> index 54c9516b..d13340c7 100644
>> --- a/convert/target_bus_assignment.ml
>> +++ b/convert/target_bus_assignment.ml
>> @@ -35,6 +35,7 @@ let rec target_bus_assignment source_disks
source_removables guestcaps >>      let bus >>        match
guestcaps.gcaps_block_bus with
>>        | Virtio_blk -> virtio_blk_bus
>> +      | Virtio_SCSI -> scsi_bus
>>        | IDE -> ide_bus in
>>      List.iteri (
>>        fun i d ->
>> diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml
>> index 5e444868..f0b32e01 100644
>> --- a/lib/create_ovf.ml
>> +++ b/lib/create_ovf.ml
>> @@ -920,6 +920,7 @@ and add_disks sizes guestcaps output_alloc
output_format
>>            "ovf:disk-interface",
>>            (match guestcaps.gcaps_block_bus with
>>            | Virtio_blk -> "VirtIO"
>> +          | Virtio_SCSI -> "VirtIO_SCSI"
>>            | IDE -> "IDE");
>>            "ovf:disk-type", "System"; (*
RHBZ#744538 *)
>>            "ovf:boot", if is_bootable_drive then
"True" else "False";
>> diff --git a/lib/types.ml b/lib/types.ml
>> index e16da007..75c14fd4 100644
>> --- a/lib/types.ml
>> +++ b/lib/types.ml
>> @@ -400,12 +400,13 @@ type guestcaps = {
>>    gcaps_arch_min_version : int;
>>    gcaps_virtio_1_0 : bool;
>>  }
>> -and guestcaps_block_type = Virtio_blk | IDE
>> +and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
>>  and guestcaps_net_type = Virtio_net | E1000 | RTL8139
>>  and guestcaps_machine = I440FX | Q35 | Virt
>>  
>>  let string_of_block_type = function
>>    | Virtio_blk -> "virtio-blk"
>> +  | Virtio_SCSI -> "virtio-scsi"
>>    | IDE -> "ide"
>>  let string_of_net_type = function
>>    | Virtio_net -> "virtio-net"
>> diff --git a/lib/types.mli b/lib/types.mli
>> index 4a183dd3..65ef2e35 100644
>> --- a/lib/types.mli
>> +++ b/lib/types.mli
>> @@ -280,7 +280,7 @@ type guestcaps = {
>>  }
>>  (** Guest capabilities after conversion.  eg. Was virtio found or
installed? *)
>>  
>> -and guestcaps_block_type = Virtio_blk | IDE
>> +and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
>>  and guestcaps_net_type = Virtio_net | E1000 | RTL8139
>>  and guestcaps_machine = I440FX | Q35 | Virt
>>  
>> diff --git a/output/openstack_image_properties.ml
b/output/openstack_image_properties.ml
>> index c75d72fe..c76ad913 100644
>> --- a/output/openstack_image_properties.ml
>> +++ b/output/openstack_image_properties.ml
>> @@ -35,6 +35,7 @@ let create source inspect { target_buses; guestcaps;
target_firmware } >>      "hw_disk_bus",
>>      (match guestcaps.gcaps_block_bus with
>>       | Virtio_blk -> "virtio"
>> +     | Virtio_SCSI -> "scsi"
>>       | IDE -> "ide");
>>      "hw_vif_model",
>>      (match guestcaps.gcaps_net_bus with
>> @@ -69,6 +70,12 @@ let create source inspect { target_buses; guestcaps;
target_firmware } >>        List.push_back properties
("hw_cpu_threads", string_of_int threads);
>>    );
>>  
>> +  (match guestcaps.gcaps_block_bus with
>> +   | Virtio_SCSI ->
>> +      List.push_back properties ("hw_scsi_model",
"virtio-scsi")
>> +   | Virtio_blk | IDE -> ()
>> +  );
>> +
>>    (match inspect.i_major_version, inspect.i_minor_version with
>>     | 0, 0 -> ()
>>     | x, 0 -> List.push_back properties ("os_version",
string_of_int x)
> 
> Looks like a reasonable revert to me.
> 
> Acked-by: Laszlo Ersek <lersek at redhat.com>
> 
Hi Laszlo,

You're reviewing v2, but the latest series which Richard has approved is v3:

https://listman.redhat.com/archives/libguestfs/2023-March/031035.html
https://listman.redhat.com/archives/libguestfs/2023-March/031041.html

I added you to Cc, but maybe it wasn't delivered?

Laszlo Ersek

2023-Mar-13 08:40 UTC

head link

[Libguestfs] [V2V PATCH v2 1/5] Revert "Remove guestcaps_block_type Virtio_SCSI"

On 3/13/23 09:22, Andrey Drobyshev wrote:> On 3/13/23 10:13, Laszlo Ersek wrote:
>> On 3/7/23 20:40, Andrey Drobyshev wrote:
>>> This code is needed to check whether virtio-scsi driver was
installed.
>>>
>>> This reverts commit f0afc439524853508938b2bfc758896f053462e3.
>>> ---
>>>  convert/convert.ml                   | 2 +-
>>>  convert/convert_linux.ml             | 9 +++++++--
>>>  convert/target_bus_assignment.ml     | 1 +
>>>  lib/create_ovf.ml                    | 1 +
>>>  lib/types.ml                         | 3 ++-
>>>  lib/types.mli                        | 2 +-
>>>  output/openstack_image_properties.ml | 7 +++++++
>>>  7 files changed, 20 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/convert/convert.ml b/convert/convert.ml
>>> index 0aa0e5cd..084619c8 100644
>>> --- a/convert/convert.ml
>>> +++ b/convert/convert.ml
>>> @@ -252,7 +252,7 @@ and do_convert g source inspect i_firmware
keep_serial_console interfaces >>>    (* Did we manage to install
virtio drivers? *)
>>>    if not (quiet ()) then (
>>>      match guestcaps.gcaps_block_bus with
>>> -    | Virtio_blk ->
>>> +    | Virtio_blk | Virtio_SCSI ->
>>>          info (f_"This guest has virtio drivers
installed.")
>>>      | IDE ->
>>>          info (f_"This guest does not have virtio drivers
installed.")
>>> diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
>>> index d5c0f24d..dab4f36d 100644
>>> --- a/convert/convert_linux.ml
>>> +++ b/convert/convert_linux.ml
>>> @@ -1068,8 +1068,12 @@ let convert (g : G.guestfs) source inspect
i_firmware keep_serial_console _ >>>      (* Update 'alias
scsi_hostadapter ...' *)
>>>      let paths = augeas_modprobe ". =~
regexp('scsi_hostadapter.*')" in
>>>      (match block_type with
>>> -    | Virtio_blk ->
>>> -      let block_module = "virtio_blk" in
>>> +    | Virtio_blk | Virtio_SCSI ->
>>> +      let block_module >>> +        match block_type with
>>> +        | Virtio_blk -> "virtio_blk"
>>> +        | Virtio_SCSI -> "virtio_scsi"
>>> +        | IDE -> assert false in
>>>  
>>>        if paths <> [] then (
>>>          (* There's only 1 scsi controller in the converted
guest.
>>> @@ -1142,6 +1146,7 @@ let convert (g : G.guestfs) source inspect
i_firmware keep_serial_console _ >>>      let
block_prefix_after_conversion >>>        match block_type with
>>>        | Virtio_blk -> "vd"
>>> +      | Virtio_SCSI -> "sd"
>>>        | IDE -> ide_block_prefix in
>>>  
>>>      let map >>> diff --git
a/convert/target_bus_assignment.ml b/convert/target_bus_assignment.ml
>>> index 54c9516b..d13340c7 100644
>>> --- a/convert/target_bus_assignment.ml
>>> +++ b/convert/target_bus_assignment.ml
>>> @@ -35,6 +35,7 @@ let rec target_bus_assignment source_disks
source_removables guestcaps >>>      let bus >>>        match
guestcaps.gcaps_block_bus with
>>>        | Virtio_blk -> virtio_blk_bus
>>> +      | Virtio_SCSI -> scsi_bus
>>>        | IDE -> ide_bus in
>>>      List.iteri (
>>>        fun i d ->
>>> diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml
>>> index 5e444868..f0b32e01 100644
>>> --- a/lib/create_ovf.ml
>>> +++ b/lib/create_ovf.ml
>>> @@ -920,6 +920,7 @@ and add_disks sizes guestcaps output_alloc
output_format
>>>            "ovf:disk-interface",
>>>            (match guestcaps.gcaps_block_bus with
>>>            | Virtio_blk -> "VirtIO"
>>> +          | Virtio_SCSI -> "VirtIO_SCSI"
>>>            | IDE -> "IDE");
>>>            "ovf:disk-type", "System"; (*
RHBZ#744538 *)
>>>            "ovf:boot", if is_bootable_drive then
"True" else "False";
>>> diff --git a/lib/types.ml b/lib/types.ml
>>> index e16da007..75c14fd4 100644
>>> --- a/lib/types.ml
>>> +++ b/lib/types.ml
>>> @@ -400,12 +400,13 @@ type guestcaps = {
>>>    gcaps_arch_min_version : int;
>>>    gcaps_virtio_1_0 : bool;
>>>  }
>>> -and guestcaps_block_type = Virtio_blk | IDE
>>> +and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
>>>  and guestcaps_net_type = Virtio_net | E1000 | RTL8139
>>>  and guestcaps_machine = I440FX | Q35 | Virt
>>>  
>>>  let string_of_block_type = function
>>>    | Virtio_blk -> "virtio-blk"
>>> +  | Virtio_SCSI -> "virtio-scsi"
>>>    | IDE -> "ide"
>>>  let string_of_net_type = function
>>>    | Virtio_net -> "virtio-net"
>>> diff --git a/lib/types.mli b/lib/types.mli
>>> index 4a183dd3..65ef2e35 100644
>>> --- a/lib/types.mli
>>> +++ b/lib/types.mli
>>> @@ -280,7 +280,7 @@ type guestcaps = {
>>>  }
>>>  (** Guest capabilities after conversion.  eg. Was virtio found or
installed? *)
>>>  
>>> -and guestcaps_block_type = Virtio_blk | IDE
>>> +and guestcaps_block_type = Virtio_blk | Virtio_SCSI | IDE
>>>  and guestcaps_net_type = Virtio_net | E1000 | RTL8139
>>>  and guestcaps_machine = I440FX | Q35 | Virt
>>>  
>>> diff --git a/output/openstack_image_properties.ml
b/output/openstack_image_properties.ml
>>> index c75d72fe..c76ad913 100644
>>> --- a/output/openstack_image_properties.ml
>>> +++ b/output/openstack_image_properties.ml
>>> @@ -35,6 +35,7 @@ let create source inspect { target_buses;
guestcaps; target_firmware } >>>      "hw_disk_bus",
>>>      (match guestcaps.gcaps_block_bus with
>>>       | Virtio_blk -> "virtio"
>>> +     | Virtio_SCSI -> "scsi"
>>>       | IDE -> "ide");
>>>      "hw_vif_model",
>>>      (match guestcaps.gcaps_net_bus with
>>> @@ -69,6 +70,12 @@ let create source inspect { target_buses;
guestcaps; target_firmware } >>>        List.push_back properties
("hw_cpu_threads", string_of_int threads);
>>>    );
>>>  
>>> +  (match guestcaps.gcaps_block_bus with
>>> +   | Virtio_SCSI ->
>>> +      List.push_back properties ("hw_scsi_model",
"virtio-scsi")
>>> +   | Virtio_blk | IDE -> ()
>>> +  );
>>> +
>>>    (match inspect.i_major_version, inspect.i_minor_version with
>>>     | 0, 0 -> ()
>>>     | x, 0 -> List.push_back properties ("os_version",
string_of_int x)
>>
>> Looks like a reasonable revert to me.
>>
>> Acked-by: Laszlo Ersek <lersek at redhat.com>
>>
> 
> Hi Laszlo,
> 
> You're reviewing v2, but the latest series which Richard has approved
is v3:
> 
> https://listman.redhat.com/archives/libguestfs/2023-March/031035.html
> https://listman.redhat.com/archives/libguestfs/2023-March/031041.html
> 
> I added you to Cc, but maybe it wasn't delivered?
> 
I had an emergency last Wednesday mid-review and could only resume email
processing this morning. Whenever I resume email processing, I
relatively firmly stick with a FIFO method. In other words, I usually
don't fetch new email until I process all previous, pending email.
Sometimes this causes me to do double work or to create a bit of
confusion. On the other hand, it very safely ensures that no email falls
through the cracks. The LIFO method (process most recent email first)
runs a very high risk of losing old email. "Old" does not translate to
"irrelevant".

So yes, I fully expected this morning that you could have posted a v3
meanwhile.

Now someone could be tempted to suggest, "why don't you fetch email in
the morning, then prioritize everything properly, and *then* work in a
priority queue order". The problem is that prioritzing everything itself
is huge work. After having been out for 2 work days, I've found 330 new
emails this morning.

For me, the only thing that works is delaying the fetching of new email,
until I'm done with the old batch. So, I can be slow, or I can be lossy.
I prefer slow.

Laszlo

Possibly Parallel Threads

Search for more reasonably related threads

Libguestfs - Mar 2023 - [V2V PATCH v2 1/5] Revert "Remove guestcaps_block_type Virtio_SCSI"

[Libguestfs] [V2V PATCH v2 1/5] Revert "Remove guestcaps_block_type Virtio_SCSI"

[Libguestfs] [V2V PATCH v2 1/5] Revert "Remove guestcaps_block_type Virtio_SCSI"

Possibly Parallel Threads