Re: [Qemu-devel] [PATCH] hostmem-file: add a property 'notrunc' to avoid data corruption

[Igor Mammedov]

On Thu, 20 Oct 2016 14:13:01 +0800
Haozhong Zhang <address@hidden> wrote:

> If a file is used as the backend of memory-backend-file and its size is
> not identical to the property 'size', the file will be truncated. For a
> file used as the backend of vNVDIMM, its data is expected to be
> persistent and the truncation may corrupt the existing data.
I wonder if it's possible just skip 'size' property in your case instead
'notrunc' property. That way if size is not present one'd get actual size
using get_file_size() and set 'size' to it?
And if 'size' is provided and 'size' != file_size then error out.

> 
> A property 'notrunc' is added to memory-backend-file to allow users to
> control the truncation. If 'notrunc' is on, QEMU will not truncate the
> backend file and require the property 'size' is not larger than the file
> size. If 'notrunc' is off, QEMU will behave as before.
[...]

>  
>  #ifdef __linux__
> +static uint64_t get_file_size(const char *path, Error **errp)
Maybe QEMU laredy has an utility to do it that could be shared,
CCing block maintainers.

> +{
> +    int fd;
> +    struct stat st;
> +    uint64_t size = 0;
> +    Error *local_err = NULL;
> +
> +    fd = qemu_open(path, O_RDONLY);
> +    if (fd < 0) {
> +        error_setg(&local_err, "cannot open file");
error_setg_errno
> +        goto out;
> +    }
> +
> +    if (stat(path, &st)) {
fstat()

> +        error_setg(&local_err, "cannot get file stat");
error_setg_errno
> +        goto out_fclose;
> +    }
> +
> +    switch (st.st_mode & S_IFMT) {
> +    case S_IFREG:
> +        size = st.st_size;
> +        break;
> +
> +    case S_IFBLK:
> +        if (ioctl(fd, BLKGETSIZE64, &size)) {
compilation might fail on platforms without  BLKGETSIZE64,


> +            error_setg(&local_err, "cannot get size of block device");
error_setg_errno
> +            size = 0;
> +        }
> +        break;
> +
> +    default:
> +        error_setg(&local_err,
> +                   "only block device on Linux and regular file are 
> supported");
what about huge page usecase, would it fall right here fail?

> +    }
> +
> + out_fclose:
> +    close(fd);
> + out:
> +    error_propagate(errp, local_err);
> +    return size;
> +}
> +
[...]