From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34811) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XkDlM-0001Hj-EW for qemu-devel@nongnu.org; Fri, 31 Oct 2014 11:05:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XkA8r-0006To-Ki for qemu-devel@nongnu.org; Fri, 31 Oct 2014 07:13:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33330) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XkA8r-0006Sr-DU for qemu-devel@nongnu.org; Fri, 31 Oct 2014 07:13:17 -0400 Date: Fri, 31 Oct 2014 11:13:04 +0000 From: Stefan Hajnoczi Message-ID: <20141031111304.GC10332@stefanha-thinkpad.redhat.com> References: <1414510422-8277-1-git-send-email-rehs@gmx.de> <20141029150934.GF19774@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="5QAgd0e35j3NYeGe" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH] Simple performance logging and network limiting based on trace option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: harald Schieche Cc: Stefan Hajnoczi , qemu-devel@nongnu.org, Anthony Liguori , Stefan Weil --5QAgd0e35j3NYeGe Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Oct 30, 2014 at 03:05:11PM +0100, harald Schieche wrote: > > Missing commit description: > >=20 > > What problem are you trying to solve? > >=20 >=20 > I want to log the storage (iops per second) and > network speed (packets and bandwidth per second) QEMU offers the query-blockstats QMP command to poll I/O statistics for block devices. Nowadays a lot of KVM users bypass the QEMU network subsystem and use the vhost-net Linux host kernel module instead. That is the highest-performance and most actively developed networking path. Are you sure you don't want to use vhost-net? > I want to limit the network traffic to a specific bandwidth. You can use the host kernel's firewall or traffic shaping features to do that when using a tap device (most common production configuration). For example, libvirt offers this feature and uses tc under the hood. > > It is simplest to have unconditional trace events and calculate > > latencies during trace file analysis. That way no arbitrary constants > > like 1 second are hard-coded into QEMU. >=20 > We already have an unconditional trace event (paio_submit) but maybe there > are too many calls of it. If you add the BlockDriverState *bs pointer to the paio_submit call, then you can distinguish between drives. However, tracing is not mean as a stable interface for building other features. Trace events can change and are mainly used for interactive or ad-hoc instrumentation. If you build a tool on top of trace events, be prepared to actively maintain it as the set of trace events evolves over time. It's not a stable ABI. > > > diff --git a/net/queue.c b/net/queue.c > > > index f948318..2b0fef7 100644 > > > --- a/net/queue.c > > > +++ b/net/queue.c > > > @@ -23,7 +23,9 @@ > > > =20 > > > #include "net/queue.h" > > > #include "qemu/queue.h" > > > +#include "qemu/timer.h" > > > #include "net/net.h" > > > +#include "trace.h" > > > =20 > > > /* The delivery handler may only return zero if it will call > > > * qemu_net_queue_flush() when it determines that it is once again a= ble > > > @@ -58,6 +60,15 @@ struct NetQueue { > > > unsigned delivering : 1; > > > }; > > > =20 > > > +static int64_t bandwidth_limit; /* maximum number of bits per se= cond */ > >=20 > > Throttling should be per-device, not global. >=20 > Maybe this would be better. But this patch should be most simple. Everything in the network subsystem is per-NetClientState. It doesn't make sense to introduce global state just because it's easier. > > > +static int64_t limit_network_performance(int64_t start_clock, > > > + int64_t bytes) > > > +{ > > > + int64_t clock =3D get_clock(); > > > + int64_t sleep_usecs =3D 0; > > > + if (bandwidth_limit > 0) { > > > + sleep_usecs =3D (bytes * 8 * 1000000LL) / bandwidth_limit - > > > + (clock - start_clock) / 1000LL; > > > + } > > > + if (sleep_usecs > 0) { > > > + usleep(sleep_usecs); > >=20 > > This does more than limit the network performance, it can also freeze > > the guest. > >=20 > > QEMU is event-driven. The event loop thread is not allowed to block or > > sleep - otherwise the vcpu threads will block when they try to acquire > > the QEMU global mutex. > >=20 >=20 > Yes, it freezes the guest. That's not fine, but simple. I won't merge this approach. --5QAgd0e35j3NYeGe Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUU27AAAoJEJykq7OBq3PIdu8H/0RAGIrdvgrVZcKj5Ug3EW5+ gxU3/VcljYKgjjwKejIjbuQDI1X+/XfyTV4zukoNzCoyj2/xBJl7AZSmeTNCEymG HXskiiW6x0snloqQyMoWwp7K5dYbxCNOO8NrCHb/GhqvuuV5cfXXJCKaux8G4SIo jHdfQ/boGB/yuvumdIhHJWDDdK0ttVeobgT/3QrRmAykrD1qDReDYoP/pLo7R94P m29RmLnsrSchGiCCKhKIyTFvCvETs8OaK7+n9x69jXE7I85RT/YgbdEabuWwRUwF Ve6uaUXPGTFTgueMUitxrwunADLFUbHUGV9UgATaDiuxlk6Gm2jNM4gQBdRl37c= =lQd+ -----END PGP SIGNATURE----- --5QAgd0e35j3NYeGe--