From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49980) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgxZr-0006jC-3g for qemu-devel@nongnu.org; Thu, 14 Nov 2013 09:07:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VgxZl-0001ap-4H for qemu-devel@nongnu.org; Thu, 14 Nov 2013 09:07:23 -0500 Received: from mx1.redhat.com ([209.132.183.28]:22550) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VgxZk-0001ah-S7 for qemu-devel@nongnu.org; Thu, 14 Nov 2013 09:07:17 -0500 Message-ID: <5284D90D.6040708@redhat.com> Date: Thu, 14 Nov 2013 15:07:09 +0100 From: Paolo Bonzini 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: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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 Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi , Max Reitz Il 14/11/2013 14:41, Stefan Hajnoczi ha scritto: > Thanks for raising this, it's a bug that we don't verify that the image > has a backing file. > > 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 to > their attention right away. I like this patch because it avoids the risk of NULL-dereferencing bs->backing_hd. See here: /* Copy if allocated in the intermediate images. Limit to the * known-unallocated area [sector_num, sector_num+n). */ ret = bdrv_is_allocated_above(bs->backing_hd, base, sector_num, n, &n); where it's not at all documented that the first argument of bdrv_is_allocated_above can be NULL. But such a no-op streaming is valid, just like it is valid to enable copy-on-read without a backing file. Think of a system that starts streaming a disk as soon as the VM starts, because *if* there's a backing file it knows it is just a template on a remote/slow filesytem. It's easier for such a system to invoke the command even if there's no backing file. Paolo