* [PULL 1/8] monitor: simplify functions for getting a dup'd fdset entry
2020-09-16 9:46 [PULL 0/8] Block odirect patches Daniel P. Berrangé
@ 2020-09-16 9:46 ` Daniel P. Berrangé
2020-09-16 9:46 ` [PULL 2/8] util: split off a helper for dealing with O_CLOEXEC flag Daniel P. Berrangé
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-09-16 9:46 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé, qemu-block,
Philippe Mathieu-Daudé, Markus Armbruster, Max Reitz
Currently code has to call monitor_fdset_get_fd, then dup
the return fd, and then add the duplicate FD back into the
fdset. This dance is overly verbose for the caller and
introduces extra failure modes which can be avoided by
folding all the logic into monitor_fdset_dup_fd_add and
removing monitor_fdset_get_fd entirely.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/monitor/monitor.h | 3 +-
include/qemu/osdep.h | 1 +
monitor/misc.c | 58 +++++++++++++++++----------------------
stubs/fdset.c | 8 ++----
util/osdep.c | 19 ++-----------
5 files changed, 32 insertions(+), 57 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1018d754a6..c0170773d4 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -43,8 +43,7 @@ int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
bool has_opaque, const char *opaque,
Error **errp);
-int monitor_fdset_get_fd(int64_t fdset_id, int flags);
-int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
+int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags);
void monitor_fdset_dup_fd_remove(int dup_fd);
int64_t monitor_fdset_dup_fd_find(int dup_fd);
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 412962d91a..66ee5bc45d 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -501,6 +501,7 @@ int qemu_open(const char *name, int flags, ...);
int qemu_close(int fd);
int qemu_unlink(const char *name);
#ifndef _WIN32
+int qemu_dup_flags(int fd, int flags);
int qemu_dup(int fd);
#endif
int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive);
diff --git a/monitor/misc.c b/monitor/misc.c
index e847b58a8c..0b1b9b196c 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -1547,69 +1547,61 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
return fdinfo;
}
-int monitor_fdset_get_fd(int64_t fdset_id, int flags)
+int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags)
{
#ifdef _WIN32
return -ENOENT;
#else
MonFdset *mon_fdset;
- MonFdsetFd *mon_fdset_fd;
- int mon_fd_flags;
- int ret;
qemu_mutex_lock(&mon_fdsets_lock);
QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
+ MonFdsetFd *mon_fdset_fd;
+ MonFdsetFd *mon_fdset_fd_dup;
+ int fd = -1;
+ int dup_fd;
+ int mon_fd_flags;
+
if (mon_fdset->id != fdset_id) {
continue;
}
+
QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) {
mon_fd_flags = fcntl(mon_fdset_fd->fd, F_GETFL);
if (mon_fd_flags == -1) {
- ret = -errno;
- goto out;
+ qemu_mutex_unlock(&mon_fdsets_lock);
+ return -1;
}
if ((flags & O_ACCMODE) == (mon_fd_flags & O_ACCMODE)) {
- ret = mon_fdset_fd->fd;
- goto out;
+ fd = mon_fdset_fd->fd;
+ break;
}
}
- ret = -EACCES;
- goto out;
- }
- ret = -ENOENT;
-
-out:
- qemu_mutex_unlock(&mon_fdsets_lock);
- return ret;
-#endif
-}
-
-int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
-{
- MonFdset *mon_fdset;
- MonFdsetFd *mon_fdset_fd_dup;
- qemu_mutex_lock(&mon_fdsets_lock);
- QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
- if (mon_fdset->id != fdset_id) {
- continue;
+ if (fd == -1) {
+ qemu_mutex_unlock(&mon_fdsets_lock);
+ errno = EACCES;
+ return -1;
}
- QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) {
- if (mon_fdset_fd_dup->fd == dup_fd) {
- goto err;
- }
+
+ dup_fd = qemu_dup_flags(fd, flags);
+ if (dup_fd == -1) {
+ qemu_mutex_unlock(&mon_fdsets_lock);
+ return -1;
}
+
mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup));
mon_fdset_fd_dup->fd = dup_fd;
QLIST_INSERT_HEAD(&mon_fdset->dup_fds, mon_fdset_fd_dup, next);
qemu_mutex_unlock(&mon_fdsets_lock);
- return 0;
+ return dup_fd;
}
-err:
qemu_mutex_unlock(&mon_fdsets_lock);
+ errno = ENOENT;
return -1;
+#endif
}
static int64_t monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove)
diff --git a/stubs/fdset.c b/stubs/fdset.c
index 67dd5e1d34..56b3663d58 100644
--- a/stubs/fdset.c
+++ b/stubs/fdset.c
@@ -1,8 +1,9 @@
#include "qemu/osdep.h"
#include "monitor/monitor.h"
-int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
+int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags)
{
+ errno = ENOSYS;
return -1;
}
@@ -11,11 +12,6 @@ int64_t monitor_fdset_dup_fd_find(int dup_fd)
return -1;
}
-int monitor_fdset_get_fd(int64_t fdset_id, int flags)
-{
- return -ENOENT;
-}
-
void monitor_fdset_dup_fd_remove(int dupfd)
{
}
diff --git a/util/osdep.c b/util/osdep.c
index 4829c07ff6..3d94b4d732 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -122,7 +122,7 @@ static int fcntl_op_getlk = -1;
/*
* Dups an fd and sets the flags
*/
-static int qemu_dup_flags(int fd, int flags)
+int qemu_dup_flags(int fd, int flags)
{
int ret;
int serrno;
@@ -293,7 +293,7 @@ int qemu_open(const char *name, int flags, ...)
/* Attempt dup of fd from fd set */
if (strstart(name, "/dev/fdset/", &fdset_id_str)) {
int64_t fdset_id;
- int fd, dupfd;
+ int dupfd;
fdset_id = qemu_parse_fdset(fdset_id_str);
if (fdset_id == -1) {
@@ -301,24 +301,11 @@ int qemu_open(const char *name, int flags, ...)
return -1;
}
- fd = monitor_fdset_get_fd(fdset_id, flags);
- if (fd < 0) {
- errno = -fd;
- return -1;
- }
-
- dupfd = qemu_dup_flags(fd, flags);
+ dupfd = monitor_fdset_dup_fd_add(fdset_id, flags);
if (dupfd == -1) {
return -1;
}
- ret = monitor_fdset_dup_fd_add(fdset_id, dupfd);
- if (ret == -1) {
- close(dupfd);
- errno = EINVAL;
- return -1;
- }
-
return dupfd;
}
#endif
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PULL 2/8] util: split off a helper for dealing with O_CLOEXEC flag
2020-09-16 9:46 [PULL 0/8] Block odirect patches Daniel P. Berrangé
2020-09-16 9:46 ` [PULL 1/8] monitor: simplify functions for getting a dup'd fdset entry Daniel P. Berrangé
@ 2020-09-16 9:46 ` Daniel P. Berrangé
2020-09-16 9:47 ` [PULL 3/8] util: rename qemu_open() to qemu_open_old() Daniel P. Berrangé
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-09-16 9:46 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé, qemu-block,
Philippe Mathieu-Daudé, Markus Armbruster, Max Reitz
We're going to have multiple callers to open() from qemu_open()
soon. Readability would thus benefit from having a helper for
dealing with O_CLOEXEC.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
util/osdep.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/util/osdep.c b/util/osdep.c
index 3d94b4d732..0d8fa9f016 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -279,6 +279,20 @@ int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive)
}
#endif
+static int qemu_open_cloexec(const char *name, int flags, mode_t mode)
+{
+ int ret;
+#ifdef O_CLOEXEC
+ ret = open(name, flags | O_CLOEXEC, mode);
+#else
+ ret = open(name, flags, mode);
+ if (ret >= 0) {
+ qemu_set_cloexec(ret);
+ }
+#endif
+ return ret;
+}
+
/*
* Opens a file with FD_CLOEXEC set
*/
@@ -318,14 +332,7 @@ int qemu_open(const char *name, int flags, ...)
va_end(ap);
}
-#ifdef O_CLOEXEC
- ret = open(name, flags | O_CLOEXEC, mode);
-#else
- ret = open(name, flags, mode);
- if (ret >= 0) {
- qemu_set_cloexec(ret);
- }
-#endif
+ ret = qemu_open_cloexec(name, flags, mode);
#ifdef O_DIRECT
if (ret == -1 && errno == EINVAL && (flags & O_DIRECT)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PULL 3/8] util: rename qemu_open() to qemu_open_old()
2020-09-16 9:46 [PULL 0/8] Block odirect patches Daniel P. Berrangé
2020-09-16 9:46 ` [PULL 1/8] monitor: simplify functions for getting a dup'd fdset entry Daniel P. Berrangé
2020-09-16 9:46 ` [PULL 2/8] util: split off a helper for dealing with O_CLOEXEC flag Daniel P. Berrangé
@ 2020-09-16 9:47 ` Daniel P. Berrangé
2020-09-16 9:47 ` [PULL 4/8] util: refactor qemu_open_old to split off variadic args handling Daniel P. Berrangé
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-09-16 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé, qemu-block,
Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz
We want to introduce a new version of qemu_open() that uses an Error
object for reporting problems and make this it the preferred interface.
Rename the existing method to release the namespace for the new impl.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
accel/kvm/kvm-all.c | 2 +-
backends/rng-random.c | 2 +-
backends/tpm/tpm_passthrough.c | 8 ++++----
block/file-posix.c | 14 +++++++-------
block/file-win32.c | 5 +++--
block/vvfat.c | 5 +++--
chardev/char-fd.c | 2 +-
chardev/char-pipe.c | 6 +++---
chardev/char.c | 2 +-
dump/dump.c | 2 +-
hw/s390x/s390-skeys.c | 2 +-
hw/usb/host-libusb.c | 2 +-
hw/usb/u2f-passthru.c | 4 ++--
hw/vfio/common.c | 4 ++--
include/qemu/osdep.h | 2 +-
io/channel-file.c | 2 +-
net/vhost-vdpa.c | 2 +-
os-posix.c | 2 +-
qga/channel-posix.c | 4 ++--
qga/commands-posix.c | 6 +++---
target/arm/kvm.c | 2 +-
ui/console.c | 2 +-
util/osdep.c | 2 +-
util/oslib-posix.c | 2 +-
24 files changed, 44 insertions(+), 42 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 63ef6af9a1..ad8b315b35 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2013,7 +2013,7 @@ static int kvm_init(MachineState *ms)
#endif
QLIST_INIT(&s->kvm_parked_vcpus);
s->vmfd = -1;
- s->fd = qemu_open("/dev/kvm", O_RDWR);
+ s->fd = qemu_open_old("/dev/kvm", O_RDWR);
if (s->fd == -1) {
fprintf(stderr, "Could not access KVM kernel module: %m\n");
ret = -errno;
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 32998d8ee7..245b12ab24 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -75,7 +75,7 @@ static void rng_random_opened(RngBackend *b, Error **errp)
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"filename", "a valid filename");
} else {
- s->fd = qemu_open(s->filename, O_RDONLY | O_NONBLOCK);
+ s->fd = qemu_open_old(s->filename, O_RDONLY | O_NONBLOCK);
if (s->fd == -1) {
error_setg_file_open(errp, errno, s->filename);
}
diff --git a/backends/tpm/tpm_passthrough.c b/backends/tpm/tpm_passthrough.c
index 10722e0a41..6d6294ef0a 100644
--- a/backends/tpm/tpm_passthrough.c
+++ b/backends/tpm/tpm_passthrough.c
@@ -218,7 +218,7 @@ static int tpm_passthrough_open_sysfs_cancel(TPMPassthruState *tpm_pt)
char path[PATH_MAX];
if (tpm_pt->options->cancel_path) {
- fd = qemu_open(tpm_pt->options->cancel_path, O_WRONLY);
+ fd = qemu_open_old(tpm_pt->options->cancel_path, O_WRONLY);
if (fd < 0) {
error_report("tpm_passthrough: Could not open TPM cancel path: %s",
strerror(errno));
@@ -236,11 +236,11 @@ static int tpm_passthrough_open_sysfs_cancel(TPMPassthruState *tpm_pt)
dev++;
if (snprintf(path, sizeof(path), "/sys/class/tpm/%s/device/cancel",
dev) < sizeof(path)) {
- fd = qemu_open(path, O_WRONLY);
+ fd = qemu_open_old(path, O_WRONLY);
if (fd < 0) {
if (snprintf(path, sizeof(path), "/sys/class/misc/%s/device/cancel",
dev) < sizeof(path)) {
- fd = qemu_open(path, O_WRONLY);
+ fd = qemu_open_old(path, O_WRONLY);
}
}
}
@@ -272,7 +272,7 @@ tpm_passthrough_handle_device_opts(TPMPassthruState *tpm_pt, QemuOpts *opts)
}
tpm_pt->tpm_dev = value ? value : TPM_PASSTHROUGH_DEFAULT_DEVICE;
- tpm_pt->tpm_fd = qemu_open(tpm_pt->tpm_dev, O_RDWR);
+ tpm_pt->tpm_fd = qemu_open_old(tpm_pt->tpm_dev, O_RDWR);
if (tpm_pt->tpm_fd < 0) {
error_report("Cannot access TPM device using '%s': %s",
tpm_pt->tpm_dev, strerror(errno));
diff --git a/block/file-posix.c b/block/file-posix.c
index 9a00d4190a..bac2566f10 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -630,7 +630,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
raw_parse_flags(bdrv_flags, &s->open_flags, false);
s->fd = -1;
- fd = qemu_open(filename, s->open_flags, 0644);
+ fd = qemu_open_old(filename, s->open_flags, 0644);
ret = fd < 0 ? -errno : 0;
if (ret < 0) {
@@ -1032,13 +1032,13 @@ static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
}
}
- /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
+ /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open_old() */
if (fd == -1) {
const char *normalized_filename = bs->filename;
ret = raw_normalize_devicepath(&normalized_filename, errp);
if (ret >= 0) {
assert(!(*open_flags & O_CREAT));
- fd = qemu_open(normalized_filename, *open_flags);
+ fd = qemu_open_old(normalized_filename, *open_flags);
if (fd == -1) {
error_setg_errno(errp, errno, "Could not reopen file");
return -1;
@@ -2411,7 +2411,7 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
}
/* Create file */
- fd = qemu_open(file_opts->filename, O_RDWR | O_CREAT | O_BINARY, 0644);
+ fd = qemu_open_old(file_opts->filename, O_RDWR | O_CREAT | O_BINARY, 0644);
if (fd < 0) {
result = -errno;
error_setg_errno(errp, -result, "Could not create file");
@@ -3335,7 +3335,7 @@ static bool setup_cdrom(char *bsd_path, Error **errp)
for (index = 0; index < num_of_test_partitions; index++) {
snprintf(test_partition, sizeof(test_partition), "%ss%d", bsd_path,
index);
- fd = qemu_open(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE);
+ fd = qemu_open_old(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE);
if (fd >= 0) {
partition_found = true;
qemu_close(fd);
@@ -3653,7 +3653,7 @@ static int cdrom_probe_device(const char *filename)
int prio = 0;
struct stat st;
- fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
+ fd = qemu_open_old(filename, O_RDONLY | O_NONBLOCK);
if (fd < 0) {
goto out;
}
@@ -3787,7 +3787,7 @@ static int cdrom_reopen(BlockDriverState *bs)
*/
if (s->fd >= 0)
qemu_close(s->fd);
- fd = qemu_open(bs->filename, s->open_flags, 0644);
+ fd = qemu_open_old(bs->filename, s->open_flags, 0644);
if (fd < 0) {
s->fd = -1;
return -EIO;
diff --git a/block/file-win32.c b/block/file-win32.c
index e2900c3a51..b28603c7d5 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -596,8 +596,9 @@ static int raw_co_create(BlockdevCreateOptions *options, Error **errp)
return -EINVAL;
}
- fd = qemu_open(file_opts->filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
- 0644);
+ fd = qemu_open_old(file_opts->filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+ 0644);
if (fd < 0) {
error_setg_errno(errp, errno, "Could not create file");
return -EIO;
diff --git a/block/vvfat.c b/block/vvfat.c
index 36b53c8757..5abb90e7c7 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1352,7 +1352,8 @@ static int open_file(BDRVVVFATState* s,mapping_t* mapping)
if(!s->current_mapping ||
strcmp(s->current_mapping->path,mapping->path)) {
/* open file */
- int fd = qemu_open(mapping->path, O_RDONLY | O_BINARY | O_LARGEFILE);
+ int fd = qemu_open_old(mapping->path,
+ O_RDONLY | O_BINARY | O_LARGEFILE);
if(fd<0)
return -1;
vvfat_close_current_file(s);
@@ -2513,7 +2514,7 @@ static int commit_one_file(BDRVVVFATState* s,
for (i = s->cluster_size; i < offset; i += s->cluster_size)
c = modified_fat_get(s, c);
- fd = qemu_open(mapping->path, O_RDWR | O_CREAT | O_BINARY, 0666);
+ fd = qemu_open_old(mapping->path, O_RDWR | O_CREAT | O_BINARY, 0666);
if (fd < 0) {
fprintf(stderr, "Could not open %s... (%s, %d)\n", mapping->path,
strerror(errno), errno);
diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index c2d8101106..1cd62f2779 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -119,7 +119,7 @@ int qmp_chardev_open_file_source(char *src, int flags, Error **errp)
{
int fd = -1;
- TFR(fd = qemu_open(src, flags, 0666));
+ TFR(fd = qemu_open_old(src, flags, 0666));
if (fd == -1) {
error_setg_file_open(errp, errno, src);
}
diff --git a/chardev/char-pipe.c b/chardev/char-pipe.c
index fd12c9e63b..7eca5d9a56 100644
--- a/chardev/char-pipe.c
+++ b/chardev/char-pipe.c
@@ -132,8 +132,8 @@ static void qemu_chr_open_pipe(Chardev *chr,
filename_in = g_strdup_printf("%s.in", filename);
filename_out = g_strdup_printf("%s.out", filename);
- TFR(fd_in = qemu_open(filename_in, O_RDWR | O_BINARY));
- TFR(fd_out = qemu_open(filename_out, O_RDWR | O_BINARY));
+ TFR(fd_in = qemu_open_old(filename_in, O_RDWR | O_BINARY));
+ TFR(fd_out = qemu_open_old(filename_out, O_RDWR | O_BINARY));
g_free(filename_in);
g_free(filename_out);
if (fd_in < 0 || fd_out < 0) {
@@ -143,7 +143,7 @@ static void qemu_chr_open_pipe(Chardev *chr,
if (fd_out >= 0) {
close(fd_out);
}
- TFR(fd_in = fd_out = qemu_open(filename, O_RDWR | O_BINARY));
+ TFR(fd_in = fd_out = qemu_open_old(filename, O_RDWR | O_BINARY));
if (fd_in < 0) {
error_setg_file_open(errp, errno, filename);
return;
diff --git a/chardev/char.c b/chardev/char.c
index 77e7ec814f..6b85099c03 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -235,7 +235,7 @@ static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
} else {
flags |= O_TRUNC;
}
- chr->logfd = qemu_open(common->logfile, flags, 0666);
+ chr->logfd = qemu_open_old(common->logfile, flags, 0666);
if (chr->logfd < 0) {
error_setg_errno(errp, errno,
"Unable to open logfile %s",
diff --git a/dump/dump.c b/dump/dump.c
index 383bc7876b..13fda440a4 100644
--- a/dump/dump.c
+++ b/dump/dump.c
@@ -1994,7 +1994,7 @@ void qmp_dump_guest_memory(bool paging, const char *file,
#endif
if (strstart(file, "file:", &p)) {
- fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
+ fd = qemu_open_old(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
if (fd < 0) {
error_setg_file_open(errp, errno, p);
return;
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index db2f49cb27..5cc559fe4c 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -125,7 +125,7 @@ void qmp_dump_skeys(const char *filename, Error **errp)
return;
}
- fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+ fd = qemu_open_old(filename, O_WRONLY | O_CREAT | O_TRUNC, 0600);
if (fd < 0) {
error_setg_file_open(errp, errno, filename);
return;
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index 43c93504a2..8b02bee547 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1147,7 +1147,7 @@ static void usb_host_realize(USBDevice *udev, Error **errp)
if (s->hostdevice) {
int fd;
s->needs_autoscan = false;
- fd = qemu_open(s->hostdevice, O_RDWR);
+ fd = qemu_open_old(s->hostdevice, O_RDWR);
if (fd < 0) {
error_setg_errno(errp, errno, "failed to open %s", s->hostdevice);
return;
diff --git a/hw/usb/u2f-passthru.c b/hw/usb/u2f-passthru.c
index e9c8aa4595..ae00e93f35 100644
--- a/hw/usb/u2f-passthru.c
+++ b/hw/usb/u2f-passthru.c
@@ -383,7 +383,7 @@ static int u2f_passthru_open_from_device(struct udev_device *device)
{
const char *devnode = udev_device_get_devnode(device);
- int fd = qemu_open(devnode, O_RDWR);
+ int fd = qemu_open_old(devnode, O_RDWR);
if (fd < 0) {
return -1;
} else if (!u2f_passthru_is_u2f_device(fd)) {
@@ -482,7 +482,7 @@ static void u2f_passthru_realize(U2FKeyState *base, Error **errp)
return;
#endif
} else {
- fd = qemu_open(key->hidraw, O_RDWR);
+ fd = qemu_open_old(key->hidraw, O_RDWR);
if (fd < 0) {
error_setg(errp, "%s: Failed to open %s", TYPE_U2F_PASSTHRU,
key->hidraw);
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 33357140b8..13471ae294 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1254,7 +1254,7 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
}
}
- fd = qemu_open("/dev/vfio/vfio", O_RDWR);
+ fd = qemu_open_old("/dev/vfio/vfio", O_RDWR);
if (fd < 0) {
error_setg_errno(errp, errno, "failed to open /dev/vfio/vfio");
ret = -errno;
@@ -1479,7 +1479,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp)
group = g_malloc0(sizeof(*group));
snprintf(path, sizeof(path), "/dev/vfio/%d", groupid);
- group->fd = qemu_open(path, O_RDWR);
+ group->fd = qemu_open_old(path, O_RDWR);
if (group->fd < 0) {
error_setg_errno(errp, errno, "failed to open %s", path);
goto free_group_exit;
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 66ee5bc45d..ae1234104c 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -497,7 +497,7 @@ int qemu_madvise(void *addr, size_t len, int advice);
int qemu_mprotect_rwx(void *addr, size_t size);
int qemu_mprotect_none(void *addr, size_t size);
-int qemu_open(const char *name, int flags, ...);
+int qemu_open_old(const char *name, int flags, ...);
int qemu_close(int fd);
int qemu_unlink(const char *name);
#ifndef _WIN32
diff --git a/io/channel-file.c b/io/channel-file.c
index dac21f6012..2ed3b75e8b 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -51,7 +51,7 @@ qio_channel_file_new_path(const char *path,
ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
- ioc->fd = qemu_open(path, flags, mode);
+ ioc->fd = qemu_open_old(path, flags, mode);
if (ioc->fd < 0) {
object_unref(OBJECT(ioc));
error_setg_errno(errp, errno,
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index bc0e0d2d35..e2b3ba85bf 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -184,7 +184,7 @@ static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
snprintf(nc->info_str, sizeof(nc->info_str), TYPE_VHOST_VDPA);
nc->queue_index = 0;
s = DO_UPCAST(VhostVDPAState, nc, nc);
- vdpa_device_fd = qemu_open(vhostdev, O_RDWR);
+ vdpa_device_fd = qemu_open_old(vhostdev, O_RDWR);
if (vdpa_device_fd == -1) {
return -errno;
}
diff --git a/os-posix.c b/os-posix.c
index bf98508b6d..0bfd8e2d3c 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -297,7 +297,7 @@ void os_setup_post(void)
error_report("not able to chdir to /: %s", strerror(errno));
exit(1);
}
- TFR(fd = qemu_open("/dev/null", O_RDWR));
+ TFR(fd = qemu_open_old("/dev/null", O_RDWR));
if (fd == -1) {
exit(1);
}
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 8fc205ad21..0373975360 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -127,7 +127,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
switch (c->method) {
case GA_CHANNEL_VIRTIO_SERIAL: {
assert(fd < 0);
- fd = qemu_open(path, O_RDWR | O_NONBLOCK
+ fd = qemu_open_old(path, O_RDWR | O_NONBLOCK
#ifndef CONFIG_SOLARIS
| O_ASYNC
#endif
@@ -157,7 +157,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
struct termios tio;
assert(fd < 0);
- fd = qemu_open(path, O_RDWR | O_NOCTTY | O_NONBLOCK);
+ fd = qemu_open_old(path, O_RDWR | O_NOCTTY | O_NONBLOCK);
if (fd == -1) {
g_critical("error opening channel: %s", strerror(errno));
return false;
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index af5a58a9fd..3bffee99d4 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1365,7 +1365,7 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
}
}
- fd = qemu_open(mount->dirname, O_RDONLY);
+ fd = qemu_open_old(mount->dirname, O_RDONLY);
if (fd == -1) {
error_setg_errno(errp, errno, "failed to open %s", mount->dirname);
goto error;
@@ -1432,7 +1432,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
QTAILQ_FOREACH(mount, &mounts, next) {
logged = false;
- fd = qemu_open(mount->dirname, O_RDONLY);
+ fd = qemu_open_old(mount->dirname, O_RDONLY);
if (fd == -1) {
continue;
}
@@ -1522,7 +1522,7 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
list->next = response->paths;
response->paths = list;
- fd = qemu_open(mount->dirname, O_RDONLY);
+ fd = qemu_open_old(mount->dirname, O_RDONLY);
if (fd == -1) {
result->error = g_strdup_printf("failed to open: %s",
strerror(errno));
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 2eae73315d..0dcb9bfe13 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -71,7 +71,7 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
{
int ret = 0, kvmfd = -1, vmfd = -1, cpufd = -1;
- kvmfd = qemu_open("/dev/kvm", O_RDWR);
+ kvmfd = qemu_open_old("/dev/kvm", O_RDWR);
if (kvmfd < 0) {
goto err;
}
diff --git a/ui/console.c b/ui/console.c
index f8d7643fe4..7592c3c324 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -373,7 +373,7 @@ void qmp_screendump(const char *filename, bool has_device, const char *device,
return;
}
- fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666);
+ fd = qemu_open_old(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666);
if (fd == -1) {
error_setg(errp, "failed to open file '%s': %s", filename,
strerror(errno));
diff --git a/util/osdep.c b/util/osdep.c
index 0d8fa9f016..7504c156e8 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -296,7 +296,7 @@ static int qemu_open_cloexec(const char *name, int flags, mode_t mode)
/*
* Opens a file with FD_CLOEXEC set
*/
-int qemu_open(const char *name, int flags, ...)
+int qemu_open_old(const char *name, int flags, ...)
{
int ret;
int mode = 0;
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index ad8001a4ad..f5f676f079 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -125,7 +125,7 @@ bool qemu_write_pidfile(const char *path, Error **errp)
.l_len = 0,
};
- fd = qemu_open(path, O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR);
+ fd = qemu_open_old(path, O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR);
if (fd == -1) {
error_setg_errno(errp, errno, "Cannot open pid file");
return false;
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PULL 4/8] util: refactor qemu_open_old to split off variadic args handling
2020-09-16 9:46 [PULL 0/8] Block odirect patches Daniel P. Berrangé
` (2 preceding siblings ...)
2020-09-16 9:47 ` [PULL 3/8] util: rename qemu_open() to qemu_open_old() Daniel P. Berrangé
@ 2020-09-16 9:47 ` Daniel P. Berrangé
2020-09-16 9:47 ` [PULL 5/8] util: add Error object for qemu_open_internal error reporting Daniel P. Berrangé
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-09-16 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé, qemu-block,
Philippe Mathieu-Daudé, Markus Armbruster, Max Reitz
This simple refactoring prepares for future patches. The variadic args
handling is split from the main bulk of the open logic.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
util/osdep.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/util/osdep.c b/util/osdep.c
index 7504c156e8..11531e8f59 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -296,10 +296,10 @@ static int qemu_open_cloexec(const char *name, int flags, mode_t mode)
/*
* Opens a file with FD_CLOEXEC set
*/
-int qemu_open_old(const char *name, int flags, ...)
+static int
+qemu_open_internal(const char *name, int flags, mode_t mode)
{
int ret;
- int mode = 0;
#ifndef _WIN32
const char *fdset_id_str;
@@ -324,15 +324,25 @@ int qemu_open_old(const char *name, int flags, ...)
}
#endif
- if (flags & O_CREAT) {
- va_list ap;
+ ret = qemu_open_cloexec(name, flags, mode);
+
+ return ret;
+}
+
- va_start(ap, flags);
+int qemu_open_old(const char *name, int flags, ...)
+{
+ va_list ap;
+ mode_t mode = 0;
+ int ret;
+
+ va_start(ap, flags);
+ if (flags & O_CREAT) {
mode = va_arg(ap, int);
- va_end(ap);
}
+ va_end(ap);
- ret = qemu_open_cloexec(name, flags, mode);
+ ret = qemu_open_internal(name, flags, mode);
#ifdef O_DIRECT
if (ret == -1 && errno == EINVAL && (flags & O_DIRECT)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PULL 5/8] util: add Error object for qemu_open_internal error reporting
2020-09-16 9:46 [PULL 0/8] Block odirect patches Daniel P. Berrangé
` (3 preceding siblings ...)
2020-09-16 9:47 ` [PULL 4/8] util: refactor qemu_open_old to split off variadic args handling Daniel P. Berrangé
@ 2020-09-16 9:47 ` Daniel P. Berrangé
2020-09-16 9:47 ` [PULL 6/8] util: introduce qemu_open and qemu_create with " Daniel P. Berrangé
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-09-16 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé, qemu-block,
Philippe Mathieu-Daudé, Markus Armbruster, Max Reitz
Instead of relying on the limited information from errno, we can now
also provide detailed error messages to callers that ask for it.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
util/osdep.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/util/osdep.c b/util/osdep.c
index 11531e8f59..28aa89adc9 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
+#include "qapi/error.h"
/* Needed early for CONFIG_BSD etc. */
@@ -297,7 +298,7 @@ static int qemu_open_cloexec(const char *name, int flags, mode_t mode)
* Opens a file with FD_CLOEXEC set
*/
static int
-qemu_open_internal(const char *name, int flags, mode_t mode)
+qemu_open_internal(const char *name, int flags, mode_t mode, Error **errp)
{
int ret;
@@ -311,12 +312,15 @@ qemu_open_internal(const char *name, int flags, mode_t mode)
fdset_id = qemu_parse_fdset(fdset_id_str);
if (fdset_id == -1) {
+ error_setg(errp, "Could not parse fdset %s", name);
errno = EINVAL;
return -1;
}
dupfd = monitor_fdset_dup_fd_add(fdset_id, flags);
if (dupfd == -1) {
+ error_setg_errno(errp, errno, "Could not dup FD for %s flags %x",
+ name, flags);
return -1;
}
@@ -326,6 +330,13 @@ qemu_open_internal(const char *name, int flags, mode_t mode)
ret = qemu_open_cloexec(name, flags, mode);
+ if (ret == -1) {
+ const char *action = flags & O_CREAT ? "create" : "open";
+ error_setg_errno(errp, errno, "Could not %s '%s'",
+ action, name);
+ }
+
+
return ret;
}
@@ -342,7 +353,7 @@ int qemu_open_old(const char *name, int flags, ...)
}
va_end(ap);
- ret = qemu_open_internal(name, flags, mode);
+ ret = qemu_open_internal(name, flags, mode, NULL);
#ifdef O_DIRECT
if (ret == -1 && errno == EINVAL && (flags & O_DIRECT)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PULL 6/8] util: introduce qemu_open and qemu_create with error reporting
2020-09-16 9:46 [PULL 0/8] Block odirect patches Daniel P. Berrangé
` (4 preceding siblings ...)
2020-09-16 9:47 ` [PULL 5/8] util: add Error object for qemu_open_internal error reporting Daniel P. Berrangé
@ 2020-09-16 9:47 ` Daniel P. Berrangé
2020-09-16 9:47 ` [PULL 7/8] util: give a specific error message when O_DIRECT doesn't work Daniel P. Berrangé
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-09-16 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé, qemu-block,
Philippe Mathieu-Daudé, Markus Armbruster, Max Reitz
qemu_open_old() works like open(): set errno and return -1 on failure.
It has even more failure modes, though. Reporting the error clearly
to users is basically impossible for many of them.
Our standard cure for "errno is too coarse" is the Error object.
Introduce two new helper methods:
int qemu_open(const char *name, int flags, Error **errp);
int qemu_create(const char *name, int flags, mode_t mode, Error **errp);
Note that with this design we no longer require or even accept the
O_CREAT flag. Avoiding overloading the two distinct operations
means we can avoid variable arguments which would prevent 'errp' from
being the last argument. It also gives us a guarantee that the 'mode' is
given when creating files, avoiding a latent security bug.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/qemu/osdep.h | 6 ++++++
util/osdep.c | 16 ++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ae1234104c..577d9e8315 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -497,7 +497,13 @@ int qemu_madvise(void *addr, size_t len, int advice);
int qemu_mprotect_rwx(void *addr, size_t size);
int qemu_mprotect_none(void *addr, size_t size);
+/*
+ * Don't introduce new usage of this function, prefer the following
+ * qemu_open/qemu_create that take an "Error **errp"
+ */
int qemu_open_old(const char *name, int flags, ...);
+int qemu_open(const char *name, int flags, Error **errp);
+int qemu_create(const char *name, int flags, mode_t mode, Error **errp);
int qemu_close(int fd);
int qemu_unlink(const char *name);
#ifndef _WIN32
diff --git a/util/osdep.c b/util/osdep.c
index 28aa89adc9..c99f1e7db2 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -341,6 +341,22 @@ qemu_open_internal(const char *name, int flags, mode_t mode, Error **errp)
}
+int qemu_open(const char *name, int flags, Error **errp)
+{
+ assert(!(flags & O_CREAT));
+
+ return qemu_open_internal(name, flags, 0, errp);
+}
+
+
+int qemu_create(const char *name, int flags, mode_t mode, Error **errp)
+{
+ assert(!(flags & O_CREAT));
+
+ return qemu_open_internal(name, flags | O_CREAT, mode, errp);
+}
+
+
int qemu_open_old(const char *name, int flags, ...)
{
va_list ap;
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PULL 7/8] util: give a specific error message when O_DIRECT doesn't work
2020-09-16 9:46 [PULL 0/8] Block odirect patches Daniel P. Berrangé
` (5 preceding siblings ...)
2020-09-16 9:47 ` [PULL 6/8] util: introduce qemu_open and qemu_create with " Daniel P. Berrangé
@ 2020-09-16 9:47 ` Daniel P. Berrangé
2020-09-16 9:47 ` [PULL 8/8] block/file: switch to use qemu_open/qemu_create for improved errors Daniel P. Berrangé
2020-09-17 12:37 ` [PULL 0/8] Block odirect patches Peter Maydell
8 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-09-16 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé, qemu-block,
Philippe Mathieu-Daudé, Markus Armbruster, Max Reitz
A common error scenario is to tell QEMU to use O_DIRECT in combination
with a filesystem that doesn't support it. To aid users to diagnosing
their mistake we want to provide a clear error message when this happens.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
util/osdep.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/util/osdep.c b/util/osdep.c
index c99f1e7db2..8ea7a807c1 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -332,11 +332,24 @@ qemu_open_internal(const char *name, int flags, mode_t mode, Error **errp)
if (ret == -1) {
const char *action = flags & O_CREAT ? "create" : "open";
+#ifdef O_DIRECT
+ /* Give more helpful error message for O_DIRECT */
+ if (errno == EINVAL && (flags & O_DIRECT)) {
+ ret = open(name, flags & ~O_DIRECT, mode);
+ if (ret != -1) {
+ close(ret);
+ error_setg(errp, "Could not %s '%s': "
+ "filesystem does not support O_DIRECT",
+ action, name);
+ errno = EINVAL; /* restore first open()'s errno */
+ return -1;
+ }
+ }
+#endif /* O_DIRECT */
error_setg_errno(errp, errno, "Could not %s '%s'",
action, name);
}
-
return ret;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PULL 8/8] block/file: switch to use qemu_open/qemu_create for improved errors
2020-09-16 9:46 [PULL 0/8] Block odirect patches Daniel P. Berrangé
` (6 preceding siblings ...)
2020-09-16 9:47 ` [PULL 7/8] util: give a specific error message when O_DIRECT doesn't work Daniel P. Berrangé
@ 2020-09-16 9:47 ` Daniel P. Berrangé
2020-09-17 12:37 ` [PULL 0/8] Block odirect patches Peter Maydell
8 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2020-09-16 9:47 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé, qemu-block,
Philippe Mathieu-Daudé, Philippe Mathieu-Daudé,
Markus Armbruster, Max Reitz
Currently at startup if using cache=none on a filesystem lacking
O_DIRECT such as tmpfs, at startup QEMU prints
qemu-system-x86_64: -drive file=/tmp/foo.img,cache=none: file system may not support O_DIRECT
qemu-system-x86_64: -drive file=/tmp/foo.img,cache=none: Could not open '/tmp/foo.img': Invalid argument
while at QMP level the hint is missing, so QEMU reports just
"error": {
"class": "GenericError",
"desc": "Could not open '/tmp/foo.img': Invalid argument"
}
which is close to useless for the end user trying to figure out what
they did wrong.
With this change at startup QEMU prints
qemu-system-x86_64: -drive file=/tmp/foo.img,cache=none: Unable to open '/tmp/foo.img': filesystem does not support O_DIRECT
while at the QMP level QEMU reports a massively more informative
"error": {
"class": "GenericError",
"desc": "Unable to open '/tmp/foo.img': filesystem does not support O_DIRECT"
}
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
block/file-posix.c | 18 +++++++-----------
block/file-win32.c | 6 ++----
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index bac2566f10..c63926d592 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -630,11 +630,10 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
raw_parse_flags(bdrv_flags, &s->open_flags, false);
s->fd = -1;
- fd = qemu_open_old(filename, s->open_flags, 0644);
+ fd = qemu_open(filename, s->open_flags, errp);
ret = fd < 0 ? -errno : 0;
if (ret < 0) {
- error_setg_file_open(errp, -ret, filename);
if (ret == -EROFS) {
ret = -EACCES;
}
@@ -1032,15 +1031,13 @@ static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
}
}
- /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open_old() */
+ /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
if (fd == -1) {
const char *normalized_filename = bs->filename;
ret = raw_normalize_devicepath(&normalized_filename, errp);
if (ret >= 0) {
- assert(!(*open_flags & O_CREAT));
- fd = qemu_open_old(normalized_filename, *open_flags);
+ fd = qemu_open(normalized_filename, *open_flags, errp);
if (fd == -1) {
- error_setg_errno(errp, errno, "Could not reopen file");
return -1;
}
}
@@ -2411,10 +2408,9 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp)
}
/* Create file */
- fd = qemu_open_old(file_opts->filename, O_RDWR | O_CREAT | O_BINARY, 0644);
+ fd = qemu_create(file_opts->filename, O_RDWR | O_BINARY, 0644, errp);
if (fd < 0) {
result = -errno;
- error_setg_errno(errp, -result, "Could not create file");
goto out;
}
@@ -3335,7 +3331,7 @@ static bool setup_cdrom(char *bsd_path, Error **errp)
for (index = 0; index < num_of_test_partitions; index++) {
snprintf(test_partition, sizeof(test_partition), "%ss%d", bsd_path,
index);
- fd = qemu_open_old(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE);
+ fd = qemu_open(test_partition, O_RDONLY | O_BINARY | O_LARGEFILE, NULL);
if (fd >= 0) {
partition_found = true;
qemu_close(fd);
@@ -3653,7 +3649,7 @@ static int cdrom_probe_device(const char *filename)
int prio = 0;
struct stat st;
- fd = qemu_open_old(filename, O_RDONLY | O_NONBLOCK);
+ fd = qemu_open(filename, O_RDONLY | O_NONBLOCK, NULL);
if (fd < 0) {
goto out;
}
@@ -3787,7 +3783,7 @@ static int cdrom_reopen(BlockDriverState *bs)
*/
if (s->fd >= 0)
qemu_close(s->fd);
- fd = qemu_open_old(bs->filename, s->open_flags, 0644);
+ fd = qemu_open(bs->filename, s->open_flags, NULL);
if (fd < 0) {
s->fd = -1;
return -EIO;
diff --git a/block/file-win32.c b/block/file-win32.c
index b28603c7d5..2642088bd6 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -596,11 +596,9 @@ static int raw_co_create(BlockdevCreateOptions *options, Error **errp)
return -EINVAL;
}
- fd = qemu_open_old(file_opts->filename,
- O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
- 0644);
+ fd = qemu_create(file_opts->filename, O_WRONLY | O_TRUNC | O_BINARY,
+ 0644, errp);
if (fd < 0) {
- error_setg_errno(errp, errno, "Could not create file");
return -EIO;
}
set_sparse(fd);
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PULL 0/8] Block odirect patches
2020-09-16 9:46 [PULL 0/8] Block odirect patches Daniel P. Berrangé
` (7 preceding siblings ...)
2020-09-16 9:47 ` [PULL 8/8] block/file: switch to use qemu_open/qemu_create for improved errors Daniel P. Berrangé
@ 2020-09-17 12:37 ` Peter Maydell
8 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2020-09-17 12:37 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Kevin Wolf, Qemu-block, QEMU Developers, Markus Armbruster,
Philippe Mathieu-Daudé, Max Reitz
On Wed, 16 Sep 2020 at 10:47, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The following changes since commit de39a045bd8d2b49e4f3d07976622c29d58e0bac:
>
> Merge remote-tracking branch 'remotes/kraxel/tags/vga-20200915-pull-request=
> ' into staging (2020-09-15 14:25:05 +0100)
>
> are available in the Git repository at:
>
> https://github.com/berrange/qemu tags/block-odirect-pull-request
>
> for you to fetch changes up to b18a24a9f889bcf722754046130507d744a1b0b9:
>
> block/file: switch to use qemu_open/qemu_create for improved errors (2020-0=
> 9-16 10:33:48 +0100)
>
> ----------------------------------------------------------------
> block: improve error reporting for unsupported O_DIRECT
>
> ----------------------------------------------------------------
>
> Daniel P. Berrang=C3=A9 (8):
> monitor: simplify functions for getting a dup'd fdset entry
> util: split off a helper for dealing with O_CLOEXEC flag
> util: rename qemu_open() to qemu_open_old()
> util: refactor qemu_open_old to split off variadic args handling
> util: add Error object for qemu_open_internal error reporting
> util: introduce qemu_open and qemu_create with error reporting
> util: give a specific error message when O_DIRECT doesn't work
> block/file: switch to use qemu_open/qemu_create for improved errors
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 10+ messages in thread