From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48344) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgxTm-00032R-CK for qemu-devel@nongnu.org; Thu, 14 Nov 2013 09:01:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VgxTh-0007tt-By for qemu-devel@nongnu.org; Thu, 14 Nov 2013 09:01:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58658) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgxTh-0007tp-41 for qemu-devel@nongnu.org; Thu, 14 Nov 2013 09:01:01 -0500 Message-ID: <5284D796.1000008@redhat.com> Date: Thu, 14 Nov 2013 07:00:54 -0700 From: Eric Blake MIME-Version: 1.0 References: <1384371478-20021-1-git-send-email-mreitz@redhat.com> <20131114134122.GC26847@stefanha-thinkpad.redhat.com> In-Reply-To: <20131114134122.GC26847@stefanha-thinkpad.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="9HAA2vuGI1dDQlINREFjhScEOHHOLdnCE" Subject: Re: [Qemu-devel] [PATCH] block/stream: Don't stream unbacked devices List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , Max Reitz Cc: Kevin Wolf , Paolo Bonzini , qemu-devel@nongnu.org, Stefan Hajnoczi This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --9HAA2vuGI1dDQlINREFjhScEOHHOLdnCE Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 11/14/2013 06:41 AM, Stefan Hajnoczi wrote: > On Wed, Nov 13, 2013 at 08:37:58PM +0100, Max Reitz wrote: >> If a block device is unbacked, a streaming blockjob should immediately= >> finish instead of beginning to try to stream, then noticing the backin= g >> file does not contain even the first sector (since it does not exist) >> and then finishing normally. >> >> Signed-off-by: Max Reitz >> --- >> block/stream.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/block/stream.c b/block/stream.c >> index 694fd42..46bec7d 100644 >> --- a/block/stream.c >> +++ b/block/stream.c >> @@ -88,6 +88,11 @@ static void coroutine_fn stream_run(void *opaque) >> int n =3D 0; >> void *buf; >> =20 >> + if (!bs->backing_hd) { >> + block_job_completed(&s->common, 0); >> + return; >> + } >> + >=20 > Thanks for raising this, it's a bug that we don't verify that the image= > has a backing file. >=20 > I'd rather return an error that the user attempted to do something > pointless. It was a mistake on their part and it helps to bring this t= o > their attention right away. Conversely, we can argue that streaming a file that has no backing should SUCCEED as a no-op (there's nothing further to stream, so we are done right away). >=20 > CCing Eric Blake because strictly speaking, adding a error where we > previously used to return success breaks API compatibility. Any > suggestions (I guess QEMU could just log a warning instead of failing > the command)? Turning this into an error would be a regression, and should not be done. Based on previous behavior, we have to keep it successful. But if I'm reading the patch correctly, it appears that all you are trying to do is optimize the no-op case to be even more of a no-op (faster success return, rather than setting up a lot of structure just to find no further work to do), and not turn a previous success case into an error. Adding a log message about a suspicious may make sense, but if there is no error, will anyone think to check the log? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --9HAA2vuGI1dDQlINREFjhScEOHHOLdnCE 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.4.15 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJShNeWAAoJEKeha0olJ0NqagwH/jys9oTFGn5GfOKcryi/tOrs IqlYVnRW9SaGtGCn80VMElcgP6XQvAeQdLK/IIRIDApUqSIpAcr82T2G+QY5m/p6 0Z4ucSireGJNPDYrmzSqd2AAen0Ecj/vyk84hATn2hPGFPCFi4PQz8imo+7sqvyI fN2jnbFlNNCDJQclKJ/HIEwydqLoKDtleUOuZWzkNSVtcVJAFEr6jPZF9ijy1MHq Gj98lKvbaoGxesdPUGywciLkuwdTBoSHppKYtsVK9Y3Wr5AbxmTCa6d+FVbTwrGt R77Dn0++53QhUz3jMcH9LYQJ5X2P9BzS5JtCp248mG3dSyuspWz42nG2KL6/EmM= =oxat -----END PGP SIGNATURE----- --9HAA2vuGI1dDQlINREFjhScEOHHOLdnCE--