On Thu, Jun 20, 2019 at 11:52:01AM +0300, Sam Eiderman wrote:
>
>
> > On 20 Jun 2019, at 8:42, Gerd Hoffmann <kra...@redhat.com> wrote:
> >
> >> +static int
> >> +overriden_lchs_supplied(struct drive_s *drive)
> >> +{
> >> + return drive->lchs.cylinder || drive->lchs.head || drive->lchs.sector;
> >> +}
> >
> >> + case TRANSLATION_MACHINE:
> >
> > Hmm, why this name? Doesn't look intuitive to me.
>
> TRANSLATION_HOST?
>
> >
> >> + desc = "overriden";
> >
> > I'd name that "host-supplied" or "fw-cfg”.
>
> “host-supplied”?
>
> >
> >> + cylinders = drive->lchs.cylinder;
> >> + heads = drive->lchs.head;
> >> + if (heads > 255)
> >> + heads = 255;
> >
> > I suggest to move these sanity checks to overriden_lchs_supplied(), then
> > ignore the override altogether when heads or sectors is out of range
> > instead of trying to fixup things.
>
> Sounds reasonable.
> I’ll rename to host_lchs_supplied()?
>
> WDYT?
looks all good to me.
cheers,
Gerd
