Re: [Qemu-devel] [PATCH 00/10] pc-bios/s390-ccw: Add DASD IPL support

[Cornelia Huck]

On Fri, 27 Jun 2014 11:27:12 +0200
Christian Borntraeger <address@hidden> wrote:

> On 27/06/14 11:05, Alexander Graf wrote:
> > 
> > 
> >> Am 27.06.2014 um 09:53 schrieb Christian Borntraeger <address@hidden>:
> >>
> >>> On 26/06/14 16:42, Alexander Graf wrote:
> >>>
> >>>> On 26.06.14 16:29, Jens Freimann wrote:
> >>>> Conny, Alex, Christian,
> >>>>
> >>>> here are some fixes for the s390-ccw bios. It's a mixture of
> >>>> additional features (DASD IPL support for different formats)
> >>>> and cleanups.
> >>>
> >>> From a quick glimpse it looks quite clean and straight forward, but I'd 
> >>> like to make sure we get rid completely of the static sector size 
> >>> assumption.
> >>
> >> Should be. I guess s/SECTOR_SIZE/MAX_SECTOR_SIZE/g would be ok for you 
> >> then?
> > 
> > I'm not 100% convinced that we're safe on all users of SECTOR_SIZE. So 
> > please make sure to replace the occasions manually and audit every single 
> > one.
> 
> Yes, a mindless sed, would also replace VIRTIO_SECTOR_SIZE with 
> VIRTIO_MAX_SECTOR_SIZE.
> Fortunately there are only 3 place in bootmap.c. Should be simple enough to 
> review.

Yes, all places that use it want a MAX_SECTOR_SIZE. All places using
the actual sector size are now using the helper function.

> >>>
> >>> Also, are we guaranteed that virtio always uses 512 byte block size? Or 
> >>> was that just an internal API thing?
> >>
> >> The virtio-blk API always talks in 512 byte sectors, no matter the block 
> >> size.
> >>
> >> Overall this is a nice improvement of the boot code - if possible I would 
> >> like to see that in 2.1.
> >>
> >> Conny, can you carry that in your tree (with 
> >> s/SECTOR_SIZE/MAX_SECTOR_SIZE/g)?
> >>
> >> Acked-by: Christian Borntraeger <address@hidden>
> >>
> >> for the series.

Will push out shortly.

Unless there are objections, I'll send a pull request for this.