> I think we can fix this by returning error from bdev_get_whole() if
> bdev_disk_changed() failed, this will cause that open disk to fail now,
> however, I think this can be acceptable.
Thanks for the response!
I agree this would fix the regression for the ioctl.
However, since returning an error from blkdev_get_whole is new
behavior, I wasn't sure what all parts it affects.
So this is just a ping to let you know that I am also waiting to hear
from Christoph.
On Mon, Feb 26, 2024 at 7:13 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote:
>
> Hi,
>
> 在 2024/02/27 10:38, Saranya Muruganandam 写道:
> > Hi,
> > Is there advice on how to fix this `blockdev --rereadpt` regression in
> > the kernel?
> > Would appreciate some advice.
> >
> > Thanks,
> > Saranya
> >
> >
> > On Mon, Feb 12, 2024 at 8:01 PM Saranya Muruganandam
> > <saranyamohan@xxxxxxxxxx> wrote:
> >>
> >> When we fail to read from the disk, BLKRRPART used to be able to
> >> capture and bubble this up to the caller.
> >> It no longer does since we no longer capture the error from bdev_disk_changed.
> >>
> >> Here is an example with fault-injection:
> >>
> >> # echo 0 > /sys/kernel/debug/fail_make_request/interval
> >> # echo 100 > /sys/kernel/debug/fail_make_request/probability
> >> # echo -1 > /sys/kernel/debug/fail_make_request/times
> >> # echo 1 > /sys/block/sdc/make-it-fail
> >> # blockdev --rereadpt /dev/sdc # no error
> >> # echo $?
> >> 0 # incorrectly reports success.
>
> I take a look at this, and it's right the root cause is commit
> 4601b4b130de ("block: reopen the device in blkdev_reread_part") ignore
> errors from bdev_disk_changed() now.
>
> I think we can fix this by returning error from bdev_get_whole() if
> bdev_disk_changed() failed, this will cause that open disk to fail now,
> however, I think this can be acceptable.
>
> Christoph, do you think so? Or we should distinguish ioctl and open
> device and only let ioctl to fail.
>
> Thanks,
> Kuai
>
>
> >>
> >> Whereas fdisk and sfdisk correctly report the issue :
> >>
> >> # sfdisk /dev/sdc
> >> sfdisk: cannot open /dev/sdc: Input/output error
> >> # fdisk /dev/sdc
> >>
> >> Welcome to fdisk (util-linux 2.28.2).
> >> Changes will remain in memory only, until you decide to write them.
> >> Be careful before using the write command.
> >>
> >> fdisk: cannot open /dev/sdc: Input/output error
> >>
> >> Best,
> >> Saranya
> >>
> >>
> >>
> >> On Mon, Feb 12, 2024 at 8:00 PM Saranya Muruganandam
> >> <saranyamohan@xxxxxxxxxx> wrote:
> >>>
> >>> When we fail to read from the disk, BLKRRPART used to be able to capture and bubble this up to the caller.
> >>> It no longer does since we no longer capture the error from bdev_disk_changed.
> >>>
> >>> Here is an example with fault-injection:
> >>>
> >>> # echo 0 > /sys/kernel/debug/fail_make_request/interval
> >>> # echo 100 > /sys/kernel/debug/fail_make_request/probability
> >>> # echo -1 > /sys/kernel/debug/fail_make_request/times
> >>> # echo 1 > /sys/block/sdc/make-it-fail
> >>> # blockdev --rereadpt /dev/sdc # no error
> >>> # echo $?
> >>> 0 # incorrectly reports success.
> >>>
> >>> Whereas fdisk and sfdisk correctly report the issue :
> >>>
> >>> # sfdisk /dev/sdc
> >>> sfdisk: cannot open /dev/sdc: Input/output error
> >>> # fdisk /dev/sdc
> >>>
> >>> Welcome to fdisk (util-linux 2.28.2).
> >>> Changes will remain in memory only, until you decide to write them.
> >>> Be careful before using the write command.
> >>>
> >>> fdisk: cannot open /dev/sdc: Input/output error
> >>>
> >>> Best,
> >>> Saranya
> >>>
> >>> On Mon, Feb 12, 2024 at 7:44 AM Christoph Hellwig <hch@xxxxxx> wrote:
> >>>>
> >>>> What scenario are you looking at?
> >
> > .
> >
>
>
