diff --git a/block.c b/block.c index 66a35b3982..c8374dac4f 100644 --- a/block.c +++ b/block.c @@ -861,35 +861,42 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo) /* * Create a uniquely-named empty temporary file. - * Return 0 upon success, otherwise a negative errno value. + * Return the actual file name used upon success, otherwise NULL. + * This string should be freed with g_free() when not needed any longer. + * + * Note: creating a temporary file for the caller to (re)open is + * inherently racy. Use g_file_open_tmp() instead whenever practical. */ -int get_tmp_filename(char *filename, int size) +char *create_tmp_file(Error **errp) { -#ifdef _WIN32 - char temp_dir[MAX_PATH]; - /* GetTempFileName requires that its output buffer (4th param) - have length MAX_PATH or greater. */ - assert(size >= MAX_PATH); - return (GetTempPath(MAX_PATH, temp_dir) - && GetTempFileName(temp_dir, "qem", 0, filename) - ? 0 : -GetLastError()); -#else int fd; const char *tmpdir; - tmpdir = getenv("TMPDIR"); - if (!tmpdir) { + g_autofree char *filename = NULL; + + tmpdir = g_get_tmp_dir(); +#ifndef _WIN32 + /* + * See commit 69bef79 ("block: use /var/tmp instead of /tmp for -snapshot") + * + * This function is used to create temporary disk images (like -snapshot), + * so the files can become very large. /tmp is often a tmpfs where as + * /var/tmp is usually on a disk, so more appropriate for disk images. + */ + if (!g_strcmp0(tmpdir, "/tmp")) { tmpdir = "/var/tmp"; } - if (snprintf(filename, size, "%s/vl.XXXXXX", tmpdir) >= size) { - return -EOVERFLOW; - } - fd = mkstemp(filename); +#endif + + filename = g_strdup_printf("%s/vl.XXXXXX", tmpdir); + fd = g_mkstemp(filename); if (fd < 0) { - return -errno; + error_setg_errno(errp, errno, "Could not open temporary file '%s'", + filename); + return NULL; } close(fd); - return 0; -#endif + + return g_steal_pointer(&filename); } /* @@ -3715,8 +3722,7 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs, QDict *snapshot_options, Error **errp) { - /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */ - char *tmp_filename = g_malloc0(PATH_MAX + 1); + g_autofree char *tmp_filename = NULL; int64_t total_size; QemuOpts *opts = NULL; BlockDriverState *bs_snapshot = NULL; @@ -3735,9 +3741,8 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs, } /* Create the temporary image */ - ret = get_tmp_filename(tmp_filename, PATH_MAX + 1); - if (ret < 0) { - error_setg_errno(errp, -ret, "Could not get temporary filename"); + tmp_filename = create_tmp_file(errp); + if (!tmp_filename) { goto out; } @@ -3771,7 +3776,6 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs, out: qobject_unref(snapshot_options); - g_free(tmp_filename); return bs_snapshot; } diff --git a/block/vvfat.c b/block/vvfat.c index d6dd919683..f9bf8406d3 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -3146,10 +3146,9 @@ static int enable_write_target(BlockDriverState *bs, Error **errp) array_init(&(s->commits), sizeof(commit_t)); - s->qcow_filename = g_malloc(PATH_MAX); - ret = get_tmp_filename(s->qcow_filename, PATH_MAX); - if (ret < 0) { - error_setg_errno(errp, -ret, "can't create temporary file"); + s->qcow_filename = create_tmp_file(errp); + if (!s->qcow_filename) { + ret = -ENOENT; goto err; } diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 8947abab76..d7c0a7e96f 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -1230,7 +1230,7 @@ static inline BlockDriverState *child_bs(BdrvChild *child) } int bdrv_check_request(int64_t offset, int64_t bytes, Error **errp); -int get_tmp_filename(char *filename, int size); +char *create_tmp_file(Error **errp); void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, QDict *options); diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index f1a506518b..4c079b11e3 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -375,7 +375,8 @@ if [ "${VALGRIND_QEMU_VM}" == "y" ]; then _casenotrun "Valgrind needs a valid TMPDIR for itself" fi VALGRIND_QEMU_VM= \ -TMPDIR=/nonexistent run_qemu -drive driver=null-co,snapshot=on +TMPDIR=/nonexistent run_qemu -drive driver=null-co,snapshot=on | + sed -e "s#'[^']*/vl\.[A-Za-z0-9]\{6\}'#SNAPSHOT_PATH#g" # Using snapshot=on together with read-only=on echo "info block" | diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out index 441f83e41a..e5ddb03bda 100644 --- a/tests/qemu-iotests/051.out +++ b/tests/qemu-iotests/051.out @@ -459,7 +459,7 @@ wrote 4096/4096 bytes at offset 0 read 4096/4096 bytes at offset 0 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) Testing: -drive driver=null-co,snapshot=on -QEMU_PROG: -drive driver=null-co,snapshot=on: Could not get temporary filename: No such file or directory +QEMU_PROG: -drive driver=null-co,snapshot=on: Could not open temporary file SNAPSHOT_PATH: No such file or directory Testing: -drive file=TEST_DIR/t.qcow2,snapshot=on,read-only=on,if=none,id=drive0 QEMU X.Y.Z monitor - type 'help' for more information diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out index 063e4fc584..bade1ff3b9 100644 --- a/tests/qemu-iotests/051.pc.out +++ b/tests/qemu-iotests/051.pc.out @@ -539,7 +539,7 @@ wrote 4096/4096 bytes at offset 0 read 4096/4096 bytes at offset 0 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) Testing: -drive driver=null-co,snapshot=on -QEMU_PROG: -drive driver=null-co,snapshot=on: Could not get temporary filename: No such file or directory +QEMU_PROG: -drive driver=null-co,snapshot=on: Could not open temporary file SNAPSHOT_PATH: No such file or directory Testing: -drive file=TEST_DIR/t.qcow2,snapshot=on,read-only=on,if=none,id=drive0 QEMU X.Y.Z monitor - type 'help' for more information