* [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 @ 2016-03-14 15:44 Kevin Wolf 2016-03-14 15:44 ` [Qemu-devel] [PATCH 1/5] block: Remove copy-on-read from bdrv_move_feature_fields() Kevin Wolf ` (5 more replies) 0 siblings, 6 replies; 13+ messages in thread From: Kevin Wolf @ 2016-03-14 15:44 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, qemu-devel, armbru, mreitz, jsnow This series contains those part of my current work that I want to be in the 2.6 release because they change the semantics of some QMP interfaces. This is especially true for the WCE changes, which concern things that weren't even accessible in 2.5 yet, so we're still completely free to change them until 2.6. The other two parts have existed for a bit longer, but the changes can reasonably be considered bug fixes. Kevin Wolf (5): block: Remove copy-on-read from bdrv_move_feature_fields() block: Remove dirty bitmaps from bdrv_move_feature_fields() block: Remove cache.writeback from blockdev-add block: Make backing files always writeback block: Reject writethrough mode except at the root block.c | 17 ++++--- blockdev.c | 26 +++++++++-- qapi/block-core.json | 4 +- tests/qemu-iotests/142 | 50 ++++++++++----------- tests/qemu-iotests/142.out | 108 ++++++++++----------------------------------- 5 files changed, 82 insertions(+), 123 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 1/5] block: Remove copy-on-read from bdrv_move_feature_fields() 2016-03-14 15:44 [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf @ 2016-03-14 15:44 ` Kevin Wolf 2016-03-14 15:51 ` Eric Blake 2016-03-14 15:44 ` [Qemu-devel] [PATCH 2/5] block: Remove dirty bitmaps " Kevin Wolf ` (4 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Kevin Wolf @ 2016-03-14 15:44 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, qemu-devel, armbru, mreitz, jsnow Ever since we first introduced bdrv_append() in commit 8802d1fd ('qapi: Introduce blockdev-group-snapshot-sync command'), the copy-on-read flag was moved to the new top layer when taking a snapshot. The only problem is that it doesn't make a whole lot of sense. The use case for manually enabled CoR is to avoid reading data twice from a slow remote image, so we want to save it to a local overlay, say an ISO image accessed via HTTP to a local qcow2 overlay. When taking a snapshot, we end up with a backing chain like this: http <- local.qcow2 <- snap_overlay.qcow2 There is no point in doing CoR from local.qcow2 into snap_overlay.qcow2, we just want to keep copying data from the remote source into local.qcow2. The other use case of CoR is in the context of streaming, which isn't very interesting for bdrv_move_feature_fields() because op blockers prevent this combination. This patch makes the copy-on-read flag stay on the image for which it was originally set and prevents it from being propagated to the new overlay. It is no longer intended to move CoR to the BlockBackend level. In order for this to make sense, we also need to keep the respective image read-write. As a side effect of these changes, creating a live snapshot image (as opposed to using an existing externally created one) on top of a COR block device works now. It used to fail because it tried to open its backing file both read-only and with COR. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 2 -- blockdev.c | 7 +++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/block.c b/block.c index cf5eb34..a75c4b3 100644 --- a/block.c +++ b/block.c @@ -2283,8 +2283,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, /* move some fields that need to stay attached to the device */ /* dev info */ - bs_dest->copy_on_read = bs_src->copy_on_read; - bs_dest->enable_write_cache = bs_src->enable_write_cache; /* dirty bitmap */ diff --git a/blockdev.c b/blockdev.c index 322ca03..a22b444 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1728,6 +1728,7 @@ static void external_snapshot_prepare(BlkActionState *common, } flags = state->old_bs->open_flags; + flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ); /* create new image w/backing file */ mode = s->has_mode ? s->mode : NEW_IMAGE_MODE_ABSOLUTE_PATHS; @@ -1798,8 +1799,10 @@ static void external_snapshot_commit(BlkActionState *common) /* We don't need (or want) to use the transactional * bdrv_reopen_multiple() across all the entries at once, because we * don't want to abort all of them if one of them fails the reopen */ - bdrv_reopen(state->old_bs, state->old_bs->open_flags & ~BDRV_O_RDWR, - NULL); + if (!state->old_bs->copy_on_read) { + bdrv_reopen(state->old_bs, state->old_bs->open_flags & ~BDRV_O_RDWR, + NULL); + } } static void external_snapshot_abort(BlkActionState *common) -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/5] block: Remove copy-on-read from bdrv_move_feature_fields() 2016-03-14 15:44 ` [Qemu-devel] [PATCH 1/5] block: Remove copy-on-read from bdrv_move_feature_fields() Kevin Wolf @ 2016-03-14 15:51 ` Eric Blake 0 siblings, 0 replies; 13+ messages in thread From: Eric Blake @ 2016-03-14 15:51 UTC (permalink / raw) To: Kevin Wolf, qemu-block; +Cc: qemu-devel, jsnow, armbru, mreitz [-- Attachment #1: Type: text/plain, Size: 1924 bytes --] On 03/14/2016 09:44 AM, Kevin Wolf wrote: > Ever since we first introduced bdrv_append() in commit 8802d1fd ('qapi: > Introduce blockdev-group-snapshot-sync command'), the copy-on-read flag > was moved to the new top layer when taking a snapshot. The only problem > is that it doesn't make a whole lot of sense. > > The use case for manually enabled CoR is to avoid reading data twice > from a slow remote image, so we want to save it to a local overlay, say > an ISO image accessed via HTTP to a local qcow2 overlay. When taking a > snapshot, we end up with a backing chain like this: > > http <- local.qcow2 <- snap_overlay.qcow2 > > There is no point in doing CoR from local.qcow2 into snap_overlay.qcow2, > we just want to keep copying data from the remote source into > local.qcow2. > > The other use case of CoR is in the context of streaming, which isn't > very interesting for bdrv_move_feature_fields() because op blockers > prevent this combination. Your arguments make sense. > > This patch makes the copy-on-read flag stay on the image for which it > was originally set and prevents it from being propagated to the new > overlay. It is no longer intended to move CoR to the BlockBackend level. > In order for this to make sense, we also need to keep the respective > image read-write. > > As a side effect of these changes, creating a live snapshot image (as > opposed to using an existing externally created one) on top of a COR > block device works now. It used to fail because it tried to open its > backing file both read-only and with COR. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 2 -- > blockdev.c | 7 +++++-- > 2 files changed, 5 insertions(+), 4 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/5] block: Remove dirty bitmaps from bdrv_move_feature_fields() 2016-03-14 15:44 [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf 2016-03-14 15:44 ` [Qemu-devel] [PATCH 1/5] block: Remove copy-on-read from bdrv_move_feature_fields() Kevin Wolf @ 2016-03-14 15:44 ` Kevin Wolf 2016-03-14 15:55 ` Eric Blake 2016-03-14 15:44 ` [Qemu-devel] [PATCH 3/5] block: Remove cache.writeback from blockdev-add Kevin Wolf ` (3 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Kevin Wolf @ 2016-03-14 15:44 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, qemu-devel, armbru, mreitz, jsnow This patch changes dirty bitmaps from following a BlockBackend in graph changes to sticking with the node they were created at. For the full discussion, read the following mailing list thread: [Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields() https://lists.nongnu.org/archive/html/qemu-block/2016-02/msg00745.html In summary, the justification for this change is: * When moving the dirty bitmap to the top of the tree was introduced in bdrv_append() in commit a9fc4408, it didn't actually have any effect because there could never be a bitmap in use when bdrv_append() was called (op blockers would prevent this). This is still true today for all internal uses of dirty bitmaps. * Support for user-defined dirty bitmaps was introduced in 2.4, but we discouraged users from using it because we didn't consider it ready yet. Moreover, in 2.5, the bdrv_swap() removal introduced a bug that left dangling pointers if a dirty bitmap was present (the anchors of the dirty bitmap were swapped, but the back link in the first element wasn't updated), so it didn't even work correctly. * block-dirty-bitmap-add takes an arbitrary node name, even if no BlockBackend is attached. This suggests that it is a node level operation and not a BlockBackend one. Consequently, there is no reason for dirty bitmaps to stay with a BlockBackend that was attached to the node they were created for. * It was suggested that block-dirty-bitmap-add could track the node if a node name was specified, and track the BlockBackend if the device name was specified. This would however be inconsistent with other QMP commands. Commands that accept both device and node names currently interpret the device name just as an alias for the current root node of that BlockBackend. * Dirty bitmaps have a name that is only unique amongst the bitmaps in a specific node. Moving bitmaps could lead to name clashes. Automatic renaming would involve too much magic. * Persistent bitmaps are stored in a specific node. Moving them around automatically might be at least surprising, but it would probably also become a real problem because that would have to happen atomically without the management tool knowing of the operation. At the end of the day it seems to be very clear that it was a mistake to include dirty bitmaps in bdrv_move_feature_fields(). The functionality of moving bitmaps and/or attaching them to a BlockBackend instead will probably be needed, but it should be done with a new explicit QMP command or option. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/block.c b/block.c index a75c4b3..698e2c7 100644 --- a/block.c +++ b/block.c @@ -2284,9 +2284,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, /* dev info */ bs_dest->enable_write_cache = bs_src->enable_write_cache; - - /* dirty bitmap */ - bs_dest->dirty_bitmaps = bs_src->dirty_bitmaps; } static void change_parent_backing_link(BlockDriverState *from, -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] block: Remove dirty bitmaps from bdrv_move_feature_fields() 2016-03-14 15:44 ` [Qemu-devel] [PATCH 2/5] block: Remove dirty bitmaps " Kevin Wolf @ 2016-03-14 15:55 ` Eric Blake 0 siblings, 0 replies; 13+ messages in thread From: Eric Blake @ 2016-03-14 15:55 UTC (permalink / raw) To: Kevin Wolf, qemu-block; +Cc: qemu-devel, jsnow, armbru, mreitz [-- Attachment #1: Type: text/plain, Size: 1635 bytes --] On 03/14/2016 09:44 AM, Kevin Wolf wrote: > This patch changes dirty bitmaps from following a BlockBackend in graph > changes to sticking with the node they were created at. For the full > discussion, read the following mailing list thread: > > [Qemu-block] block: Dirty bitmaps and COR in bdrv_move_feature_fields() > https://lists.nongnu.org/archive/html/qemu-block/2016-02/msg00745.html > > In summary, the justification for this change is: [snip nice list] > > At the end of the day it seems to be very clear that it was a mistake to > include dirty bitmaps in bdrv_move_feature_fields(). The functionality > of moving bitmaps and/or attaching them to a BlockBackend instead will > probably be needed, but it should be done with a new explicit QMP > command or option. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 3 --- > 1 file changed, 3 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> [Always interesting when a patch is way shorter than the justification for why we need the patch] > > diff --git a/block.c b/block.c > index a75c4b3..698e2c7 100644 > --- a/block.c > +++ b/block.c > @@ -2284,9 +2284,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, > > /* dev info */ > bs_dest->enable_write_cache = bs_src->enable_write_cache; > - > - /* dirty bitmap */ > - bs_dest->dirty_bitmaps = bs_src->dirty_bitmaps; > } > > static void change_parent_backing_link(BlockDriverState *from, > -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 3/5] block: Remove cache.writeback from blockdev-add 2016-03-14 15:44 [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf 2016-03-14 15:44 ` [Qemu-devel] [PATCH 1/5] block: Remove copy-on-read from bdrv_move_feature_fields() Kevin Wolf 2016-03-14 15:44 ` [Qemu-devel] [PATCH 2/5] block: Remove dirty bitmaps " Kevin Wolf @ 2016-03-14 15:44 ` Kevin Wolf 2016-03-14 16:10 ` Eric Blake 2016-03-14 15:44 ` [Qemu-devel] [PATCH 4/5] block: Make backing files always writeback Kevin Wolf ` (2 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Kevin Wolf @ 2016-03-14 15:44 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, qemu-devel, armbru, mreitz, jsnow The WCE bit is a frontend property and should not be part of the backend configuration. This is especially important because the same BDS can be used by different users with different WCE requirements. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- qapi/block-core.json | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 9bf1b22..e3617e2 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1614,7 +1614,6 @@ # # Includes cache-related options for block devices # -# @writeback: #optional enables writeback mode for any caches (default: true) # @direct: #optional enables use of O_DIRECT (bypass the host page cache; # default: false) # @no-flush: #optional ignore any flush requests for the device (default: @@ -1623,8 +1622,7 @@ # Since: 1.7 ## { 'struct': 'BlockdevCacheOptions', - 'data': { '*writeback': 'bool', - '*direct': 'bool', + 'data': { '*direct': 'bool', '*no-flush': 'bool' } } ## -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] block: Remove cache.writeback from blockdev-add 2016-03-14 15:44 ` [Qemu-devel] [PATCH 3/5] block: Remove cache.writeback from blockdev-add Kevin Wolf @ 2016-03-14 16:10 ` Eric Blake 2016-03-14 16:22 ` Kevin Wolf 0 siblings, 1 reply; 13+ messages in thread From: Eric Blake @ 2016-03-14 16:10 UTC (permalink / raw) To: Kevin Wolf, qemu-block; +Cc: qemu-devel, jsnow, armbru, mreitz [-- Attachment #1: Type: text/plain, Size: 1610 bytes --] On 03/14/2016 09:44 AM, Kevin Wolf wrote: > The WCE bit is a frontend property and should not be part of the backend > configuration. This is especially important because the same BDS can be > used by different users with different WCE requirements. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qapi/block-core.json | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 9bf1b22..e3617e2 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1614,7 +1614,6 @@ > # > # Includes cache-related options for block devices > # > -# @writeback: #optional enables writeback mode for any caches (default: true) > # @direct: #optional enables use of O_DIRECT (bypass the host page cache; > # default: false) > # @no-flush: #optional ignore any flush requests for the device (default: > @@ -1623,8 +1622,7 @@ > # Since: 1.7 > ## > { 'struct': 'BlockdevCacheOptions', > - 'data': { '*writeback': 'bool', > - '*direct': 'bool', > + 'data': { '*direct': 'bool', > '*no-flush': 'bool' } } Observable through introspection. Not quite backwards-compatible, but at least clients can learn about it, and arguably clients shouldn't have been using it. I can accept it as a bug fix, even though it does risk breaking old clients that were trying to use it. If it helps, libvirt does not seem to have been using it. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/5] block: Remove cache.writeback from blockdev-add 2016-03-14 16:10 ` Eric Blake @ 2016-03-14 16:22 ` Kevin Wolf 0 siblings, 0 replies; 13+ messages in thread From: Kevin Wolf @ 2016-03-14 16:22 UTC (permalink / raw) To: Eric Blake; +Cc: qemu-devel, jsnow, armbru, qemu-block, mreitz [-- Attachment #1: Type: text/plain, Size: 1882 bytes --] Am 14.03.2016 um 17:10 hat Eric Blake geschrieben: > On 03/14/2016 09:44 AM, Kevin Wolf wrote: > > The WCE bit is a frontend property and should not be part of the backend > > configuration. This is especially important because the same BDS can be > > used by different users with different WCE requirements. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > qapi/block-core.json | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index 9bf1b22..e3617e2 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -1614,7 +1614,6 @@ > > # > > # Includes cache-related options for block devices > > # > > -# @writeback: #optional enables writeback mode for any caches (default: true) > > # @direct: #optional enables use of O_DIRECT (bypass the host page cache; > > # default: false) > > # @no-flush: #optional ignore any flush requests for the device (default: > > @@ -1623,8 +1622,7 @@ > > # Since: 1.7 > > ## > > { 'struct': 'BlockdevCacheOptions', > > - 'data': { '*writeback': 'bool', > > - '*direct': 'bool', > > + 'data': { '*direct': 'bool', > > '*no-flush': 'bool' } } > > Observable through introspection. Not quite backwards-compatible, but > at least clients can learn about it, and arguably clients shouldn't have > been using it. I can accept it as a bug fix, even though it does risk > breaking old clients that were trying to use it. > > If it helps, libvirt does not seem to have been using it. I think we declared that blockdev-add is still experimental, so it should be okay. And I was actually pondering whether this struct even makes sense any more or whether we should move 'direct' and 'no-flush' directly to the parent struct. Kevin [-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 4/5] block: Make backing files always writeback 2016-03-14 15:44 [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf ` (2 preceding siblings ...) 2016-03-14 15:44 ` [Qemu-devel] [PATCH 3/5] block: Remove cache.writeback from blockdev-add Kevin Wolf @ 2016-03-14 15:44 ` Kevin Wolf 2016-03-14 16:46 ` Eric Blake 2016-03-14 15:44 ` [Qemu-devel] [PATCH 5/5] block: Reject writethrough mode except at the root Kevin Wolf 2016-03-18 17:10 ` [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf 5 siblings, 1 reply; 13+ messages in thread From: Kevin Wolf @ 2016-03-14 15:44 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, qemu-devel, armbru, mreitz, jsnow First of all, we're generally not writing to backing files, but when we do, it's in the context of block jobs which know very well when to flush the image. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 5 +++-- tests/qemu-iotests/142.out | 10 +++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index 698e2c7..666fe61 100644 --- a/block.c +++ b/block.c @@ -762,8 +762,9 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options, { int flags = parent_flags; - /* The cache mode is inherited unmodified for backing files */ - qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_WB); + /* The cache mode is inherited unmodified for backing files; except WCE, + * which is only applied on the top level (BlockBackend) */ + qdict_set_default_str(child_options, BDRV_OPT_CACHE_WB, "on"); qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT); qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH); diff --git a/tests/qemu-iotests/142.out b/tests/qemu-iotests/142.out index b555d5a..abe94c3 100644 --- a/tests/qemu-iotests/142.out +++ b/tests/qemu-iotests/142.out @@ -62,7 +62,7 @@ cache.direct=on on backing-file cache.writeback=off on none0 Cache mode: writethrough Cache mode: writeback - Cache mode: writethrough + Cache mode: writeback Cache mode: writeback cache.writeback=off on file @@ -143,7 +143,7 @@ cache.writeback=off on none0 Cache mode: writethrough Cache mode: writethrough Cache mode: writeback - Cache mode: writethrough + Cache mode: writeback Cache mode: writeback cache.writeback=off on file @@ -302,7 +302,7 @@ cache.direct=on on backing-file cache.writeback=off on none0 Cache mode: writethrough, direct Cache mode: writeback, direct - Cache mode: writethrough, direct + Cache mode: writeback, direct Cache mode: writeback, direct cache.writeback=off on file @@ -383,7 +383,7 @@ cache.writeback=off on none0 Cache mode: writeback, direct Cache mode: writethrough Cache mode: writeback - Cache mode: writethrough + Cache mode: writeback Cache mode: writeback cache.writeback=off on file @@ -718,7 +718,7 @@ cache.direct=on on backing-file cache.writeback=off on none0 Cache mode: writethrough Cache mode: writeback - Cache mode: writethrough, direct + Cache mode: writeback, direct Cache mode: writeback, direct cache.writeback=off on file -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 4/5] block: Make backing files always writeback 2016-03-14 15:44 ` [Qemu-devel] [PATCH 4/5] block: Make backing files always writeback Kevin Wolf @ 2016-03-14 16:46 ` Eric Blake 0 siblings, 0 replies; 13+ messages in thread From: Eric Blake @ 2016-03-14 16:46 UTC (permalink / raw) To: Kevin Wolf, qemu-block; +Cc: qemu-devel, jsnow, armbru, mreitz [-- Attachment #1: Type: text/plain, Size: 571 bytes --] On 03/14/2016 09:44 AM, Kevin Wolf wrote: > First of all, we're generally not writing to backing files, but when we > do, it's in the context of block jobs which know very well when to flush > the image. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 5 +++-- > tests/qemu-iotests/142.out | 10 +++++----- > 2 files changed, 8 insertions(+), 7 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 5/5] block: Reject writethrough mode except at the root 2016-03-14 15:44 [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf ` (3 preceding siblings ...) 2016-03-14 15:44 ` [Qemu-devel] [PATCH 4/5] block: Make backing files always writeback Kevin Wolf @ 2016-03-14 15:44 ` Kevin Wolf 2016-03-14 16:50 ` Eric Blake 2016-03-18 17:10 ` [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf 5 siblings, 1 reply; 13+ messages in thread From: Kevin Wolf @ 2016-03-14 15:44 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, qemu-devel, armbru, mreitz, jsnow Writethrough mode is going to become a BlockBackend feature rather than a BDS one, so forbid it in places where we won't be able to support it when the code finally matches the envisioned design. We only allowed setting the cache mode of non-root nodes after the 2.5 release, so we're still free to make this change. The target of block jobs is now always opened in a writeback mode because it doesn't have a BlockBackend attached. This makes more sense anyway because block jobs know when to flush. If the graph is modified on job completion, the original cache mode moves to the new root, so for the guest device writethough always stays enabled if it was configured this way. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 7 ++++ blockdev.c | 19 ++++++++- tests/qemu-iotests/142 | 50 +++++++++++------------ tests/qemu-iotests/142.out | 98 +++++++++------------------------------------- 4 files changed, 68 insertions(+), 106 deletions(-) diff --git a/block.c b/block.c index 666fe61..94b8894 100644 --- a/block.c +++ b/block.c @@ -999,6 +999,13 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, /* Apply cache mode options */ update_flags_from_options(&bs->open_flags, opts); + + if (!bs->blk && (bs->open_flags & BDRV_O_CACHE_WB) == 0) { + error_setg(errp, "Can't set writethrough mode except for the root"); + ret = -EINVAL; + goto free_and_fail; + } + bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB); /* Open the image, either directly or using a protocol */ diff --git a/blockdev.c b/blockdev.c index a22b444..9f36087 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1758,6 +1758,12 @@ static void external_snapshot_prepare(BlkActionState *common, flags |= BDRV_O_NO_BACKING; } + /* There is no BB attached during bdrv_open(), so we can't set a + * writethrough mode. bdrv_append() will swap the WCE setting so that the + * backing file becomes unconditionally writeback (which is what backing + * files should always be) and the new overlay gets the original setting. */ + flags |= BDRV_O_CACHE_WB; + assert(state->new_bs == NULL); ret = bdrv_open(&state->new_bs, new_image_file, snapshot_ref, options, flags, errp); @@ -2509,6 +2515,7 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, BlockBackend *blk; BlockDriverState *medium_bs = NULL; int bdrv_flags, ret; + bool writethrough; QDict *options = NULL; Error *err = NULL; @@ -2527,6 +2534,12 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, bdrv_flags &= ~(BDRV_O_TEMPORARY | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_PROTOCOL); + /* Must open the image in writeback mode as long as no BlockBackend is + * attached. The right mode can be set as the final step after changing the + * medium. */ + writethrough = !(bdrv_flags & BDRV_O_CACHE_WB); + bdrv_flags |= BDRV_O_CACHE_WB; + if (!has_read_only) { read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN; } @@ -2584,6 +2597,8 @@ void qmp_blockdev_change_medium(const char *device, const char *filename, goto fail; } + bdrv_set_enable_write_cache(medium_bs, !writethrough); + qmp_blockdev_close_tray(device, errp); fail: @@ -3209,7 +3224,7 @@ static void do_drive_backup(const char *device, const char *target, goto out; } - flags = bs->open_flags | BDRV_O_RDWR; + flags = bs->open_flags | BDRV_O_CACHE_WB | BDRV_O_RDWR; /* See if we have a backing HD we can use to create our new image * on top of. */ @@ -3504,7 +3519,7 @@ void qmp_drive_mirror(const char *device, const char *target, format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name; } - flags = bs->open_flags | BDRV_O_RDWR; + flags = bs->open_flags | BDRV_O_CACHE_WB | BDRV_O_RDWR; source = backing_bs(bs); if (!source && sync == MIRROR_SYNC_MODE_TOP) { sync = MIRROR_SYNC_MODE_FULL; diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142 index 8aa50f8..80834b5 100755 --- a/tests/qemu-iotests/142 +++ b/tests/qemu-iotests/142 @@ -96,36 +96,36 @@ function check_cache_all() # bs->backing echo -e "cache.direct=on on none0" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.direct=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.direct=on | grep -e "Cache" -e "[Cc]annot|[Cc]ould not|[Cc]an't" echo -e "\ncache.direct=on on file" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.direct=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.direct=on | grep -e "Cache" -e "[Cc]annot|[Cc]ould not|[Cc]an't" echo -e "\ncache.direct=on on backing" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.direct=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.direct=on | grep -e "Cache" -e "[Cc]annot|[Cc]ould not|[Cc]an't" echo -e "\ncache.direct=on on backing-file" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.direct=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.direct=on | grep -e "Cache" -e "[Cc]annot|[Cc]ould not|[Cc]an't" # cache.writeback is supposed to be inherited by bs->backing; bs->file # always gets cache.writeback=on echo -e "\n\ncache.writeback=off on none0" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.writeback=off | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.writeback=off on file" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.writeback=off | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.writeback=off on backing" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.writeback=off | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.writeback=off on backing-file" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.writeback=off | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" # cache.no-flush is supposed to be inherited by both bs->file and bs->backing echo -e "\n\ncache.no-flush=on on none0" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.no-flush=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.no-flush=on | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.no-flush=on on file" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.no-flush=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.no-flush=on | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.no-flush=on on backing" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.no-flush=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.no-flush=on | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.no-flush=on on backing-file" - echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.no-flush=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.no-flush=on | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" } echo @@ -218,7 +218,7 @@ info block image info block blkdebug info block file" -echo "$hmp_cmds" | run_qemu -drive if=none,file="blkdebug::json:{\"filename\":\"$TEST_IMG\",,\"cache\":{\"writeback\":false,,\"direct\":false}}",node-name=image,file.node-name=blkdebug,file.image.node-name=file | grep "Cache" +echo "$hmp_cmds" | run_qemu -drive if=none,file="blkdebug::json:{\"filename\":\"$TEST_IMG\",,\"cache\":{\"direct\":false}}",node-name=image,file.node-name=blkdebug,file.image.node-name=file | grep "Cache" echo echo "=== Check that referenced BDSes don't inherit ===" @@ -234,35 +234,35 @@ function check_cache_all_separate() # Check cache.direct echo -e "cache.direct=on on blk" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img",cache.direct=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img",cache.direct=on | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.direct=on on file" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file",cache.direct=on -drive "$drv_img" | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file",cache.direct=on -drive "$drv_img" | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.direct=on on backing" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk",cache.direct=on -drive "$drv_file" -drive "$drv_img" | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk",cache.direct=on -drive "$drv_file" -drive "$drv_img" | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.direct=on on backing-file" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.direct=on -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.direct=on -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" # Check cache.writeback echo -e "\n\ncache.writeback=off on blk" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img",cache.writeback=off | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img",cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.writeback=off on file" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file",cache.writeback=off -drive "$drv_img" | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file",cache.writeback=off -drive "$drv_img" | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.writeback=off on backing" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk",cache.writeback=off -drive "$drv_file" -drive "$drv_img" | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk",cache.writeback=off -drive "$drv_file" -drive "$drv_img" | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.writeback=off on backing-file" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.writeback=off -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.writeback=off -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" # Check cache.no-flush echo -e "\n\ncache.no-flush=on on blk" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img",cache.no-flush=on | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img",cache.no-flush=on | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.no-flush=on on file" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file",cache.no-flush=on -drive "$drv_img" | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive "$drv_file",cache.no-flush=on -drive "$drv_img" | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.no-flush=on on backing" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk",cache.no-flush=on -drive "$drv_file" -drive "$drv_img" | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk",cache.no-flush=on -drive "$drv_file" -drive "$drv_img" | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" echo -e "\ncache.no-flush=on on backing-file" - echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.no-flush=on -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep "Cache" + echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.no-flush=on -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't" } echo diff --git a/tests/qemu-iotests/142.out b/tests/qemu-iotests/142.out index abe94c3..5dd5bd0 100644 --- a/tests/qemu-iotests/142.out +++ b/tests/qemu-iotests/142.out @@ -66,22 +66,13 @@ cache.writeback=off on none0 Cache mode: writeback cache.writeback=off on file - Cache mode: writeback - Cache mode: writethrough - Cache mode: writeback - Cache mode: writeback +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root cache.writeback=off on backing - Cache mode: writeback - Cache mode: writeback - Cache mode: writethrough - Cache mode: writeback +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.writeback=off on backing-file - Cache mode: writeback - Cache mode: writeback - Cache mode: writeback - Cache mode: writethrough +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.no-flush=on on none0 @@ -147,25 +138,13 @@ cache.writeback=off on none0 Cache mode: writeback cache.writeback=off on file - Cache mode: writeback - Cache mode: writeback - Cache mode: writethrough - Cache mode: writeback - Cache mode: writeback +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root cache.writeback=off on backing - Cache mode: writeback - Cache mode: writeback - Cache mode: writeback - Cache mode: writethrough - Cache mode: writeback +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.writeback=off on backing-file - Cache mode: writeback - Cache mode: writeback - Cache mode: writeback - Cache mode: writeback - Cache mode: writethrough +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.no-flush=on on none0 @@ -230,22 +209,13 @@ cache.writeback=off on none0 Cache mode: writeback, direct cache.writeback=off on file - Cache mode: writeback, direct - Cache mode: writethrough, direct - Cache mode: writeback, direct - Cache mode: writeback, direct +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root cache.writeback=off on backing - Cache mode: writeback, direct - Cache mode: writeback, direct - Cache mode: writethrough, direct - Cache mode: writeback, direct +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.writeback=off on backing-file - Cache mode: writeback, direct - Cache mode: writeback, direct - Cache mode: writeback, direct - Cache mode: writethrough, direct +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.no-flush=on on none0 @@ -306,22 +276,13 @@ cache.writeback=off on none0 Cache mode: writeback, direct cache.writeback=off on file - Cache mode: writeback, direct - Cache mode: writethrough, direct - Cache mode: writeback, direct - Cache mode: writeback, direct +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root cache.writeback=off on backing - Cache mode: writeback, direct - Cache mode: writeback, direct - Cache mode: writethrough, direct - Cache mode: writeback, direct +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.writeback=off on backing-file - Cache mode: writeback, direct - Cache mode: writeback, direct - Cache mode: writeback, direct - Cache mode: writethrough, direct +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.no-flush=on on none0 @@ -387,25 +348,13 @@ cache.writeback=off on none0 Cache mode: writeback cache.writeback=off on file - Cache mode: writeback, direct - Cache mode: writeback - Cache mode: writethrough - Cache mode: writeback - Cache mode: writeback +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root cache.writeback=off on backing - Cache mode: writeback, direct - Cache mode: writeback - Cache mode: writeback - Cache mode: writethrough - Cache mode: writeback +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.writeback=off on backing-file - Cache mode: writeback, direct - Cache mode: writeback - Cache mode: writeback - Cache mode: writeback - Cache mode: writethrough +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.no-flush=on on none0 @@ -440,7 +389,7 @@ cache.no-flush=on on backing-file Cache mode: writethrough, direct, ignore flushes Cache mode: writeback, direct, ignore flushes - Cache mode: writethrough, ignore flushes + Cache mode: writeback, ignore flushes === Check that referenced BDSes don't inherit === @@ -722,22 +671,13 @@ cache.writeback=off on none0 Cache mode: writeback, direct cache.writeback=off on file - Cache mode: writeback - Cache mode: writethrough - Cache mode: writeback, direct - Cache mode: writeback, direct +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root cache.writeback=off on backing - Cache mode: writeback - Cache mode: writeback - Cache mode: writethrough, direct - Cache mode: writeback, direct +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.writeback=off on backing-file - Cache mode: writeback - Cache mode: writeback - Cache mode: writeback, direct - Cache mode: writethrough, direct +QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root cache.no-flush=on on none0 -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 5/5] block: Reject writethrough mode except at the root 2016-03-14 15:44 ` [Qemu-devel] [PATCH 5/5] block: Reject writethrough mode except at the root Kevin Wolf @ 2016-03-14 16:50 ` Eric Blake 0 siblings, 0 replies; 13+ messages in thread From: Eric Blake @ 2016-03-14 16:50 UTC (permalink / raw) To: Kevin Wolf, qemu-block; +Cc: qemu-devel, jsnow, armbru, mreitz [-- Attachment #1: Type: text/plain, Size: 1586 bytes --] On 03/14/2016 09:44 AM, Kevin Wolf wrote: > Writethrough mode is going to become a BlockBackend feature rather than > a BDS one, so forbid it in places where we won't be able to support it > when the code finally matches the envisioned design. > > We only allowed setting the cache mode of non-root nodes after the 2.5 > release, so we're still free to make this change. > > The target of block jobs is now always opened in a writeback mode > because it doesn't have a BlockBackend attached. This makes more sense > anyway because block jobs know when to flush. If the graph is modified > on job completion, the original cache mode moves to the new root, so > for the guest device writethough always stays enabled if it was > configured this way. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > block.c | 7 ++++ > blockdev.c | 19 ++++++++- > tests/qemu-iotests/142 | 50 +++++++++++------------ > tests/qemu-iotests/142.out | 98 +++++++++------------------------------------- > 4 files changed, 68 insertions(+), 106 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com> > +++ b/tests/qemu-iotests/142 > @@ -96,36 +96,36 @@ function check_cache_all() > # bs->backing > > echo -e "cache.direct=on on none0" Pre-existing, but 'echo -e' is non-portable (even in bash, you can set options such that it changes behavior). printf is better, if we're worried about it. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 2016-03-14 15:44 [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf ` (4 preceding siblings ...) 2016-03-14 15:44 ` [Qemu-devel] [PATCH 5/5] block: Reject writethrough mode except at the root Kevin Wolf @ 2016-03-18 17:10 ` Kevin Wolf 5 siblings, 0 replies; 13+ messages in thread From: Kevin Wolf @ 2016-03-18 17:10 UTC (permalink / raw) To: qemu-block; +Cc: qemu-devel, jsnow, armbru, mreitz Am 14.03.2016 um 16:44 hat Kevin Wolf geschrieben: > This series contains those part of my current work that I want to be in > the 2.6 release because they change the semantics of some QMP > interfaces. > > This is especially true for the WCE changes, which concern things that > weren't even accessible in 2.5 yet, so we're still completely free to > change them until 2.6. The other two parts have existed for a bit > longer, but the changes can reasonably be considered bug fixes. Applied to the block branch. Kevin ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-03-18 17:10 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-14 15:44 [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf 2016-03-14 15:44 ` [Qemu-devel] [PATCH 1/5] block: Remove copy-on-read from bdrv_move_feature_fields() Kevin Wolf 2016-03-14 15:51 ` Eric Blake 2016-03-14 15:44 ` [Qemu-devel] [PATCH 2/5] block: Remove dirty bitmaps " Kevin Wolf 2016-03-14 15:55 ` Eric Blake 2016-03-14 15:44 ` [Qemu-devel] [PATCH 3/5] block: Remove cache.writeback from blockdev-add Kevin Wolf 2016-03-14 16:10 ` Eric Blake 2016-03-14 16:22 ` Kevin Wolf 2016-03-14 15:44 ` [Qemu-devel] [PATCH 4/5] block: Make backing files always writeback Kevin Wolf 2016-03-14 16:46 ` Eric Blake 2016-03-14 15:44 ` [Qemu-devel] [PATCH 5/5] block: Reject writethrough mode except at the root Kevin Wolf 2016-03-14 16:50 ` Eric Blake 2016-03-18 17:10 ` [Qemu-devel] [PATCH 0/5] block: API changes for 2.6 Kevin Wolf
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.