Re: [Qemu-devel] [PATCH] block: add watermark event

[Stefan Hajnoczi]

On Tue, Aug 05, 2014 at 10:47:57AM +0200, Kevin Wolf wrote:
> Am 01.08.2014 um 13:39 hat Stefan Hajnoczi geschrieben:
> > On Tue, Jul 08, 2014 at 04:49:24PM +0200, Francesco Romani wrote:
> > > @@ -5813,3 +5815,57 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
> > >          bdrv_flush_io_queue(bs->file);
> > >      }
> > >  }
> > > +
> > > +static bool watermark_exceeded(BlockDriverState *bs,
> > > +                               int64_t sector_num,
> > > +                               int nb_sectors)
> > > +{
> > > +
> > > +    if (bs->wr_watermark_perc > 0) {
> > > +        int64_t watermark = (bs->total_sectors) / 100 * 
> > > bs->wr_watermark_perc;
> > 
> > bs->total_sectors should not be used directly.
> > 
> > Have you considered making the watermark parameter take sector units
> > instead of a percentage?
> > 
> > I'm not sure whether a precentage makes sense because 25% of a 10GB
> > image is 2.5 GB so a 75% watermark might be reasonable.  25% of a 1 TB
> > image is 250 GB and that's probably not a reasonable watermark.
> > 
> > So let the block-set-watermark caller pass an absolute sector number
> > instead.  It keeps things simple for both QEMU and thin provisioning
> > manager.
> 
> No sector numbers in external interfaces, please. These units of 512
> bytes are completely arbitrary and don't make any sense. I hope to get
> rid of BDRV_SECTOR_* eventually even internally.
> 
> So for external APIs, please use bytes instead.

I agree and forgot about that.  Please use bytes instead of sectors or a
percentage.

pgpQgMMKOxxdx.pgp
Description: PGP signature