Re: [PATCH V1] hostdisk: Fix linux disk cache workaround on multipath disks

[Daniel Kiper]

On Thu, Sep 13, 2018 at 03:20:34PM +0800, Michael Chang wrote:
> In grub-core/osdep/linux/hostdisk.c::grub_util_fd_open_device() there's 
> comment
> about linux disk cache issue as below:
>
>   /* Linux has a bug that the disk cache for a whole disk is not consistent
>      with the one for a partition of the disk.  */
>   {
>     ....
>   }
>
> As the input argument of grub_util_fd_open_device() is using address in unit 
> of
> sector size offset from the "disk", and in a bid to avoid Linux disk cache
> inconsistency problem described by comment above, grub translates the address
> again into the address offset from partition that has encompassed it, then use
> that partition device in place of disk device.
>
> The problem we encountered was that installing grub into multipath disk's
> partition didn't work reliably. It boiled down to the disk cache problem
> described above as strace result shown it was still using the whole disk
> device, not the partition device we would expect.
>
> This patch fixes the problem by adding the missing "/dev/dm-" name scheme
> handling in grub_hostdisk_linux_find_partition(). After applying the patch
> problem gets solved and we would like to have this fixing patch upstreamed as
> it looks good material to be.
>
> v1: Rework commit message.

Thanks! Right now it looks much better for me.

> Signed-off-by: Michael Chang <address@hidden>
> ---
>  grub-core/osdep/linux/hostdisk.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/grub-core/osdep/linux/hostdisk.c 
> b/grub-core/osdep/linux/hostdisk.c
> index 06179fca7..ed530bdc4 100644
> --- a/grub-core/osdep/linux/hostdisk.c
> +++ b/grub-core/osdep/linux/hostdisk.c
> @@ -263,6 +263,12 @@ grub_hostdisk_linux_find_partition (char *dev, 
> grub_disk_addr_t sector)
>        p = real_dev + len;
>        format = "-part%d";
>      }
> +  else if (strncmp (real_dev, "/dev/dm-",
> +                 sizeof ("/dev/dm-") - 1) == 0)
> +    {
> +      p = real_dev + len - 1;
> +      format = "%d";
> +    }

What will happen if the device path is /dev/dm-10?

>    else if (real_dev[len - 1] >= '0' && real_dev[len - 1] <= '9')
>      {
>        p = real_dev + len;

...and I am afraid that above line is buggy too...
What about the other cases?

Daniel