Re: [Qemu-devel] [PATCH] qemu-img: set nocow flag to new file

[Kevin Wolf]

Am 15.11.2013 um 05:05 hat Chunyan Liu geschrieben:
> 
> 
> 
> 2013/11/14 Kevin Wolf <address@hidden>
> 
>     Am 14.11.2013 um 09:15 hat Chunyan Liu geschrieben:
>     > Set NOCOW flag to newly created images to solve performance issues 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 mitigate 
> this
>     bad
>     > performance is to turn off COW attributes on VM files (since having copy
>     on
>     > write for this kind of data is not useful).
>     >
>     > Signed-off-by: Chunyan Liu <address@hidden>
>     > ---
>     >  block/raw-posix.c     |    6 ++++++
>     >  block/vdi.c           |    7 +++++++
>     >  block/vmdk.c          |    7 +++++++
>     >  include/qemu-common.h |    9 +++++++++
>     >  4 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,
>     >          result = -errno;
>     >          error_setg_errno(errp, -result, "Could not create file");
>     >      } else {
>     > +#ifdef __linux__
>     > +        /* set NOCOW flag to solve performance issue on fs like btrfs 
> */
>     > +        int attr;
>     > +        attr = FS_NOCOW_FL;
>     > +        ioctl(fd, FS_IOC_SETFLAGS, &attr);
>     > +#endif
> 
>     ioctl() returning an error is ignored. This is probably okay because
>     we're only talking about an optimisation here. Perhaps worth a word or
>     two in the comment.
> 
>     However, while this ioctl is setting FS_NOCOW_FL, it is at the same time
>     clearing all other flags. This doesn't look right.
> 
>  
> 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_open()
> parameter, in fact no FLAGS has been set, so just setting FS_NOCOW_FL 
> directly.
> 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