| Message ID | 20220628131032.213986-6-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390-ccw bios fixes and clean-ups | expand |
On Tue, 2022-06-28 at 15:10 +0200, Thomas Huth wrote: > The virtio_assume_scsi() function is very questionable: First, it > is only called for virtio-blk, and not for virtio-scsi, so the naming > is already quite confusing. Second, it is called if we detected a > "invalid" IPL disk, trying to fix it by blindly setting a sector > size of 512. This of course won't work in most cases since disks > might have a different sector size for a reason. > > Thus let's remove this strange function now. The calling code can > also be removed completely, since there is another spot in main.c > that does "IPL_assert(virtio_ipl_disk_is_valid(), ...)" to make > sure that we do not try to IPL from an invalid device. > > Signed-off-by: Thomas Huth <thuth@redhat.com> Agreed. Reviewed-by: Eric Farman <farman@linux.ibm.com> > --- > pc-bios/s390-ccw/virtio.h | 1 - > pc-bios/s390-ccw/virtio-blkdev.c | 24 ------------------------ > 2 files changed, 25 deletions(-) > > diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h > index 8f917d47a9..303438f159 100644 > --- a/pc-bios/s390-ccw/virtio.h > +++ b/pc-bios/s390-ccw/virtio.h > @@ -182,7 +182,6 @@ enum guessed_disk_nature_type { > typedef enum guessed_disk_nature_type VirtioGDN; > > VirtioGDN virtio_guessed_disk_nature(void); > -void virtio_assume_scsi(void); > void virtio_assume_eckd(void); > void virtio_assume_iso9660(void); > > diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390- > ccw/virtio-blkdev.c > index b14cbc3d9e..11820754f3 100644 > --- a/pc-bios/s390-ccw/virtio-blkdev.c > +++ b/pc-bios/s390-ccw/virtio-blkdev.c > @@ -112,23 +112,6 @@ VirtioGDN virtio_guessed_disk_nature(void) > return virtio_get_device()->guessed_disk_nature; > } > > -void virtio_assume_scsi(void) > -{ > - VDev *vdev = virtio_get_device(); > - > - switch (vdev->senseid.cu_model) { > - case VIRTIO_ID_BLOCK: > - vdev->guessed_disk_nature = VIRTIO_GDN_SCSI; > - vdev->config.blk.blk_size = VIRTIO_SCSI_BLOCK_SIZE; > - vdev->config.blk.physical_block_exp = 0; > - vdev->blk_factor = 1; > - break; > - case VIRTIO_ID_SCSI: > - vdev->scsi_block_size = VIRTIO_SCSI_BLOCK_SIZE; > - break; > - } > -} > - > void virtio_assume_iso9660(void) > { > VDev *vdev = virtio_get_device(); > @@ -247,13 +230,6 @@ int virtio_blk_setup_device(SubChannelId schid) > switch (vdev->senseid.cu_model) { > case VIRTIO_ID_BLOCK: > sclp_print("Using virtio-blk.\n"); > - if (!virtio_ipl_disk_is_valid()) { > - /* make sure all getters but blocksize return 0 for > - * invalid IPL disk > - */ > - memset(&vdev->config.blk, 0, sizeof(vdev->config.blk)); > - virtio_assume_scsi(); > - } > break; > case VIRTIO_ID_SCSI: > IPL_assert(vdev->config.scsi.sense_size == > VIRTIO_SCSI_SENSE_SIZE,
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h index 8f917d47a9..303438f159 100644 --- a/pc-bios/s390-ccw/virtio.h +++ b/pc-bios/s390-ccw/virtio.h @@ -182,7 +182,6 @@ enum guessed_disk_nature_type { typedef enum guessed_disk_nature_type VirtioGDN; VirtioGDN virtio_guessed_disk_nature(void); -void virtio_assume_scsi(void); void virtio_assume_eckd(void); void virtio_assume_iso9660(void); diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c index b14cbc3d9e..11820754f3 100644 --- a/pc-bios/s390-ccw/virtio-blkdev.c +++ b/pc-bios/s390-ccw/virtio-blkdev.c @@ -112,23 +112,6 @@ VirtioGDN virtio_guessed_disk_nature(void) return virtio_get_device()->guessed_disk_nature; } -void virtio_assume_scsi(void) -{ - VDev *vdev = virtio_get_device(); - - switch (vdev->senseid.cu_model) { - case VIRTIO_ID_BLOCK: - vdev->guessed_disk_nature = VIRTIO_GDN_SCSI; - vdev->config.blk.blk_size = VIRTIO_SCSI_BLOCK_SIZE; - vdev->config.blk.physical_block_exp = 0; - vdev->blk_factor = 1; - break; - case VIRTIO_ID_SCSI: - vdev->scsi_block_size = VIRTIO_SCSI_BLOCK_SIZE; - break; - } -} - void virtio_assume_iso9660(void) { VDev *vdev = virtio_get_device(); @@ -247,13 +230,6 @@ int virtio_blk_setup_device(SubChannelId schid) switch (vdev->senseid.cu_model) { case VIRTIO_ID_BLOCK: sclp_print("Using virtio-blk.\n"); - if (!virtio_ipl_disk_is_valid()) { - /* make sure all getters but blocksize return 0 for - * invalid IPL disk - */ - memset(&vdev->config.blk, 0, sizeof(vdev->config.blk)); - virtio_assume_scsi(); - } break; case VIRTIO_ID_SCSI: IPL_assert(vdev->config.scsi.sense_size == VIRTIO_SCSI_SENSE_SIZE,
The virtio_assume_scsi() function is very questionable: First, it is only called for virtio-blk, and not for virtio-scsi, so the naming is already quite confusing. Second, it is called if we detected a "invalid" IPL disk, trying to fix it by blindly setting a sector size of 512. This of course won't work in most cases since disks might have a different sector size for a reason. Thus let's remove this strange function now. The calling code can also be removed completely, since there is another spot in main.c that does "IPL_assert(virtio_ipl_disk_is_valid(), ...)" to make sure that we do not try to IPL from an invalid device. Signed-off-by: Thomas Huth <thuth@redhat.com> --- pc-bios/s390-ccw/virtio.h | 1 - pc-bios/s390-ccw/virtio-blkdev.c | 24 ------------------------ 2 files changed, 25 deletions(-)