From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57661) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VhIU5-0002lm-Hw for qemu-devel@nongnu.org; Fri, 15 Nov 2013 07:26:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VhIU0-00016J-Ex for qemu-devel@nongnu.org; Fri, 15 Nov 2013 07:26:49 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59326) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VhIU0-000161-6m for qemu-devel@nongnu.org; Fri, 15 Nov 2013 07:26:44 -0500 Date: Fri, 15 Nov 2013 13:26:40 +0100 From: Kevin Wolf Message-ID: <20131115122640.GB3092@dhcp-200-207.str.redhat.com> References: <1384416928-3441-1-git-send-email-cyliu@suse.com> <20131114091726.GA9755@dhcp-200-207.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qemu-img: set nocow flag to new file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chunyan Liu Cc: qemu-devel@nongnu.org, stefanha@redhat.com Am 15.11.2013 um 05:05 hat Chunyan Liu geschrieben: >=20 >=20 >=20 > 2013/11/14 Kevin Wolf >=20 > Am 14.11.2013 um 09:15 hat Chunyan Liu geschrieben: > > Set NOCOW flag to newly created images to solve performance issue= s on > btrfs. > > > > Btrfs has terrible performance when hosting VM images, even more = when the > guest > > in those VM are also using btrfs as file system. One way to mitig= ate this > bad > > performance is to turn off COW attributes on VM files (since havi= ng copy > on > > write for this kind of data is not useful). > > > > Signed-off-by: Chunyan Liu > > --- > > =A0block/raw-posix.c =A0 =A0 | =A0 =A06 ++++++ > > =A0block/vdi.c =A0 =A0 =A0 =A0 =A0 | =A0 =A07 +++++++ > > =A0block/vmdk.c =A0 =A0 =A0 =A0 =A0| =A0 =A07 +++++++ > > =A0include/qemu-common.h | =A0 =A09 +++++++++ > > =A04 files changed, 29 insertions(+), 0 deletions(-) > > > > diff --git a/block/raw-posix.c b/block/raw-posix.c > > index f6d48bb..4a3e9d0 100644 > > --- a/block/raw-posix.c > > +++ b/block/raw-posix.c > > @@ -1072,6 +1072,12 @@ static int raw_create(const char *filename= , > QEMUOptionParameter *options, > > =A0 =A0 =A0 =A0 =A0result =3D -errno; > > =A0 =A0 =A0 =A0 =A0error_setg_errno(errp, -result, "Could not cre= ate file"); > > =A0 =A0 =A0} else { > > +#ifdef __linux__ > > + =A0 =A0 =A0 =A0/* set NOCOW flag to solve performance issue on = fs like btrfs */ > > + =A0 =A0 =A0 =A0int attr; > > + =A0 =A0 =A0 =A0attr =3D FS_NOCOW_FL; > > + =A0 =A0 =A0 =A0ioctl(fd, FS_IOC_SETFLAGS, &attr); > > +#endif >=20 > ioctl() returning an error is ignored. This is probably okay becaus= e > we're only talking about an optimisation here. Perhaps worth a word= or > two in the comment. >=20 > However, while this ioctl is setting FS_NOCOW_FL, it is at the same= time > clearing all other flags. This doesn't look right. >=20 > =A0 > Yes, strictly it should be GETFLAGS and then SETFLAGS. Here because it = does > FS_IOC_SETFLAGS right after creating the file, and checking the qemu_op= en() > parameter, in fact no FLAGS has been set, so just setting FS_NOCOW_FL d= irectly. > I can revise that if it's not good. Is there a guarantee that even just creating the file doesn't set any flags? For example, while reading up on this, I saw that there's a nodatacow mount option, which means exactly that FS_NOCOW_FL gets set for new files. If any similar mechanism existed for other flags (or was to be introduced in future kernel versions), we would break it here. So, I'd prefer to play it safe and use GETFLAGS first. Kevin