From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47036) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X25Br-0000kS-HG for qemu-devel@nongnu.org; Tue, 01 Jul 2014 17:02:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X25Bm-0004K4-Hl for qemu-devel@nongnu.org; Tue, 01 Jul 2014 17:02:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53902) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X25Bm-0004Jy-95 for qemu-devel@nongnu.org; Tue, 01 Jul 2014 17:02:06 -0400 Message-ID: <53B321CC.7070300@redhat.com> Date: Tue, 01 Jul 2014 15:02:04 -0600 From: Eric Blake MIME-Version: 1.0 References: <1403515022-24802-1-git-send-email-cyliu@suse.com> In-Reply-To: <1403515022-24802-1-git-send-email-cyliu@suse.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gBqwKmJx4IBctRGJ1CJ0TCVKSFmJJK5kQ" Subject: Re: [Qemu-devel] [PATCH V3] qemu-img create: add 'nocow' option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chunyan Liu , qemu-devel@nongnu.org Cc: stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --gBqwKmJx4IBctRGJ1CJ0TCVKSFmJJK5kQ Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 06/23/2014 03:17 AM, Chunyan Liu wrote: > Add 'nocow' option so that users could have a chance to set NOCOW flag = to > newly created files. It's useful on btrfs file system to enhance perfor= mance. >=20 > Btrfs has low performance when hosting VM images, even more when the gu= est > in those VM are also using btrfs as file system. One way to mitigate th= is bad > performance is to turn off COW attributes on VM files. Generally, there= are > two ways to turn off NOCOW on btrfs: a) by mounting fs with nodatacow, = then > all newly created files will be NOCOW. b) per file. Add the NOCOW file > attribute. It could only be done to empty or new files. >=20 > This patch tries the second way, according to the option, it could add = NOCOW > per file. >=20 > For most block drivers, since the create file step is in raw-posix.c, s= o we > can do setting NOCOW flag ioctl in raw-posix.c only. >=20 > But there are some exceptions, like block/vpc.c and block/vdi.c, they a= re > creating file by calling qemu_open directly. For them, do the same sett= ing > NOCOW flag ioctl work in them separately. Design question (not a patch review): It looks like your patch allows one to set the NOCOW flag via ioctl when requested. But how does one learn if the flag is already set? Can you update 'qemu-img info' to show whether a file currently has the flag set? Can 'qemu-img amend' be taught to set and/or clear the flag on an already existing file? > @@ -1291,6 +1296,21 @@ static int raw_create(const char *filename, Qemu= Opts *opts, Error **errp) > result =3D -errno; > error_setg_errno(errp, -result, "Could not create file"); > } else { > + if (nocow) { > +#ifdef __linux__ > + /* Set NOCOW flag to solve performance issue on fs like bt= rfs. > + * This is an optimisation. The FS_IOC_SETFLAGS ioctl retu= rn value > + * will be ignored since any failure of this operation sho= uld not > + * block the left work. > + */ > + int attr; > + if (ioctl(fd, FS_IOC_GETFLAGS, &attr) =3D=3D 0) { > + attr |=3D FS_NOCOW_FL; > + ioctl(fd, FS_IOC_SETFLAGS, &attr); > + } > +#endif > + } This silently ignores the nocow flag on non-Linux. Wouldn't it be better to reject the option as unsupported? What happens if the ioctl fails? Would it be better to fail the qemu-img creation if the flag is requested but can't be honored? > +++ b/qemu-doc.texi > @@ -589,6 +589,22 @@ check -r all} is required, which may take some tim= e. > =20 > This option can only be enabled if @code{compat=3D1.1} is specified. > =20 > +@item nocow > +If this option is set to @code{on}, it will trun off COW of the file. = It's only s/trun/turn/ > +valid on btrfs, no effect on other file systems. This sort of statement may get stale, if other file systems learn to honor the same ioctl as btrfs. > + > +Btrfs has low performance when hosting a VM image file, even more when= the guest > +on the VM also using btrfs as file system. Turning off COW is a way to= mitigate > +this bad performance. Generally there are two ways to turn off COW on = btrfs: > +a) Disable it by mounting with nodatacow, then all newly created files= will be > +NOCOW. b) For an empty file, add the NOCOW file attribute. That's what= this option > +does. > + > +Note: this option is only valid to new or empty files. If there is an = existing > +file which is COW and has data blocks already, it couldn't be changed = to NOCOW > +by setting @code{nocow=3Don}. One can issue @code{lsattr filename} to = check if > +the NOCOW flag is set or not (Capitabl 'C' is NOCOW flag). s/Capitabl/Capital/ Oh, so it looks like setting the attribute is one-way, and can't be undone once something is written? Or is it that it can only be set on an empty file, but can be cleared at any time? Again, making people refer to lsattr to learn if the flag is already set seems painful; can qemu-img info be taught to expose this information, so that one tool is sufficient to manage the entire experience? > +++ b/qemu-img.texi > @@ -474,6 +474,22 @@ check -r all} is required, which may take some tim= e. > =20 > This option can only be enabled if @code{compat=3D1.1} is specified. > =20 > +@item nocow > +If this option is set to @code{on}, it will trun off COW of the file. = It's only s/trun/turn/ > +valid on btrfs, no effect on other file systems. > + > +Btrfs has low performance when hosting a VM image file, even more when= the guest > +on the VM also using btrfs as file system. Turning off COW is a way to= mitigate > +this bad performance. Generally there are two ways to turn off COW on = btrfs: > +a) Disable it by mounting with nodatacow, then all newly created files= will be > +NOCOW. b) For an empty file, add the NOCOW file attribute. That's what= this option > +does. > + > +Note: this option is only valid to new or empty files. If there is an = existing > +file which is COW and has data blocks already, it couldn't be changed = to NOCOW > +by setting @code{nocow=3Don}. One can issue @code{lsattr filename} to = check if > +the NOCOW flag is set or not (Capitabl 'C' is NOCOW flag). s/Capitabl/Capital/ --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --gBqwKmJx4IBctRGJ1CJ0TCVKSFmJJK5kQ 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 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTsyHMAAoJEKeha0olJ0NqpCMIAK4T8zh7PkU6iSy92mhknKY/ oFa8WCgnkPkX5OwATuGpHsFU0rxHyIZlj+0Vnvn1btqWNy8fq1skQ2N3QI/wHQmQ wuP6DVaJ/QFO8z86mD5D9OhKJBDLGZQTZ6gpMnpG/IHMaxG4TueUEbY9qrUz7+3N 7I1yLjqv4I4ImjhJ8k1NJBXpZKsE2xEo/iDZDNmytFZtBir5C/OpE/cTwukBiWTY OXOzStrDZqyYPMn1RTuG/a5GYiCrhZI7Lxa0DEG4mVCPmezCz+2uyJvlvESiMusG yL7Rc1v4ghSKuyP7jwqVsRngOwB4j/VfojWh6CaBr6yLVEPUM1IhZqZ1HVy1A84= =xEwv -----END PGP SIGNATURE----- --gBqwKmJx4IBctRGJ1CJ0TCVKSFmJJK5kQ--