Re: [PATCH] hw/ufs: Modify lu.c to share codes with SCSI subsystem

[Philippe Mathieu-Daudé]

Hi Jeuk,

On 20/10/23 03:51, Jeuk Kim wrote:
> This patch removes the code that ufs-lu was duplicating from
> scsi-hd and allows them to share code.
>
> It makes ufs-lu have a virtual scsi-bus and scsi-hd internally.
> This allows scsi related commands to be passed thorugh to the scsi-hd.
> The query request and nop command work the same as the existing logic.
>
> Well-known lus do not have a virtual scsi-bus and scsi-hd, and
> handle the necessary scsi commands by emulating them directly.
>
> Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>
> ---
>   hw/ufs/lu.c            | 1473 +++++++---------------------------------

I liked this patch intent, but almost 1500 lines changed in a single
patch make it impossible to review. Ideally each patch shouldn't modify
more than 100 lines, otherwise reviewers are either exhausted or can't
be careful enough and miss possible bugs or design issues.

Regards,

Phil.

>   hw/ufs/trace-events    |   25 -
>   hw/ufs/ufs.c           |  202 +-----
>   hw/ufs/ufs.h           |   36 +-
>   include/block/ufs.h    |    2 +-
>   tests/qtest/ufs-test.c |   37 +-
>   6 files changed, 315 insertions(+), 1460 deletions(-)