Re: [PATCH hurd] rumpdisk: do not open device if block size is 0

[Flávio Cruz]

On Thu, Feb 29, 2024 at 7:29 PM Samuel Thibault <samuel.thibault@gnu.org> wrote:

Applied, thanks!

Flavio Cruz, le mer. 28 févr. 2024 22:39:10 -0500, a ecrit:
> The computer seems to get stuck, caused by the divide by 0 in the
> rumpdisk server in device_get_status. I noticed that if we have no disk in the
> cdrom device, we can still open it but block and media size will be 0
> and the message "cd0 dos partition I/O error" will be printed to the
> console. To avoid this problem, we check the block size and throw an error
> when it is 0. This also works correctly when a disk actually exists.
>
> This should help fix the perl and likely the vim test suites that are
> currently failing in https://buildd.debian.org/.

That seems unrelated? perl and vim are not making the whole rumpdisk
server crash, and their failing tests don't seem to be related with
/dev/cd0?

I could be wrong but if you look at this build log https://buildd.debian.org/status/fetch.php?pkg=perl&arch=hurd-i386&ver=5.38.2-3&stamp=1705087520&raw=0 it fails on t/op/stat:

t/op/stat ........................................................ # Failed test 36 - ls and -b agreeing on /dev (0 310) at op/stat.t line 322
#      got "0"
# expected "310"
# = before
# total 1912

I did run the suite myself and it did seem to get stuck at listing /dev when it attempted to open /dev/cd0.

Flavio

> ---
>  rumpdisk/block-rump.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/rumpdisk/block-rump.c b/rumpdisk/block-rump.c
> index a29ebe73..71435f20 100644
> --- a/rumpdisk/block-rump.c
> +++ b/rumpdisk/block-rump.c
> @@ -277,18 +277,25 @@ rumpdisk_device_open (mach_port_t reply_port, mach_msg_type_name_t reply_port_ty
>        return rump_errno2host (errno);
>      }
> 
> -  ret = rump_sys_ioctl (fd, DIOCGMEDIASIZE, &media_size);
> +  ret = rump_sys_ioctl (fd, DIOCGSECTORSIZE, &block_size);
>    if (ret < 0)
>      {
> -      mach_print ("DIOCGMEDIASIZE ioctl fails\n");
> +      mach_print ("DIOCGSECTORSIZE ioctl fails\n");
>        pthread_rwlock_unlock (&rumpdisk_rwlock);
>        return rump_errno2host (errno);
>      }
> 
> -  ret = rump_sys_ioctl (fd, DIOCGSECTORSIZE, &block_size);
> +  if (block_size == 0) {
> +    mach_print ("Unable to get block size\n");
> +    rump_sys_close (fd);
> +    pthread_rwlock_unlock (&rumpdisk_rwlock);
> +    return D_IO_ERROR;
> +  }
> +
> +  ret = rump_sys_ioctl (fd, DIOCGMEDIASIZE, &media_size);
>    if (ret < 0)
>      {
> -      mach_print ("DIOCGSECTORSIZE ioctl fails\n");
> +      mach_print ("DIOCGMEDIASIZE ioctl fails\n");
>        pthread_rwlock_unlock (&rumpdisk_rwlock);
>        return rump_errno2host (errno);
>      }
> @@ -509,8 +516,12 @@ rumpdisk_device_get_status (void *d, dev_flavor_t flavor, dev_status_t status,
>        break;
>      case DEV_GET_RECORDS:
>        status[DEV_GET_RECORDS_RECORD_SIZE] = bd->block_size;
> -      status[DEV_GET_RECORDS_DEVICE_RECORDS] =
> -     bd->media_size / (unsigned long long) bd->block_size;
> +      if (bd->block_size == 0)
> +     status[DEV_GET_RECORDS_DEVICE_RECORDS] = 0;
> +      else {
> +     status[DEV_GET_RECORDS_DEVICE_RECORDS] =
> +       bd->media_size / (unsigned long long) bd->block_size;
> +      }
>        *count = 2;
>        break;
>      default:
> --
> 2.43.0
>
>

--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.