* [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic
@ 2019-03-08 15:37 Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only Kevin Wolf
` (8 more replies)
0 siblings, 9 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-03-08 15:37 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, pkrempa, qemu-devel
We introduced the auto-read-only option to fix the problem that block
jobs that reopen a backing file read-write don't work any more when all
nodes are created individually with -blockdev. The reason is that
bs->file of these backing files doesn't inherit the read-only option
from the format layer node any more if it's created separately.
The way auto-read-only was designed to fix this is that it just always
opens the file node read-write if it can, so reopening the format layer
node is enough to make the backing file writable when necessary.
This works in principle, but not when libvirt uses sVirt: Then QEMU
doesn't even have the permissions to open the image file read-write
until libvirt performs an operation where write access is needed.
This series changes auto-read-only so that it works dynamically and
automatically reopens the file read-only or read-write depending on the
permissions that users attached to the node requested.
See also: https://bugzilla.redhat.com/show_bug.cgi?id=1685989
Kevin Wolf (8):
tests/virtio-blk-test: Disable auto-read-only
block: Avoid useless local_err
block: Make permission changes in reopen less wrong
file-posix: Factor out raw_reconfigure_getfd()
file-posix: Store BDRVRawState.reopen_state during reopen
file-posix: Lock new fd in raw_reopen_prepare()
file-posix: Prepare permission code for fd switching
file-posix: Make auto-read-only dynamic
block.c | 39 +++---
block/file-posix.c | 243 ++++++++++++++++++++++++++-----------
tests/virtio-blk-test.c | 2 +-
tests/qemu-iotests/232.out | 12 +-
4 files changed, 203 insertions(+), 93 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
@ 2019-03-08 15:37 ` Kevin Wolf
2019-03-12 2:46 ` Eric Blake
2019-03-08 15:37 ` [Qemu-devel] [PATCH 2/8] block: Avoid useless local_err Kevin Wolf
` (7 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Kevin Wolf @ 2019-03-08 15:37 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, pkrempa, qemu-devel
tests/virtio-blk-test uses a temporary image file that it deletes while
QEMU is still running, so it can't be reopened when writers are
attached or detached. Disable auto-read-only to keep it always writable.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/virtio-blk-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 8d2fc9c710..0e464aeea4 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -72,7 +72,7 @@ static QOSState *pci_test_start(void)
QOSState *qs;
const char *arch = qtest_get_arch();
char *tmp_path;
- const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
+ const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw,auto-read-only=off "
"-drive if=none,id=drive1,file=null-co://,format=raw "
"-device virtio-blk-pci,id=drv0,drive=drive0,"
"addr=%x.%x";
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 2/8] block: Avoid useless local_err
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only Kevin Wolf
@ 2019-03-08 15:37 ` Kevin Wolf
2019-03-11 10:16 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2019-03-08 15:37 ` [Qemu-devel] [PATCH 3/8] block: Make permission changes in reopen less wrong Kevin Wolf
` (6 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Kevin Wolf @ 2019-03-08 15:37 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, pkrempa, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/block.c b/block.c
index ccf008c177..e18bd5eefd 100644
--- a/block.c
+++ b/block.c
@@ -3155,14 +3155,12 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er
{
int ret = -1;
BlockReopenQueueEntry *bs_entry, *next;
- Error *local_err = NULL;
assert(bs_queue != NULL);
QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
assert(bs_entry->state.bs->quiesce_counter > 0);
- if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, &local_err)) {
- error_propagate(errp, local_err);
+ if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, errp)) {
goto cleanup;
}
bs_entry->prepared = true;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 3/8] block: Make permission changes in reopen less wrong
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 2/8] block: Avoid useless local_err Kevin Wolf
@ 2019-03-08 15:37 ` Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 4/8] file-posix: Factor out raw_reconfigure_getfd() Kevin Wolf
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-03-08 15:37 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, pkrempa, qemu-devel
The way that reopen interacts with permission changes has one big
problem: Both operations are recursive, and the permissions are changes
for each node in the reopen queue.
For a simple graph that consists just of parent and child,
.bdrv_check_perm will be called twice for the child, once recursively
when adjusting the permissions of parent, and once again when the child
itself is reopened.
Even worse, the first .bdrv_check_perm call happens before
.bdrv_reopen_prepare was called for the child and the second one is
called afterwards.
Making sure that .bdrv_check_perm (and the other permission callbacks)
are called only once is hard. We can cope with multiple calls right now,
but as soon as file-posix gets a dynamic auto-read-only that may need to
open a new file descriptor, we get the additional requirement that all
of them are after the .bdrv_reopen_prepare call.
So reorder things in bdrv_reopen_multiple() to first call
.bdrv_reopen_prepare for all involved nodes and only then adjust
permissions.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/block.c b/block.c
index e18bd5eefd..9b9d25e843 100644
--- a/block.c
+++ b/block.c
@@ -1698,6 +1698,7 @@ static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
typedef struct BlockReopenQueueEntry {
bool prepared;
+ bool perms_checked;
BDRVReopenState state;
QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;
} BlockReopenQueueEntry;
@@ -3166,6 +3167,16 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er
bs_entry->prepared = true;
}
+ QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+ BDRVReopenState *state = &bs_entry->state;
+ ret = bdrv_check_perm(state->bs, bs_queue, state->perm,
+ state->shared_perm, NULL, errp);
+ if (ret < 0) {
+ goto cleanup_perm;
+ }
+ bs_entry->perms_checked = true;
+ }
+
/* If we reach this point, we have success and just need to apply the
* changes
*/
@@ -3174,7 +3185,20 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er
}
ret = 0;
+cleanup_perm:
+ QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
+ BDRVReopenState *state = &bs_entry->state;
+
+ if (!bs_entry->perms_checked) {
+ continue;
+ }
+ if (ret == 0) {
+ bdrv_set_perm(state->bs, state->perm, state->shared_perm);
+ } else {
+ bdrv_abort_perm_update(state->bs);
+ }
+ }
cleanup:
QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
if (ret) {
@@ -3428,12 +3452,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
} while ((entry = qdict_next(reopen_state->options, entry)));
}
- ret = bdrv_check_perm(reopen_state->bs, queue, reopen_state->perm,
- reopen_state->shared_perm, NULL, errp);
- if (ret < 0) {
- goto error;
- }
-
ret = 0;
/* Restore the original reopen_state->options QDict */
@@ -3500,9 +3518,6 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
bdrv_refresh_limits(bs, NULL);
- bdrv_set_perm(reopen_state->bs, reopen_state->perm,
- reopen_state->shared_perm);
-
new_can_write =
!bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);
if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) {
@@ -3534,8 +3549,6 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
if (drv->bdrv_reopen_abort) {
drv->bdrv_reopen_abort(reopen_state);
}
-
- bdrv_abort_perm_update(reopen_state->bs);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 4/8] file-posix: Factor out raw_reconfigure_getfd()
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
` (2 preceding siblings ...)
2019-03-08 15:37 ` [Qemu-devel] [PATCH 3/8] block: Make permission changes in reopen less wrong Kevin Wolf
@ 2019-03-08 15:37 ` Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 5/8] file-posix: Store BDRVRawState.reopen_state during reopen Kevin Wolf
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-03-08 15:37 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, pkrempa, qemu-devel
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/file-posix.c | 107 ++++++++++++++++++++++++++-------------------
1 file changed, 62 insertions(+), 45 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index ba6ab62a38..ae57ba1fc6 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -842,6 +842,62 @@ static int raw_handle_perm_lock(BlockDriverState *bs,
return ret;
}
+static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
+ int *open_flags, Error **errp)
+{
+ BDRVRawState *s = bs->opaque;
+ int fd = -1;
+ int ret;
+ int fcntl_flags = O_APPEND | O_NONBLOCK;
+#ifdef O_NOATIME
+ fcntl_flags |= O_NOATIME;
+#endif
+
+ *open_flags = 0;
+ if (s->type == FTYPE_CD) {
+ *open_flags |= O_NONBLOCK;
+ }
+
+ raw_parse_flags(flags, open_flags);
+
+#ifdef O_ASYNC
+ /* Not all operating systems have O_ASYNC, and those that don't
+ * will not let us track the state into rs->open_flags (typically
+ * you achieve the same effect with an ioctl, for example I_SETSIG
+ * on Solaris). But we do not use O_ASYNC, so that's fine.
+ */
+ assert((s->open_flags & O_ASYNC) == 0);
+#endif
+
+ if ((*open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
+ /* dup the original fd */
+ fd = qemu_dup(s->fd);
+ if (fd >= 0) {
+ ret = fcntl_setfl(fd, *open_flags);
+ if (ret) {
+ qemu_close(fd);
+ fd = -1;
+ }
+ }
+ }
+
+ /* 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(normalized_filename, *open_flags);
+ if (fd == -1) {
+ error_setg_errno(errp, errno, "Could not reopen file");
+ return -1;
+ }
+ }
+ }
+
+ return fd;
+}
+
static int raw_reopen_prepare(BDRVReopenState *state,
BlockReopenQueue *queue, Error **errp)
{
@@ -858,7 +914,6 @@ static int raw_reopen_prepare(BDRVReopenState *state,
state->opaque = g_new0(BDRVRawReopenState, 1);
rs = state->opaque;
- rs->fd = -1;
/* Handle options changes */
opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort);
@@ -877,50 +932,12 @@ static int raw_reopen_prepare(BDRVReopenState *state,
* bdrv_reopen_prepare() will detect changes and complain. */
qemu_opts_to_qdict(opts, state->options);
- if (s->type == FTYPE_CD) {
- rs->open_flags |= O_NONBLOCK;
- }
-
- raw_parse_flags(state->flags, &rs->open_flags);
-
- int fcntl_flags = O_APPEND | O_NONBLOCK;
-#ifdef O_NOATIME
- fcntl_flags |= O_NOATIME;
-#endif
-
-#ifdef O_ASYNC
- /* Not all operating systems have O_ASYNC, and those that don't
- * will not let us track the state into rs->open_flags (typically
- * you achieve the same effect with an ioctl, for example I_SETSIG
- * on Solaris). But we do not use O_ASYNC, so that's fine.
- */
- assert((s->open_flags & O_ASYNC) == 0);
-#endif
-
- if ((rs->open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
- /* dup the original fd */
- rs->fd = qemu_dup(s->fd);
- if (rs->fd >= 0) {
- ret = fcntl_setfl(rs->fd, rs->open_flags);
- if (ret) {
- qemu_close(rs->fd);
- rs->fd = -1;
- }
- }
- }
-
- /* If we cannot use fcntl, or fcntl failed, fall back to qemu_open() */
- if (rs->fd == -1) {
- const char *normalized_filename = state->bs->filename;
- ret = raw_normalize_devicepath(&normalized_filename, errp);
- if (ret >= 0) {
- assert(!(rs->open_flags & O_CREAT));
- rs->fd = qemu_open(normalized_filename, rs->open_flags);
- if (rs->fd == -1) {
- error_setg_errno(errp, errno, "Could not reopen file");
- ret = -1;
- }
- }
+ rs->fd = raw_reconfigure_getfd(state->bs, state->flags, &rs->open_flags,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ ret = -1;
+ goto out;
}
/* Fail already reopen_prepare() if we can't get a working O_DIRECT
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 5/8] file-posix: Store BDRVRawState.reopen_state during reopen
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
` (3 preceding siblings ...)
2019-03-08 15:37 ` [Qemu-devel] [PATCH 4/8] file-posix: Factor out raw_reconfigure_getfd() Kevin Wolf
@ 2019-03-08 15:37 ` Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 6/8] file-posix: Lock new fd in raw_reopen_prepare() Kevin Wolf
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-03-08 15:37 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, pkrempa, qemu-devel
We'll want to access the file descriptor in the reopen_state while
processing permission changes in the context of the repoen.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/file-posix.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/block/file-posix.c b/block/file-posix.c
index ae57ba1fc6..6aaee1df16 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -144,6 +144,8 @@ typedef struct BDRVRawState {
uint64_t locked_perm;
uint64_t locked_shared_perm;
+ BDRVReopenState *reopen_state;
+
#ifdef CONFIG_XFS
bool is_xfs:1;
#endif
@@ -952,6 +954,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
}
}
+ s->reopen_state = state;
out:
qemu_opts_del(opts);
return ret;
@@ -978,12 +981,16 @@ static void raw_reopen_commit(BDRVReopenState *state)
g_free(state->opaque);
state->opaque = NULL;
+
+ assert(s->reopen_state == state);
+ s->reopen_state = NULL;
}
static void raw_reopen_abort(BDRVReopenState *state)
{
BDRVRawReopenState *rs = state->opaque;
+ BDRVRawState *s = state->bs->opaque;
/* nothing to do if NULL, we didn't get far enough */
if (rs == NULL) {
@@ -996,6 +1003,9 @@ static void raw_reopen_abort(BDRVReopenState *state)
}
g_free(state->opaque);
state->opaque = NULL;
+
+ assert(s->reopen_state == state);
+ s->reopen_state = NULL;
}
static int hdev_get_max_transfer_length(BlockDriverState *bs, int fd)
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 6/8] file-posix: Lock new fd in raw_reopen_prepare()
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
` (4 preceding siblings ...)
2019-03-08 15:37 ` [Qemu-devel] [PATCH 5/8] file-posix: Store BDRVRawState.reopen_state during reopen Kevin Wolf
@ 2019-03-08 15:37 ` Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 7/8] file-posix: Prepare permission code for fd switching Kevin Wolf
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-03-08 15:37 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, pkrempa, qemu-devel
There is no reason why we can take locks on the new file descriptor only
in raw_reopen_commit() where error handling isn't possible any more.
Instead, we can already do this in raw_reopen_prepare().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/file-posix.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 6aaee1df16..932cc8e58c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -906,7 +906,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
BDRVRawState *s;
BDRVRawReopenState *rs;
QemuOpts *opts;
- int ret = 0;
+ int ret;
Error *local_err = NULL;
assert(state != NULL);
@@ -947,14 +947,27 @@ static int raw_reopen_prepare(BDRVReopenState *state,
if (rs->fd != -1) {
raw_probe_alignment(state->bs, rs->fd, &local_err);
if (local_err) {
- qemu_close(rs->fd);
- rs->fd = -1;
error_propagate(errp, local_err);
ret = -EINVAL;
+ goto out_fd;
+ }
+
+ /* Copy locks to the new fd */
+ ret = raw_apply_lock_bytes(NULL, rs->fd, s->locked_perm,
+ s->locked_shared_perm, false, errp);
+ if (ret < 0) {
+ ret = -EINVAL;
+ goto out_fd;
}
}
s->reopen_state = state;
+ ret = 0;
+out_fd:
+ if (ret < 0) {
+ qemu_close(rs->fd);
+ rs->fd = -1;
+ }
out:
qemu_opts_del(opts);
return ret;
@@ -964,18 +977,10 @@ static void raw_reopen_commit(BDRVReopenState *state)
{
BDRVRawReopenState *rs = state->opaque;
BDRVRawState *s = state->bs->opaque;
- Error *local_err = NULL;
s->check_cache_dropped = rs->check_cache_dropped;
s->open_flags = rs->open_flags;
- /* Copy locks to the new fd before closing the old one. */
- raw_apply_lock_bytes(NULL, rs->fd, s->locked_perm,
- s->locked_shared_perm, false, &local_err);
- if (local_err) {
- /* shouldn't fail in a sane host, but report it just in case. */
- error_report_err(local_err);
- }
qemu_close(s->fd);
s->fd = rs->fd;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 7/8] file-posix: Prepare permission code for fd switching
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
` (5 preceding siblings ...)
2019-03-08 15:37 ` [Qemu-devel] [PATCH 6/8] file-posix: Lock new fd in raw_reopen_prepare() Kevin Wolf
@ 2019-03-08 15:37 ` Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 8/8] file-posix: Make auto-read-only dynamic Kevin Wolf
2019-03-11 13:09 ` [Qemu-devel] [PATCH 0/8] " Peter Krempa
8 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-03-08 15:37 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, pkrempa, qemu-devel
In order to be able to dynamically reopen the file read-only or
read-write, depending on the users that are attached, we need to be able
to switch to a different file descriptor during the permission change.
This interacts with reopen, which also creates a new file descriptor and
performs permission changes internally. In this case, the permission
change code must reuse the reopen file descriptor instead of creating a
third one.
In turn, reopen can drop its code to copy file locks to the new file
descriptor because that is now done when applying the new permissions.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/file-posix.c | 91 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 80 insertions(+), 11 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 932cc8e58c..bcfb38ec4b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -144,6 +144,7 @@ typedef struct BDRVRawState {
uint64_t locked_perm;
uint64_t locked_shared_perm;
+ int perm_change_fd;
BDRVReopenState *reopen_state;
#ifdef CONFIG_XFS
@@ -845,7 +846,8 @@ static int raw_handle_perm_lock(BlockDriverState *bs,
}
static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
- int *open_flags, Error **errp)
+ int *open_flags, bool force_dup,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
int fd = -1;
@@ -871,6 +873,11 @@ static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
assert((s->open_flags & O_ASYNC) == 0);
#endif
+ if (!force_dup && *open_flags == s->open_flags) {
+ /* We're lucky, the existing fd is fine */
+ return s->fd;
+ }
+
if ((*open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {
/* dup the original fd */
fd = qemu_dup(s->fd);
@@ -935,7 +942,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
qemu_opts_to_qdict(opts, state->options);
rs->fd = raw_reconfigure_getfd(state->bs, state->flags, &rs->open_flags,
- &local_err);
+ true, &local_err);
if (local_err) {
error_propagate(errp, local_err);
ret = -1;
@@ -951,14 +958,6 @@ static int raw_reopen_prepare(BDRVReopenState *state,
ret = -EINVAL;
goto out_fd;
}
-
- /* Copy locks to the new fd */
- ret = raw_apply_lock_bytes(NULL, rs->fd, s->locked_perm,
- s->locked_shared_perm, false, errp);
- if (ret < 0) {
- ret = -EINVAL;
- goto out_fd;
- }
}
s->reopen_state = state;
@@ -2696,12 +2695,73 @@ static QemuOptsList raw_create_opts = {
static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared,
Error **errp)
{
- return raw_handle_perm_lock(bs, RAW_PL_PREPARE, perm, shared, errp);
+ BDRVRawState *s = bs->opaque;
+ BDRVRawReopenState *rs = NULL;
+ int open_flags;
+ int ret;
+
+ if (s->reopen_state) {
+ /* We already have a new file descriptor to set permissions for */
+ assert(s->reopen_state->perm == perm);
+ assert(s->reopen_state->shared_perm == shared);
+ if (s->perm_change_fd) {
+ /* reopen may call this function several times (directly and
+ * recursively while change permissions of the parent). Ignore all
+ * but the first call. */
+ return 0;
+ }
+ rs = s->reopen_state->opaque;
+ s->perm_change_fd = rs->fd;
+ } else {
+ /* We may need a new fd if auto-read-only switches the mode */
+ assert(!s->perm_change_fd);
+ ret = raw_reconfigure_getfd(bs, bs->open_flags, &open_flags,
+ false, errp);
+ if (ret < 0) {
+ return ret;
+ } else if (ret != s->fd) {
+ s->perm_change_fd = ret;
+ }
+ }
+
+ /* Prepare permissions on old fd to avoid conflicts between old and new,
+ * but keep everything locked that new will need. */
+ ret = raw_handle_perm_lock(bs, RAW_PL_PREPARE, perm, shared, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+
+ /* Copy locks to the new fd */
+ if (s->perm_change_fd) {
+ ret = raw_apply_lock_bytes(NULL, s->perm_change_fd, perm, ~shared,
+ false, errp);
+ if (ret < 0) {
+ raw_handle_perm_lock(bs, RAW_PL_ABORT, 0, 0, NULL);
+ goto fail;
+ }
+ }
+ return 0;
+
+fail:
+ if (s->perm_change_fd && !s->reopen_state) {
+ qemu_close(s->perm_change_fd);
+ }
+ s->perm_change_fd = 0;
+ return ret;
}
static void raw_set_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared)
{
BDRVRawState *s = bs->opaque;
+
+ /* For reopen, we have already switched to the new fd (.bdrv_set_perm is
+ * called after .bdrv_reopen_commit) */
+ if (s->perm_change_fd && s->fd != s->perm_change_fd) {
+ qemu_close(s->fd);
+ s->fd = s->perm_change_fd;
+ }
+ s->perm_change_fd = 0;
+
raw_handle_perm_lock(bs, RAW_PL_COMMIT, perm, shared, NULL);
s->perm = perm;
s->shared_perm = shared;
@@ -2709,6 +2769,15 @@ static void raw_set_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared)
static void raw_abort_perm_update(BlockDriverState *bs)
{
+ BDRVRawState *s = bs->opaque;
+
+ /* For reopen, .bdrv_reopen_abort is called afterwards and will close
+ * the file descriptor. */
+ if (s->perm_change_fd && !s->reopen_state) {
+ qemu_close(s->perm_change_fd);
+ }
+ s->perm_change_fd = 0;
+
raw_handle_perm_lock(bs, RAW_PL_ABORT, 0, 0, NULL);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 8/8] file-posix: Make auto-read-only dynamic
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
` (6 preceding siblings ...)
2019-03-08 15:37 ` [Qemu-devel] [PATCH 7/8] file-posix: Prepare permission code for fd switching Kevin Wolf
@ 2019-03-08 15:37 ` Kevin Wolf
2019-03-11 13:09 ` [Qemu-devel] [PATCH 0/8] " Peter Krempa
8 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-03-08 15:37 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, mreitz, eblake, pkrempa, qemu-devel
Until now, with auto-read-only=on we tried to open the file read-write
first and if that failed, read-only was tried. This is actually not good
enough for libvirt, which gives QEMU SELinux permissions for read-write
only as soon as it actually intends to write to the image. So we need to
be able to switch between read-only and read-write at runtime.
This patch makes auto-read-only dynamic, i.e. the file is opened
read-only as long as no user of the node has requested write
permissions, but it is automatically reopened read-write as soon as the
first writer is attached. Conversely, if the last writer goes away, the
file is reopened read-only again.
bs->read_only is no longer set for auto-read-only=on files even if the
file descriptor is opened read-only because it will be transparently
upgraded as soon as a writer is attached. This changes the output of
qemu-iotests 232.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/file-posix.c | 36 +++++++++++++++++-------------------
tests/qemu-iotests/232.out | 12 ++++++------
2 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index bcfb38ec4b..6718bc2e9c 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -376,13 +376,21 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
}
}
-static void raw_parse_flags(int bdrv_flags, int *open_flags)
+static void raw_parse_flags(int bdrv_flags, int *open_flags, bool has_writers)
{
+ bool read_write = false;
assert(open_flags != NULL);
*open_flags |= O_BINARY;
*open_flags &= ~O_ACCMODE;
- if (bdrv_flags & BDRV_O_RDWR) {
+
+ if (bdrv_flags & BDRV_O_AUTO_RDONLY) {
+ read_write = has_writers;
+ } else if (bdrv_flags & BDRV_O_RDWR) {
+ read_write = true;
+ }
+
+ if (read_write) {
*open_flags |= O_RDWR;
} else {
*open_flags |= O_RDONLY;
@@ -518,24 +526,12 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
false);
s->open_flags = open_flags;
- raw_parse_flags(bdrv_flags, &s->open_flags);
+ raw_parse_flags(bdrv_flags, &s->open_flags, false);
s->fd = -1;
fd = qemu_open(filename, s->open_flags, 0644);
ret = fd < 0 ? -errno : 0;
- if (ret == -EACCES || ret == -EROFS) {
- /* Try to degrade to read-only, but if it doesn't work, still use the
- * normal error message. */
- if (bdrv_apply_auto_read_only(bs, NULL, NULL) == 0) {
- bdrv_flags &= ~BDRV_O_RDWR;
- raw_parse_flags(bdrv_flags, &s->open_flags);
- assert(!(s->open_flags & O_CREAT));
- fd = qemu_open(filename, s->open_flags);
- ret = fd < 0 ? -errno : 0;
- }
- }
-
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not open '%s'", filename);
if (ret == -EROFS) {
@@ -846,12 +842,14 @@ static int raw_handle_perm_lock(BlockDriverState *bs,
}
static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
- int *open_flags, bool force_dup,
+ int *open_flags, uint64_t perm, bool force_dup,
Error **errp)
{
BDRVRawState *s = bs->opaque;
int fd = -1;
int ret;
+ bool has_writers = perm &
+ (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED | BLK_PERM_RESIZE);
int fcntl_flags = O_APPEND | O_NONBLOCK;
#ifdef O_NOATIME
fcntl_flags |= O_NOATIME;
@@ -862,7 +860,7 @@ static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
*open_flags |= O_NONBLOCK;
}
- raw_parse_flags(flags, open_flags);
+ raw_parse_flags(flags, open_flags, has_writers);
#ifdef O_ASYNC
/* Not all operating systems have O_ASYNC, and those that don't
@@ -942,7 +940,7 @@ static int raw_reopen_prepare(BDRVReopenState *state,
qemu_opts_to_qdict(opts, state->options);
rs->fd = raw_reconfigure_getfd(state->bs, state->flags, &rs->open_flags,
- true, &local_err);
+ state->perm, true, &local_err);
if (local_err) {
error_propagate(errp, local_err);
ret = -1;
@@ -2715,7 +2713,7 @@ static int raw_check_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared,
} else {
/* We may need a new fd if auto-read-only switches the mode */
assert(!s->perm_change_fd);
- ret = raw_reconfigure_getfd(bs, bs->open_flags, &open_flags,
+ ret = raw_reconfigure_getfd(bs, bs->open_flags, &open_flags, perm,
false, errp);
if (ret < 0) {
return ret;
diff --git a/tests/qemu-iotests/232.out b/tests/qemu-iotests/232.out
index dcb683afa3..3bd1a920af 100644
--- a/tests/qemu-iotests/232.out
+++ b/tests/qemu-iotests/232.out
@@ -22,12 +22,12 @@ NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)
NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)
QEMU_PROG: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none,read-only=off,auto-read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
-NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)
-NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)
+NODE_NAME: TEST_DIR/t.IMGFMT (file)
+NODE_NAME: TEST_DIR/t.IMGFMT (file)
QEMU_PROG: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none,auto-read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
-NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)
-NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)
+NODE_NAME: TEST_DIR/t.IMGFMT (file)
+NODE_NAME: TEST_DIR/t.IMGFMT (file)
=== -blockdev with read-write image: read-only/auto-read-only combinations ===
@@ -50,10 +50,10 @@ node0: TEST_DIR/t.IMGFMT (file, read-only)
node0: TEST_DIR/t.IMGFMT (file, read-only)
QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0,read-only=off,auto-read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
-node0: TEST_DIR/t.IMGFMT (file, read-only)
+node0: TEST_DIR/t.IMGFMT (file)
QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0,read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0,auto-read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
-node0: TEST_DIR/t.IMGFMT (file, read-only)
+node0: TEST_DIR/t.IMGFMT (file)
QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0: Could not open 'TEST_DIR/t.IMGFMT': Permission denied
*** done
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/8] block: Avoid useless local_err
2019-03-08 15:37 ` [Qemu-devel] [PATCH 2/8] block: Avoid useless local_err Kevin Wolf
@ 2019-03-11 10:16 ` Alberto Garcia
0 siblings, 0 replies; 12+ messages in thread
From: Alberto Garcia @ 2019-03-11 10:16 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel, mreitz
On Fri 08 Mar 2019 04:37:51 PM CET, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
` (7 preceding siblings ...)
2019-03-08 15:37 ` [Qemu-devel] [PATCH 8/8] file-posix: Make auto-read-only dynamic Kevin Wolf
@ 2019-03-11 13:09 ` Peter Krempa
8 siblings, 0 replies; 12+ messages in thread
From: Peter Krempa @ 2019-03-11 13:09 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, mreitz, eblake, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 18919 bytes --]
On Fri, Mar 08, 2019 at 16:37:49 +0100, Kevin Wolf wrote:
> We introduced the auto-read-only option to fix the problem that block
> jobs that reopen a backing file read-write don't work any more when all
> nodes are created individually with -blockdev. The reason is that
> bs->file of these backing files doesn't inherit the read-only option
> from the format layer node any more if it's created separately.
>
> The way auto-read-only was designed to fix this is that it just always
> opens the file node read-write if it can, so reopening the format layer
> node is enough to make the backing file writable when necessary.
>
> This works in principle, but not when libvirt uses sVirt: Then QEMU
> doesn't even have the permissions to open the image file read-write
> until libvirt performs an operation where write access is needed.
>
> This series changes auto-read-only so that it works dynamically and
> automatically reopens the file read-only or read-write depending on the
> permissions that users attached to the node requested.
While testing this series by attempting a non-active layer block-commit
I've got an assertion failure:
qemu-system-x86_64: block/file-posix.c:2715: raw_check_perm: Assertion `!s->perm_change_fd' failed.
Thread 1 "qemu-system-x86" received signal SIGABRT, Aborted.
0x00007f69fe08753f in raise () from target:/lib64/libc.so.6
Thread 1 (Thread 0x7f69f3b08680 (LWP 2694)):
#0 0x00007f69fe08753f in raise () from target:/lib64/libc.so.6
#1 0x00007f69fe071895 in abort () from target:/lib64/libc.so.6
#2 0x00007f69fe071769 in __assert_fail_base.cold.0 ()
from target:/lib64/libc.so.6
#3 0x00007f69fe07f9f6 in __assert_fail () from target:/lib64/libc.so.6
#4 0x00005563cfeea7e9 in raw_check_perm (bs=0x5563d1f90100, perm=11, shared=21, errp=0x7fffc280c0f0) at block/file-posix.c:2715
#5 0x00005563cfe9aab4 in bdrv_check_perm (bs=bs@entry=0x5563d1f90100, q=0x5563d2bd0170, q@entry=0xbed88f135e2a5600, cumulative_perms=11, cumulative_shared_perms=21, ignore_children=ignore_children@entry=0x7f69a47f9710, errp=errp@entry=0x7fffc280c0f0) at block.c:1790
#6 0x00005563cfe9a875 in bdrv_check_update_perm (bs=0x5563d1f90100, q=0xbed88f135e2a5600, q@entry=0x5563d2bd0170, new_used_perm=new_used_perm@entry=11, new_shared_perm=new_shared_perm@entry=21, ignore_children=ignore_children@entry=0x7f69a47f9710, errp=errp@entry=0x7fffc280c0f0) at block.c:1976
#7 0x00005563cfe9a9ba in bdrv_child_check_perm (c=c@entry=0x5563d1eb9dd0, q=q@entry=0x5563d2bd0170, perm=11, shared=21, ignore_children=0x7f69a47f9710, ignore_children@entry=0x5563d1a57810, errp=errp@entry=0x7fffc280c0f0) at block.c:1989
#8 0x00005563cfe9ab44 in bdrv_check_perm (bs=0x5563d1f943a0, bs@entry=0xb, q=0x5563d2bd0170, q@entry=0xbed88f135e2a5600, cumulative_perms=0, cumulative_shared_perms=31, ignore_children=ignore_children@entry=0x5563d1a57810, errp=0x7fffc280c0f0, errp@entry=0x15) at block.c:1806
#9 0x00005563cfe9a875 in bdrv_check_update_perm (bs=0xb, q=0xbed88f135e2a5600, q@entry=0x5563d2bd0170, new_used_perm=new_used_perm@entry=0, new_shared_perm=new_shared_perm@entry=31, ignore_children=ignore_children@entry=0x5563d1a57810, errp=0x15, errp@entry=0x7fffc280c0f0) at block.c:1976
#10 0x00005563cfe9a9ba in bdrv_child_check_perm (c=c@entry=0x5563d1eb9910, q=q@entry=0x5563d2bd0170, perm=0, shared=31, ignore_children=0x5563d1a57810, ignore_children@entry=0x5563d32dfd40, errp=errp@entry=0x7fffc280c0f0) at block.c:1989
#11 0x00005563cfe9ab44 in bdrv_check_perm (bs=0x5563d2011320, bs@entry=0x0, q=0x5563d2bd0170, q@entry=0xbed88f135e2a5600, cumulative_perms=0, cumulative_shared_perms=6, ignore_children=ignore_children@entry=0x5563d32dfd40, errp=0x7fffc280c0f0, errp@entry=0x1f) at block.c:1806
#12 0x00005563cfe9a875 in bdrv_check_update_perm (bs=0x0, q=0xbed88f135e2a5600, q@entry=0x5563d2bd0170, new_used_perm=new_used_perm@entry=0, new_shared_perm=new_shared_perm@entry=31, ignore_children=ignore_children@entry=0x5563d32dfd40, errp=0x1f, errp@entry=0x7fffc280c0f0) at block.c:1976
#13 0x00005563cfe9a9ba in bdrv_child_check_perm (c=c@entry=0x5563d1d96690, q=q@entry=0x5563d2bd0170, perm=0, shared=31, ignore_children=0x5563d32dfd40, ignore_children@entry=0x5563d32dfe00, errp=errp@entry=0x7fffc280c0f0) at block.c:1989
#14 0x00005563cfe9ab44 in bdrv_check_perm (bs=0x5563d2092340, bs@entry=0x0, q=0x5563d2bd0170, q@entry=0xbed88f135e2a5600, cumulative_perms=0, cumulative_shared_perms=6, ignore_children=ignore_children@entry=0x5563d32dfe00, errp=0x7fffc280c0f0, errp@entry=0x1f) at block.c:1806
#15 0x00005563cfe9a875 in bdrv_check_update_perm (bs=0x0, q=0xbed88f135e2a5600, q@entry=0x5563d2bd0170, new_used_perm=new_used_perm@entry=0, new_shared_perm=new_shared_perm@entry=31, ignore_children=ignore_children@entry=0x5563d32dfe00, errp=0x1f, errp@entry=0x7fffc280c0f0) at block.c:1976
#16 0x00005563cfe9a9ba in bdrv_child_check_perm (c=c@entry=0x5563d37424d0, q=q@entry=0x5563d2bd0170, perm=0, shared=31, ignore_children=0x5563d32dfe00, ignore_children@entry=0x5563d32dfb40, errp=errp@entry=0x7fffc280c0f0) at block.c:1989
#17 0x00005563cfe9ab44 in bdrv_check_perm (bs=0x5563d2fea010, bs@entry=0x0, q=0x5563d2bd0170, q@entry=0xbed88f135e2a5600, cumulative_perms=1, cumulative_shared_perms=21, ignore_children=ignore_children@entry=0x5563d32dfb40, errp=0x7fffc280c0f0, errp@entry=0x1f) at block.c:1806
#18 0x00005563cfe9a875 in bdrv_check_update_perm (bs=0x0, q=0xbed88f135e2a5600, q@entry=0x5563d2bd0170, new_used_perm=new_used_perm@entry=1, new_shared_perm=new_shared_perm@entry=21, ignore_children=ignore_children@entry=0x5563d32dfb40, errp=0x1f, errp@entry=0x7fffc280c0f0) at block.c:1976
#19 0x00005563cfe9a9ba in bdrv_child_check_perm (c=c@entry=0x5563d1e17a50, q=q@entry=0x5563d2bd0170, perm=1, shared=21, ignore_children=0x5563d32dfb40, ignore_children@entry=0x0, errp=errp@entry=0x7fffc280c0f0) at block.c:1989
#20 0x00005563cfe9ab44 in bdrv_check_perm (bs=0x5563d2113340, q=q@entry=0x5563d2bd0170, cumulative_perms=1, cumulative_shared_perms=21, ignore_children=ignore_children@entry=0x0, errp=errp@entry=0x7fffc280c0f0) at block.c:1806
#21 0x00005563cfe9f317 in bdrv_reopen_multiple (ctx=<optimized out>, bs_queue=0x5563d2bd0170, errp=errp@entry=0x7fffc280c0f0) at block.c:3167
#22 0x00005563cfe9f451 in bdrv_reopen_set_read_only (bs=bs@entry=0x5563d2113340, read_only=read_only@entry=false, errp=errp@entry=0x7fffc280c0f0) at block.c:5293
#23 0x00005563cfe9f4fd in bdrv_backing_update_filename (c=<optimized out>, base=0x5563d1f943a0, filename=0x5563d1f943d1 "/tmp/commit0.qcow2", errp=0x7fffc280c0f0) at block.c:1119
#24 0x00005563cfe9f66f in bdrv_drop_intermediate (top=0x5563d2fea010, base=0x5563d1f943a0, backing_file_str=0x5563d1f943d1 "/tmp/commit0.qcow2") at block.c:4018
#25 0x00005563cfea4689 in job_prepare (job=0x5563d2539880) at job.c:771
#26 job_txn_apply (txn=<optimized out>, fn=<optimized out>) at job.c:146
#27 job_do_finalize (job=0x5563d2539880) at job.c:788
#28 0x00005563cfea4941 in job_completed_txn_success (job=0x5563d2539880) at job.c:842
#29 job_completed (job=0x5563d2539880) at job.c:855
#30 job_completed (job=0x5563d2539880) at job.c:846
#31 0x00005563cfea49a0 in job_exit (opaque=0x5563d2539880) at job.c:874
#32 0x00005563cff991de in aio_bh_call (bh=0x7f69a0004800) at util/async.c:118
#33 aio_bh_poll (ctx=ctx@entry=0x5563d1beb7a0) at util/async.c:118
#34 0x00005563cff9c790 in aio_dispatch (ctx=0x5563d1beb7a0) at util/aio-posix.c:460
#35 0x00005563cff990be in aio_ctx_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at util/async.c:261
#36 0x00007f6a0013506d in g_main_context_dispatch () from target:/lib64/libglib-2.0.so.0
#37 0x00005563cff9b9a8 in glib_pollfds_poll () at util/main-loop.c:215
#38 os_host_main_loop_wait (timeout=<optimized out>) at util/main-loop.c:238
#39 main_loop_wait (nonblocking=<optimized out>) at util/main-loop.c:514
#40 0x00005563cfcac499 in main_loop () at vl.c:1923
#41 0x00005563cfb4c943 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4579
This is the log of QEMU monitor interactions by libvirtd:
[trimmed boring startup of qemu]
2019-03-11 12:57:10.485+0000: 3228: info : qemuMonitorIOWrite:553 : QEMU_MONITOR_IO_WRITE: mon=0x7fcff4027880 buf={"execute":"cont","id":"libvirt-18"}^M
2019-03-11 12:57:10.485+0000: 3228: info : qemuMonitorJSONIOProcessLine:219 : QEMU_MONITOR_RECV_EVENT: mon=0x7fcff4027880 event={"timestamp": {"seconds": 1552309030, "microseconds": 485648}, "event": "RESUME"}
2019-03-11 12:57:10.486+0000: 3228: info : qemuMonitorJSONIOProcessLine:224 : QEMU_MONITOR_RECV_REPLY: mon=0x7fcff4027880 reply={"return": {}, "id": "libvirt-18"}
2019-03-11 12:57:20.398+0000: 3243: info : qemuMonitorSend:1085 : QEMU_MONITOR_SEND_MSG: mon=0x7fcff4027880 msg={"execute":"block-commit","arguments":{"device":"libvirt-2-format","job-id":"commit-vdb-libvirt-10-format","top-node":"libvirt-10-format","base-node":"libvirt-12-format","auto-finalize":true,"auto-dismiss":false},"id":"libvirt-19"}^M
2019-03-11 12:57:20.398+0000: 3228: info : qemuMonitorIOWrite:553 : QEMU_MONITOR_IO_WRITE: mon=0x7fcff4027880 buf={"execute":"block-commit","arguments":{"device":"libvirt-2-format","job-id":"commit-vdb-libvirt-10-format","top-node":"libvirt-10-format","base-node":"libvirt-12-format","auto-finalize":true,"auto-dismiss":false},"id":"libvirt-19"}^M
2019-03-11 12:57:20.400+0000: 3228: info : qemuMonitorJSONIOProcessLine:219 : QEMU_MONITOR_RECV_EVENT: mon=0x7fcff4027880 event={"timestamp": {"seconds": 1552309040, "microseconds": 400150}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "commit-vdb-libvirt-10-format"}}
2019-03-11 12:57:20.400+0000: 3228: info : qemuMonitorJSONIOProcessLine:219 : QEMU_MONITOR_RECV_EVENT: mon=0x7fcff4027880 event={"timestamp": {"seconds": 1552309040, "microseconds": 400643}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "commit-vdb-libvirt-10-format"}}
2019-03-11 12:57:20.400+0000: 3228: info : qemuMonitorJSONIOProcessLine:224 : QEMU_MONITOR_RECV_REPLY: mon=0x7fcff4027880 reply={"return": {}, "id": "libvirt-19"}
2019-03-11 12:57:20.400+0000: 3228: info : qemuMonitorJSONIOProcessLine:219 : QEMU_MONITOR_RECV_EVENT: mon=0x7fcff4027880 event={"timestamp": {"seconds": 1552309040, "microseconds": 400800}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "commit-vdb-libvirt-10-format"}}
2019-03-11 12:57:20.401+0000: 3228: info : qemuMonitorJSONIOProcessLine:219 : QEMU_MONITOR_RECV_EVENT: mon=0x7fcff4027880 event={"timestamp": {"seconds": 1552309040, "microseconds": 400825}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "commit-vdb-libvirt-10-format"}}
2019-03-11 12:57:20.737+0000: 3386: info : qemuMonitorClose:1007 : QEMU_MONITOR_CLOSE: mon=0x7fcff4027880 refs=1
The following command line was used:
LC_ALL=C \
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin \
QEMU_AUDIO_DRV=spice \
/home/pipo/git/qemu.git/x86_64-softmmu/qemu-system-x86_64 \
-name guest=upstream-bj,debug-threads=on \
-S \
-object secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-23-upstream-bj/master-key.aes \
-machine pc-i440fx-2.9,accel=kvm,usb=off,vmport=off,dump-guest-core=off \
-m 1000 \
-realtime mlock=off \
-smp 2,maxcpus=8,sockets=8,cores=1,threads=1 \
-object memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/23-upstream-bj/ram-node0,share=yes,size=524288000,host-nodes=0,policy=bind \
-numa node,nodeid=0,cpus=0,cpus=2,cpus=4,cpus=6,memdev=ram-node0 \
-object memory-backend-file,id=ram-node1,mem-path=/var/lib/libvirt/qemu/ram/libvirt/qemu/23-upstream-bj/ram-node1,share=yes,size=524288000,host-nodes=0,policy=bind \
-numa node,nodeid=1,cpus=1,cpus=3,cpus=5,cpus=7,memdev=ram-node1 \
-uuid 841752b8-9452-4078-a62b-8fd9a9af011c \
-no-user-config \
-nodefaults \
-chardev socket,id=charmonitor,fd=29,server,nowait \
-mon chardev=charmonitor,id=monitor,mode=control \
-rtc base=utc,driftfix=slew \
-global kvm-pit.lost_tick_policy=delay \
-no-hpet \
-no-shutdown \
-global PIIX4_PM.disable_s3=1 \
-global PIIX4_PM.disable_s4=1 \
-boot menu=on,strict=on \
-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x6.0x7 \
-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x6 \
-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x6.0x1 \
-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x6.0x2 \
-device lsi,id=scsi0,bus=pci.0,addr=0x9 \
-device ahci,id=sata0,bus=pci.0,addr=0xb \
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 \
-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/systemrescuecd-x86-4.9.5.iso","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"raw","file":"libvirt-4-storage"}' \
-device ide-cd,bus=ide.0,unit=0,drive=libvirt-4-format,id=ide0-0-0,bootindex=1 \
-blockdev '{"driver":"file","filename":"/tmp/pull0.qcow2","node-name":"libvirt-16-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-16-format","read-only":true,"driver":"qcow2","file":"libvirt-16-storage","backing":null}' \
-blockdev '{"driver":"file","filename":"/tmp/pull1.qcow2","node-name":"libvirt-15-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-15-format","read-only":true,"driver":"qcow2","file":"libvirt-15-storage","backing":"libvirt-16-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/pull2.qcow2","node-name":"libvirt-14-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-14-format","read-only":true,"driver":"qcow2","file":"libvirt-14-storage","backing":"libvirt-15-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/pull3.qcow2","node-name":"libvirt-13-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-13-format","read-only":true,"driver":"qcow2","file":"libvirt-13-storage","backing":"libvirt-14-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/pull4.qcow2","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"qcow2","file":"libvirt-3-storage","backing":"libvirt-13-format"}' \
-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0xa,drive=libvirt-3-format,id=virtio-disk0 \
-blockdev '{"driver":"file","filename":"/tmp/commit0.qcow2","node-name":"libvirt-12-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-12-format","read-only":true,"driver":"qcow2","file":"libvirt-12-storage","backing":null}' \
-blockdev '{"driver":"file","filename":"/tmp/commit1.qcow2","node-name":"libvirt-11-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-11-format","read-only":true,"driver":"qcow2","file":"libvirt-11-storage","backing":"libvirt-12-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/commit2.qcow2","node-name":"libvirt-10-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-10-format","read-only":true,"driver":"qcow2","file":"libvirt-10-storage","backing":"libvirt-11-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/commit3.qcow2","node-name":"libvirt-9-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-9-format","read-only":true,"driver":"qcow2","file":"libvirt-9-storage","backing":"libvirt-10-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/commit4.qcow2","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"qcow2","file":"libvirt-2-storage","backing":"libvirt-9-format"}' \
-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0xc,drive=libvirt-2-format,id=virtio-disk1 \
-blockdev '{"driver":"file","filename":"/tmp/copy0.qcow2","node-name":"libvirt-8-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-8-format","read-only":true,"driver":"qcow2","file":"libvirt-8-storage","backing":null}' \
-blockdev '{"driver":"file","filename":"/tmp/copy1.qcow2","node-name":"libvirt-7-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-7-format","read-only":true,"driver":"qcow2","file":"libvirt-7-storage","backing":"libvirt-8-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/copy2.qcow2","node-name":"libvirt-6-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-6-format","read-only":true,"driver":"qcow2","file":"libvirt-6-storage","backing":"libvirt-7-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/copy3.qcow2","node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-5-format","read-only":true,"driver":"qcow2","file":"libvirt-5-storage","backing":"libvirt-6-format"}' \
-blockdev '{"driver":"file","filename":"/tmp/copy4.qcow2","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2","file":"libvirt-1-storage","backing":"libvirt-5-format"}' \
-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0xd,drive=libvirt-1-format,id=virtio-disk2 \
-netdev tap,fd=31,id=hostnet0,vhost=on,vhostfd=32 \
-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:36:bd:3b,bus=pci.0,addr=0x3 \
-netdev tap,fd=33,id=hostnet1 \
-device rtl8139,netdev=hostnet1,id=net1,mac=52:54:00:2e:f1:27,bus=pci.0,addr=0x8 \
-chardev pty,id=charserial0 \
-device isa-serial,chardev=charserial0,id=serial0 \
-chardev socket,id=charchannel0,fd=34,server,nowait \
-device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 \
-chardev spicevmc,id=charchannel1,name=vdagent \
-device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,id=channel1,name=com.redhat.spice.0 \
-device usb-tablet,id=input0,bus=usb.0,port=1 \
-spice port=5900,addr=127.0.0.1,disable-ticketing,image-compression=off,seamless-migration=on \
-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2 \
-device intel-hda,id=sound0,bus=pci.0,addr=0x4 \
-device hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 \
-chardev spicevmc,id=charredir0,name=usbredir \
-device usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=2 \
-chardev spicevmc,id=charredir1,name=usbredir \
-device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=3 \
-object rng-random,id=objrng0,filename=/dev/random \
-device virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.0,addr=0x7 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
-msg timestamp=on
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only
2019-03-08 15:37 ` [Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only Kevin Wolf
@ 2019-03-12 2:46 ` Eric Blake
0 siblings, 0 replies; 12+ messages in thread
From: Eric Blake @ 2019-03-12 2:46 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, pkrempa, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]
On 3/8/19 9:37 AM, Kevin Wolf wrote:
> tests/virtio-blk-test uses a temporary image file that it deletes while
> QEMU is still running, so it can't be reopened when writers are
> attached or detached. Disable auto-read-only to keep it always writable.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/virtio-blk-test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
>
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 8d2fc9c710..0e464aeea4 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -72,7 +72,7 @@ static QOSState *pci_test_start(void)
> QOSState *qs;
> const char *arch = qtest_get_arch();
> char *tmp_path;
> - const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
> + const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw,auto-read-only=off "
> "-drive if=none,id=drive1,file=null-co://,format=raw "
> "-device virtio-blk-pci,id=drv0,drive=drive0,"
> "addr=%x.%x";
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-03-12 2:51 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08 15:37 [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only Kevin Wolf
2019-03-12 2:46 ` Eric Blake
2019-03-08 15:37 ` [Qemu-devel] [PATCH 2/8] block: Avoid useless local_err Kevin Wolf
2019-03-11 10:16 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2019-03-08 15:37 ` [Qemu-devel] [PATCH 3/8] block: Make permission changes in reopen less wrong Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 4/8] file-posix: Factor out raw_reconfigure_getfd() Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 5/8] file-posix: Store BDRVRawState.reopen_state during reopen Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 6/8] file-posix: Lock new fd in raw_reopen_prepare() Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 7/8] file-posix: Prepare permission code for fd switching Kevin Wolf
2019-03-08 15:37 ` [Qemu-devel] [PATCH 8/8] file-posix: Make auto-read-only dynamic Kevin Wolf
2019-03-11 13:09 ` [Qemu-devel] [PATCH 0/8] " Peter Krempa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.