From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37412) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4X2y-0000Ml-3W for qemu-devel@nongnu.org; Tue, 08 Jul 2014 11:11:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X4X2s-0000wk-07 for qemu-devel@nongnu.org; Tue, 08 Jul 2014 11:11:08 -0400 Received: from qmta10.emeryville.ca.mail.comcast.net ([76.96.30.17]:42926) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4X2r-0000vc-Lj for qemu-devel@nongnu.org; Tue, 08 Jul 2014 11:11:01 -0400 Message-ID: <53BC0A01.8020609@redhat.com> Date: Tue, 08 Jul 2014 09:10:57 -0600 From: Eric Blake MIME-Version: 1.0 References: <1404830964-10733-1-git-send-email-fromani@redhat.com> <1404830964-10733-2-git-send-email-fromani@redhat.com> In-Reply-To: <1404830964-10733-2-git-send-email-fromani@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="GlmoQpnLSNlowKiU7hI3O23FpNEpeSthP" Subject: Re: [Qemu-devel] [PATCH] block: add watermark event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Francesco Romani , qemu-devel@nongnu.org Cc: kwolf@redhat.com, mdroth@linux.vnet.ibm.com, stefanha@redhat.com, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --GlmoQpnLSNlowKiU7hI3O23FpNEpeSthP Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 07/08/2014 08:49 AM, Francesco Romani wrote: > Managing applications, like oVirt (http://www.ovirt.org), make extensiv= e > use of thin-provisioned disk images. > In order to let the guest run flawlessly and be not unnecessarily > paused, oVirt sets a watermark based on the percentage occupation of th= e > device against the advertised size, and automatically resizes the image= > once the watermark is reached or exceeded. >=20 > In order to detect the mark crossing, the managing application has no > choice than aggressively polling the QEMU monitor using the > query-blockstats command. This lead to unnecessary system > load, and is made even worse under scale: scenarios > with hundreds of VM are becoming not unusual. >=20 > To fix this, this patch adds: > * A new monitor command to set a mark for a given block device. > * A new event to report if a block device usage exceeds the threshold. >=20 > This will allow the managing application to drop the polling > alltogether and just wait for a watermark crossing event. s/alltogether/altogether/ >=20 > Signed-off-by: Francesco Romani > --- > block.c | 56 +++++++++++++++++++++++++++++++++++++++= ++++++++ > blockdev.c | 21 ++++++++++++++++++ > include/block/block.h | 2 ++ > include/block/block_int.h | 3 +++ > qapi/block-core.json | 33 ++++++++++++++++++++++++++++ > qmp-commands.hx | 24 ++++++++++++++++++++ > 6 files changed, 139 insertions(+) >=20 > +void bdrv_set_watermark_perc(BlockDriverState *bs, > + int watermark_perc) > +{ > + NotifierWithReturn before_write =3D { > + .notify =3D watermark_before_write_notify, > + }; > + > + if (watermark_perc <=3D 0) { > + return; > + } Shouldn't you uninstall the write_notifier if someone is changing the watermark_perc from non-zero back to zero? > +++ b/include/block/block_int.h > @@ -393,6 +393,9 @@ struct BlockDriverState { > =20 > /* The error object in use for blocking operations on backing_hd *= / > Error *backing_blocker; > + > + /* watermark limit for writes, percentage of sectors */ > + int wr_watermark_perc; I didn't see any code that ensures that this limit is between 0 and 100; since it is under user control, your code probably misbehaves for values such as 101. > + > +## > +# @BLOCK_WATERMARK > +# > +# Emitted when a block device reaches or exceeds the watermark limit. > +# > +# @device: device name > +# > +# @sector-num: number of the sector exceeding the threshold > +# > +# @sector-highest: number of the last highest written sector > +# > +# Since: 2.1 2.2. You've missed hard freeze for 2.1. > +## > +{ 'event': 'BLOCK_WATERMARK', > + 'data': { 'device': 'str', 'sector-num': 'int', 'sector-highest': 'i= nt' } } > + > +## > +# @block_set_watermark s/_/-/2 - new commands should use - in their name. > +# > +# Change watermark percentage for a block drive. > +# > +# @device: The name of the device > +# > +# @watermark: high water mark, percentage Is percentage the right unit? On a 10G disk, that means you only have a granularity down to 100M. Should the watermark limit instead be expressed as a byte value instead of a percentage? > +# > +# Returns: Nothing on success > +# If @device is not a valid block device, DeviceNotFound > +# > +# Since: 2.1 2.2. > +## > +{ 'command': 'block_set_watermark', > + 'data': { 'device': 'str', 'watermark': 'int' } } I hate write-only commands. Which query-* command are you modifying to output the current watermark level? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --GlmoQpnLSNlowKiU7hI3O23FpNEpeSthP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTvAoBAAoJEKeha0olJ0Nq/WwIAKsguzLp2Rn3SJglwe///IN9 xi9yrlC7s/b1hgk7jae8im1hOVEOLloLExSQhBy2rCa2rRsWHF8+BGZd/v649z50 VE8d6x2v/RQoG5Q+jn/2H+1KqJzRuoVuO0tR9kIaa8wxOugNPASJX79ifOM3kPjD ge6xYIJnFpOjg3r6eNyYt2PuJLN/vP+wQJzHqxA+RJ3uZJtAIZMnrZYaSwMQnbAN FiewFYK738FP+iv5BNxpp8dKjnQbzZjYw60tC02U97nZhNLUFHzGdKP/O3og/ENZ hrI1mQZ6ELejL47fN4aYZnnsTavZnc3nMNVOZgSbH/3buPE+ZscpB9scpBAYgtY= =NuC/ -----END PGP SIGNATURE----- --GlmoQpnLSNlowKiU7hI3O23FpNEpeSthP--