From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uzsaz-0003h7-NQ for qemu-devel@nongnu.org; Thu, 18 Jul 2013 14:06:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Uzsav-000480-H0 for qemu-devel@nongnu.org; Thu, 18 Jul 2013 14:06:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12381) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Uzsav-00047w-87 for qemu-devel@nongnu.org; Thu, 18 Jul 2013 14:06:25 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r6II6OG6008548 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 18 Jul 2013 14:06:24 -0400 Date: Thu, 18 Jul 2013 11:06:22 -0700 From: Ian Main Message-ID: <20130718180621.GB13675@gate.mains.priv> References: <1374091462-18391-1-git-send-email-imain@redhat.com> <1374091462-18391-5-git-send-email-imain@redhat.com> <51E82579.20201@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51E82579.20201@redhat.com> Subject: Re: [Qemu-devel] [PATCH V4 4/4] Change default to qcow2 for sync mode none. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org On Thu, Jul 18, 2013 at 11:27:21AM -0600, Eric Blake wrote: > On 07/17/2013 02:04 PM, Ian Main wrote: > > qcow2 supports backing files so it makes sense to default to qcow2 > > for MIRROR_SYNC_MODE_NONE so that we can use the source as a backing > > drive and export it via nbd. Defaulting FULL and TOP to SYNC_MODE_NONE > > breaks tests but that could be fixed if we wanted it. > > > > Signed-off-by: Ian Main > > --- > > blockdev.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/blockdev.c b/blockdev.c > > index 000dea6..805b0e5 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -1462,7 +1462,7 @@ void qmp_drive_backup(const char *device, const char *target, > > } > > > > if (!has_format) { > > - format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name; > > + format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : "qcow2"; > > Is this the right thing to do? Or should we do: > > if (!has_format) { > if (mode == NEW_IMAGE_MODE_EXISTING) { > format = NULL; > } else { > format = bs->drv->format_name ?: "qcow2"; > } > } > > That is, I think we should default to doing a backup in the format given > by the original (what if the original is qed, which also supports > backing files), and only use qcow2 when there is no guidance whatsoever. > > But in practice, I don't care - format probing is a security hole, so > libvirt should always be passing a format, at which point the entire > !has_format condition should be skipped when called by libvirt. Heh, actually that is basically what I have now, as with the doc change I forgot to git add it. Doh! I'll repost.. I'm also not opposed to format being non-optional. Ian