On Mon, Oct 24, 2016 at 05:21:50PM +0800, Haozhong Zhang wrote:
> For '-object memory-backend-file,mem-path=foo,size=xyz', if the size of
> file 'foo' does not match the given size 'xyz', the current QEMU will
> truncate the file to the given size, which may corrupt the existing data
> in that file. To avoid such data corruption, this patch disables
> truncating non-empty backend files.
>
> Signed-off-by: Haozhong Zhang <haozhong.zh...@intel.com>
> ---
> exec.c | 37 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/exec.c b/exec.c
> index e63c5a1..95983c9 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1188,6 +1188,15 @@ void qemu_mutex_unlock_ramlist(void)
> }
>
> #ifdef __linux__
> +static int64_t get_file_size(int fd)
> +{
> + int64_t size = lseek(fd, 0, SEEK_END);
> + if (size < 0) {
> + return -errno;
> + }
> + return size;
> +}
> +
> static void *file_ram_alloc(RAMBlock *block,
> ram_addr_t memory,
> const char *path,
> @@ -1199,6 +1208,7 @@ static void *file_ram_alloc(RAMBlock *block,
> char *c;
> void *area = MAP_FAILED;
> int fd = -1;
> + int64_t file_size;
>
> if (kvm_enabled() && !kvm_has_sync_mmu()) {
> error_setg(errp,
> @@ -1256,6 +1266,14 @@ static void *file_ram_alloc(RAMBlock *block,
> block->page_size = qemu_fd_getpagesize(fd);
> block->mr->align = MAX(block->page_size, QEMU_VMALLOC_ALIGN);
>
> + file_size = get_file_size(fd);
> + if (file_size < 0) {
> + error_setg_errno(errp, file_size,
> + "can't get size of backing store %s",
> + path);
What about block devices or filesystems where lseek(SEEK_END) is
not supported? They work today, and would break with this patch.
I suggest just continuing without any errors (and not truncating
the file) in case it is not possible to get the file size.
> + goto error;
> + }
> +
> if (memory < block->page_size) {
> error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to "
> "or larger than page size 0x%zx",
> @@ -1266,12 +1284,29 @@ static void *file_ram_alloc(RAMBlock *block,
> memory = ROUND_UP(memory, block->page_size);
>
> /*
> + * Do not extend/shrink the backend file if it's not empty, or its
> + * size does not match the aligned 'size=xxx' option. Otherwise,
> + * it is possible to corrupt the existing data in the file.
> + *
> + * Disabling shrinking is not enough. For example, the current
> + * vNVDIMM implementation stores the guest NVDIMM labels at the
> + * end of the backend file. If the backend file is later extended,
> + * QEMU will not be able to find those labels. Therefore,
> + * extending the non-empty backend file is disabled as well.
> + */
> + if (file_size && file_size != memory) {
> + error_setg(errp, "backing store %s size %"PRId64
> + " does not math with aligned 'size' option %"PRIu64,
Did you mean "specified 'size' option"?
> + path, file_size, memory);
We already support size being smaller than the backing file and
people may rely on it, so we shouldn't change this behavior. This
can be changed to:
if (file_size > 0 && file_size < memory)
I also suggest doing this check in a separate patch. The two
changes (skipping truncation of non-empty files and exiting on
size mismatch) don't depend on each other.
> + goto error;
> + }
> + /*
> * ftruncate is not supported by hugetlbfs in older
> * hosts, so don't bother bailing out on errors.
> * If anything goes wrong with it under other filesystems,
> * mmap will fail.
> */
> - if (ftruncate(fd, memory)) {
> + if (!file_size && ftruncate(fd, memory)) {
> perror("ftruncate");
> }
>
> --
> 2.10.1
>
--
Eduardo
