From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52909)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1V9Xj3-0006w0-KV
	for qemu-devel@nongnu.org; Wed, 14 Aug 2013 05:50:51 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1V9Xiw-0001Y4-Ix
	for qemu-devel@nongnu.org; Wed, 14 Aug 2013 05:50:45 -0400
Received: from mx1.redhat.com ([209.132.183.28]:53802)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <famz@redhat.com>) id 1V9Xiv-0001Xp-Nc
	for qemu-devel@nongnu.org; Wed, 14 Aug 2013 05:50:37 -0400
Date: Wed, 14 Aug 2013 17:50:30 +0800
From: Fam Zheng <famz@redhat.com>
Message-ID: <20130814095030.GE11081@localhost.localdomain>
References: <1376326396-7676-1-git-send-email-benoit@irqsave.net>
	<1376326396-7676-4-git-send-email-benoit@irqsave.net>
	<20130814093125.GC11081@localhost.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
In-Reply-To: <20130814093125.GC11081@localhost.localdomain>
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH V5 3/5] block: Enable the new throttling
 code in the block layer.
Reply-To: famz@redhat.com
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: =?iso-8859-1?Q?Beno=EEt?= Canet <benoit@irqsave.net>
Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com

On Wed, 08/14 17:31, Fam Zheng wrote:
> On Mon, 08/12 18:53, Beno=EEt Canet wrote:
> > Signed-off-by: Benoit Canet <benoit@irqsave.net>
> > @@ -1262,7 +1260,7 @@ void qmp_block_set_io_throttle(const char *devi=
ce, int64_t bps, int64_t bps_rd,
> >                                 int64_t bps_wr, int64_t iops, int64_t=
 iops_rd,
> >                                 int64_t iops_wr, Error **errp)
> >  {
> > -    BlockIOLimit io_limits;
> > +    ThrottleConfig cfg;
> >      BlockDriverState *bs;
> > =20
> >      bs =3D bdrv_find(device);
> > @@ -1271,27 +1269,37 @@ void qmp_block_set_io_throttle(const char *de=
vice, int64_t bps, int64_t bps_rd,
> >          return;
> >      }
> > =20
> > -    io_limits.bps[BLOCK_IO_LIMIT_TOTAL] =3D bps;
> > -    io_limits.bps[BLOCK_IO_LIMIT_READ]  =3D bps_rd;
> > -    io_limits.bps[BLOCK_IO_LIMIT_WRITE] =3D bps_wr;
> > -    io_limits.iops[BLOCK_IO_LIMIT_TOTAL]=3D iops;
> > -    io_limits.iops[BLOCK_IO_LIMIT_READ] =3D iops_rd;
> > -    io_limits.iops[BLOCK_IO_LIMIT_WRITE]=3D iops_wr;
> > +    cfg.buckets[THROTTLE_BPS_TOTAL].ups =3D bps;
> > +    cfg.buckets[THROTTLE_BPS_READ].ups  =3D bps_rd;
> > +    cfg.buckets[THROTTLE_BPS_WRITE].ups =3D bps_wr;
> > +
> > +    cfg.buckets[THROTTLE_OPS_TOTAL].ups =3D iops;
> > +    cfg.buckets[THROTTLE_OPS_READ].ups  =3D iops_rd;
> > +    cfg.buckets[THROTTLE_OPS_WRITE].ups =3D iops_wr;
> > +
> > +    cfg.buckets[THROTTLE_BPS_TOTAL].max =3D 0;
> > +    cfg.buckets[THROTTLE_BPS_READ].max  =3D 0;
> > +    cfg.buckets[THROTTLE_BPS_WRITE].max =3D 0;
> > +
> > +    cfg.buckets[THROTTLE_OPS_TOTAL].max =3D 0;
> > +    cfg.buckets[THROTTLE_OPS_READ].max  =3D 0;
> > +    cfg.buckets[THROTTLE_OPS_WRITE].max =3D 0;
> > =20
> > -    if (!do_check_io_limits(&io_limits, errp)) {
> > +    cfg.unit_size =3D BDRV_SECTOR_SIZE;
>=20
> Does this mean user set bps limit in sector now? I think we should be
> consistent to existing parameter unit.
>=20
> > +    cfg.op_size =3D 0;
>=20
> Why not set op_size to 1?
>=20
Never mind. You se it to 0 here and it's the condition of setting
unit=3D1.

Fam