* [Qemu-devel] [PATCH v5 1/3] block: reintroduce bdrv_flush_all
2016-09-23 1:45 [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray John Snow
@ 2016-09-23 1:45 ` John Snow
2016-09-23 1:45 ` [Qemu-devel] [PATCH v5 2/3] qemu: use bdrv_flush_all for vm_stop et al John Snow
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2016-09-23 1:45 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-stable, qemu-devel, mreitz, John Snow
Commit fe1a9cbc moved the flush_all routine from the bdrv layer to the
block-backend layer. In doing so, however, the semantics of the routine
changed slightly such that flush_all now used blk_flush instead of
bdrv_flush.
blk_flush can fail if the attached device model reports that it is not
"available," (i.e. the tray is open.) This changed the semantics of
flush_all such that it can now fail for e.g. open CDROM drives.
Reintroduce bdrv_flush_all to regain the old semantics without having to
alter the behavior of blk_flush or blk_flush_all, which are already
'doing the right thing.'
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 25 +++++++++++++++++++++++++
include/block/block.h | 1 +
2 files changed, 26 insertions(+)
diff --git a/block/io.c b/block/io.c
index fdf7080..57a2eeb 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1619,6 +1619,31 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset,
BDRV_REQ_ZERO_WRITE | flags);
}
+/*
+ * Flush ALL BDSes regardless of if they are reachable via a BlkBackend or not.
+ */
+int bdrv_flush_all(void)
+{
+ BdrvNextIterator it;
+ BlockDriverState *bs = NULL;
+ int result = 0;
+
+ for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
+ AioContext *aio_context = bdrv_get_aio_context(bs);
+ int ret;
+
+ aio_context_acquire(aio_context);
+ ret = bdrv_flush(bs);
+ if (ret < 0 && !result) {
+ result = ret;
+ }
+ aio_context_release(aio_context);
+ }
+
+ return result;
+}
+
+
typedef struct BdrvCoGetBlockStatusData {
BlockDriverState *bs;
BlockDriverState *base;
diff --git a/include/block/block.h b/include/block/block.h
index ffecebf..5e81281 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -332,6 +332,7 @@ int bdrv_inactivate_all(void);
/* Ensure contents are flushed to disk. */
int bdrv_flush(BlockDriverState *bs);
int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
+int bdrv_flush_all(void);
void bdrv_close_all(void);
void bdrv_drain(BlockDriverState *bs);
void coroutine_fn bdrv_co_drain(BlockDriverState *bs);
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH v5 2/3] qemu: use bdrv_flush_all for vm_stop et al
2016-09-23 1:45 [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray John Snow
2016-09-23 1:45 ` [Qemu-devel] [PATCH v5 1/3] block: reintroduce bdrv_flush_all John Snow
@ 2016-09-23 1:45 ` John Snow
2016-09-23 1:45 ` [Qemu-devel] [PATCH v5 3/3] block-backend: remove blk_flush_all John Snow
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2016-09-23 1:45 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-stable, qemu-devel, mreitz, John Snow
Reimplement bdrv_flush_all for vm_stop. In contrast to blk_flush_all,
bdrv_flush_all does not have device model restrictions. This allows
us to flush and halt unconditionally without error.
This allows us to do things like migrate when we have a device with
an open tray, but has a node that may need to be flushed, or nodes
that aren't currently attached to any device and need to be flushed.
Specifically, this allows us to migrate when we have a CDROM with
an open tray.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
cpus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cpus.c b/cpus.c
index e39ccb7..96d9352 100644
--- a/cpus.c
+++ b/cpus.c
@@ -751,7 +751,7 @@ static int do_vm_stop(RunState state)
}
bdrv_drain_all();
- ret = blk_flush_all();
+ ret = bdrv_flush_all();
return ret;
}
@@ -1494,7 +1494,7 @@ int vm_stop_force_state(RunState state)
bdrv_drain_all();
/* Make sure to return an error if the flush in a previous vm_stop()
* failed. */
- return blk_flush_all();
+ return bdrv_flush_all();
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH v5 3/3] block-backend: remove blk_flush_all
2016-09-23 1:45 [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray John Snow
2016-09-23 1:45 ` [Qemu-devel] [PATCH v5 1/3] block: reintroduce bdrv_flush_all John Snow
2016-09-23 1:45 ` [Qemu-devel] [PATCH v5 2/3] qemu: use bdrv_flush_all for vm_stop et al John Snow
@ 2016-09-23 1:45 ` John Snow
2016-09-23 4:11 ` [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray Fam Zheng
2016-09-23 15:35 ` Max Reitz
4 siblings, 0 replies; 8+ messages in thread
From: John Snow @ 2016-09-23 1:45 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-stable, qemu-devel, mreitz, John Snow
We can teach Xen to drain and flush each device as it needs to, instead
of trying to flush ALL devices. This removes the last user of
blk_flush_all.
The function is therefore removed under the premise that any new uses
of blk_flush_all would be the wrong paradigm: either flush the single
device that requires flushing, or use an appropriate flush_all mechanism
from outside of the BlkBackend layer.
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/block-backend.c | 22 ----------------------
hw/i386/xen/xen_platform.c | 2 --
hw/ide/piix.c | 4 ++++
include/sysemu/block-backend.h | 1 -
4 files changed, 4 insertions(+), 25 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index d1349d9..bfb1ddb 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1621,28 +1621,6 @@ int blk_commit_all(void)
return 0;
}
-int blk_flush_all(void)
-{
- BlockBackend *blk = NULL;
- int result = 0;
-
- while ((blk = blk_all_next(blk)) != NULL) {
- AioContext *aio_context = blk_get_aio_context(blk);
- int ret;
-
- aio_context_acquire(aio_context);
- if (blk_is_inserted(blk)) {
- ret = blk_flush(blk);
- if (ret < 0 && !result) {
- result = ret;
- }
- }
- aio_context_release(aio_context);
- }
-
- return result;
-}
-
/* throttling disk I/O limits */
void blk_set_io_limits(BlockBackend *blk, ThrottleConfig *cfg)
diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index aa78393..f85635c 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -134,8 +134,6 @@ static void platform_fixed_ioport_writew(void *opaque, uint32_t addr, uint32_t v
devices, and bit 2 the non-primary-master IDE devices. */
if (val & UNPLUG_ALL_IDE_DISKS) {
DPRINTF("unplug disks\n");
- blk_drain_all();
- blk_flush_all();
pci_unplug_disks(pci_dev->bus);
}
if (val & UNPLUG_ALL_NICS) {
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index c190fca..d5777fd 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -179,6 +179,10 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev)
if (di != NULL && !di->media_cd) {
BlockBackend *blk = blk_by_legacy_dinfo(di);
DeviceState *ds = blk_get_attached_dev(blk);
+
+ blk_drain(blk);
+ blk_flush(blk);
+
if (ds) {
blk_detach_dev(blk, ds);
}
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 4808a96..3d43592 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -150,7 +150,6 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int count);
int blk_co_flush(BlockBackend *blk);
int blk_flush(BlockBackend *blk);
-int blk_flush_all(void);
int blk_commit_all(void);
void blk_drain(BlockBackend *blk);
void blk_drain_all(void);
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray
2016-09-23 1:45 [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray John Snow
` (2 preceding siblings ...)
2016-09-23 1:45 ` [Qemu-devel] [PATCH v5 3/3] block-backend: remove blk_flush_all John Snow
@ 2016-09-23 4:11 ` Fam Zheng
2016-09-23 15:35 ` Max Reitz
4 siblings, 0 replies; 8+ messages in thread
From: Fam Zheng @ 2016-09-23 4:11 UTC (permalink / raw)
To: John Snow; +Cc: qemu-block, kwolf, mreitz, qemu-stable, qemu-devel
On Thu, 09/22 21:45, John Snow wrote:
> When I said "Final re-send," I was lying. Here's a v5.
> The title is also a misnomer by now :)
>
> The move to blk_flush altered the behavior of migration and flushing
> nodes that are not reachable via the guest, but are still reachable
> via QEMU and may or may not need to be flushed.
>
> This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt
> et al being unable to migrate QEMU when the CDROM tray is open.
>
> v5:
> Fix bracket spacing in patch 1. By one space. :(
> Added third patch to remove blk_flush_all.
>
> v4:
> Commit message update.
>
> v3:
> Reworking approach and reinstating bdrv_flush_all at Kevin's suggestion.
>
> ________________________________________________________________________________
>
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch atapi-tray-migfix
> https://github.com/jnsnow/qemu/tree/atapi-tray-migfix
>
> This version is tagged atapi-tray-migfix-v5:
> https://github.com/jnsnow/qemu/releases/tag/atapi-tray-migfix-v5
>
> John Snow (3):
> block: reintroduce bdrv_flush_all
> qemu: use bdrv_flush_all for vm_stop et al
> block-backend: remove blk_flush_all
>
> block/block-backend.c | 22 ----------------------
> block/io.c | 25 +++++++++++++++++++++++++
> cpus.c | 4 ++--
> hw/i386/xen/xen_platform.c | 2 --
> hw/ide/piix.c | 4 ++++
> include/block/block.h | 1 +
> include/sysemu/block-backend.h | 1 -
> 7 files changed, 32 insertions(+), 27 deletions(-)
>
> --
> 2.7.4
>
>
Acked-by: Fam Zheng <famz@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray
2016-09-23 1:45 [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray John Snow
` (3 preceding siblings ...)
2016-09-23 4:11 ` [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray Fam Zheng
@ 2016-09-23 15:35 ` Max Reitz
2016-09-23 19:38 ` John Snow
4 siblings, 1 reply; 8+ messages in thread
From: Max Reitz @ 2016-09-23 15:35 UTC (permalink / raw)
To: John Snow, qemu-block; +Cc: kwolf, qemu-stable, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]
On 23.09.2016 03:45, John Snow wrote:
> When I said "Final re-send," I was lying. Here's a v5.
> The title is also a misnomer by now :)
>
> The move to blk_flush altered the behavior of migration and flushing
> nodes that are not reachable via the guest, but are still reachable
> via QEMU and may or may not need to be flushed.
>
> This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt
> et al being unable to migrate QEMU when the CDROM tray is open.
>
> v5:
> Fix bracket spacing in patch 1. By one space. :(
> Added third patch to remove blk_flush_all.
>
> v4:
> Commit message update.
>
> v3:
> Reworking approach and reinstating bdrv_flush_all at Kevin's suggestion.
>
> ________________________________________________________________________________
>
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch atapi-tray-migfix
> https://github.com/jnsnow/qemu/tree/atapi-tray-migfix
>
> This version is tagged atapi-tray-migfix-v5:
> https://github.com/jnsnow/qemu/releases/tag/atapi-tray-migfix-v5
>
> John Snow (3):
> block: reintroduce bdrv_flush_all
> qemu: use bdrv_flush_all for vm_stop et al
> block-backend: remove blk_flush_all
>
> block/block-backend.c | 22 ----------------------
> block/io.c | 25 +++++++++++++++++++++++++
> cpus.c | 4 ++--
> hw/i386/xen/xen_platform.c | 2 --
> hw/ide/piix.c | 4 ++++
> include/block/block.h | 1 +
> include/sysemu/block-backend.h | 1 -
> 7 files changed, 32 insertions(+), 27 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray
2016-09-23 15:35 ` Max Reitz
@ 2016-09-23 19:38 ` John Snow
2016-09-26 8:28 ` Kevin Wolf
0 siblings, 1 reply; 8+ messages in thread
From: John Snow @ 2016-09-23 19:38 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: kwolf, qemu-stable, qemu-devel
On 09/23/2016 11:35 AM, Max Reitz wrote:
> On 23.09.2016 03:45, John Snow wrote:
>> When I said "Final re-send," I was lying. Here's a v5.
>> The title is also a misnomer by now :)
>>
>> The move to blk_flush altered the behavior of migration and flushing
>> nodes that are not reachable via the guest, but are still reachable
>> via QEMU and may or may not need to be flushed.
>>
>> This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt
>> et al being unable to migrate QEMU when the CDROM tray is open.
>>
>> v5:
>> Fix bracket spacing in patch 1. By one space. :(
>> Added third patch to remove blk_flush_all.
>>
>> v4:
>> Commit message update.
>>
>> v3:
>> Reworking approach and reinstating bdrv_flush_all at Kevin's suggestion.
>>
>> ________________________________________________________________________________
>>
>> For convenience, this branch is available at:
>> https://github.com/jnsnow/qemu.git branch atapi-tray-migfix
>> https://github.com/jnsnow/qemu/tree/atapi-tray-migfix
>>
>> This version is tagged atapi-tray-migfix-v5:
>> https://github.com/jnsnow/qemu/releases/tag/atapi-tray-migfix-v5
>>
>> John Snow (3):
>> block: reintroduce bdrv_flush_all
>> qemu: use bdrv_flush_all for vm_stop et al
>> block-backend: remove blk_flush_all
>>
>> block/block-backend.c | 22 ----------------------
>> block/io.c | 25 +++++++++++++++++++++++++
>> cpus.c | 4 ++--
>> hw/i386/xen/xen_platform.c | 2 --
>> hw/ide/piix.c | 4 ++++
>> include/block/block.h | 1 +
>> include/sysemu/block-backend.h | 1 -
>> 7 files changed, 32 insertions(+), 27 deletions(-)
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
>
Since Fam acked this, I suppose it's for Kevin's tree?
--js
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v5 0/3] block: allow flush on devices with open tray
2016-09-23 19:38 ` John Snow
@ 2016-09-26 8:28 ` Kevin Wolf
0 siblings, 0 replies; 8+ messages in thread
From: Kevin Wolf @ 2016-09-26 8:28 UTC (permalink / raw)
To: John Snow; +Cc: Max Reitz, qemu-block, qemu-stable, qemu-devel
Am 23.09.2016 um 21:38 hat John Snow geschrieben:
> On 09/23/2016 11:35 AM, Max Reitz wrote:
> >On 23.09.2016 03:45, John Snow wrote:
> >> block/block-backend.c | 22 ----------------------
> >> block/io.c | 25 +++++++++++++++++++++++++
> >> cpus.c | 4 ++--
> >> hw/i386/xen/xen_platform.c | 2 --
> >> hw/ide/piix.c | 4 ++++
> >> include/block/block.h | 1 +
> >> include/sysemu/block-backend.h | 1 -
> >> 7 files changed, 32 insertions(+), 27 deletions(-)
> >
> >Reviewed-by: Max Reitz <mreitz@redhat.com>
>
> Since Fam acked this, I suppose it's for Kevin's tree?
If nobody else wants the patches, I'll take them.
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 8+ messages in thread