On 3/13/23 10:40, Laszlo Ersek wrote:
> 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 <ler...@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
>
Thanks for detailed explanation, using FIFO makes perfect sense.
And of course there's no rush whatsoever -- I just thought that maybe
you've missed it.
Andrey
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
