[Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic

[Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic

[Kevin Wolf]

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

[Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only

[Kevin Wolf]

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

[Qemu-devel] [PATCH 2/8] block: Avoid useless local_err

[Kevin Wolf]

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

[Qemu-devel] [PATCH 3/8] block: Make permission changes in reopen less wrong

[Kevin Wolf]

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

[Qemu-devel] [PATCH 4/8] file-posix: Factor out raw_reconfigure_getfd()

[Kevin Wolf]

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

[Qemu-devel] [PATCH 5/8] file-posix: Store BDRVRawState.reopen_state during reopen

[Kevin Wolf]

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

[Qemu-devel] [PATCH 6/8] file-posix: Lock new fd in raw_reopen_prepare()

[Kevin Wolf]

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

[Qemu-devel] [PATCH 7/8] file-posix: Prepare permission code for fd switching

[Kevin Wolf]

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

[Qemu-devel] [PATCH 8/8] file-posix: Make auto-read-only dynamic

[Kevin Wolf]

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

Re: [Qemu-devel] [Qemu-block] [PATCH 2/8] block: Avoid useless local_err

[Alberto Garcia]

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

Re: [Qemu-devel] [PATCH 0/8] file-posix: Make auto-read-only dynamic

[Peter Krempa]

[-- 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 --]

Re: [Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only

[Eric Blake]

[-- 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 --]