All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions
@ 2017-04-11 14:50 Max Reitz
  2017-04-11 14:52 ` Max Reitz
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Max Reitz @ 2017-04-11 14:50 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, Max Reitz, Kevin Wolf, Laurent Vivier,
	Stefan Hajnoczi, Fam Zheng

In case of block migration, there may be writes to BlockBackends that do
not have the write permission taken. Before this issue is fixed (which
is not going to happen in 2.9), we therefore cannot assert that this is
the case.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block.c    |  6 +++++-
 block/io.c | 12 ++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 086a12df97..1fbbb8d606 100644
--- a/block.c
+++ b/block.c
@@ -3274,7 +3274,11 @@ int bdrv_truncate(BdrvChild *child, int64_t offset)
     BlockDriver *drv = bs->drv;
     int ret;
 
-    assert(child->perm & BLK_PERM_RESIZE);
+    /* FIXME: Some format block drivers use this function instead of implicitly
+     *        growing their file by writing beyond its end.
+     *        See bdrv_aligned_pwritev() for an explanation why we currently
+     *        cannot assert this permission in that case. */
+    // assert(child->perm & BLK_PERM_RESIZE);
 
     if (!drv)
         return -ENOMEDIUM;
diff --git a/block/io.c b/block/io.c
index bae6947032..8706bfa578 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1345,8 +1345,16 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
     assert(!waited || !req->serialising);
     assert(req->overlap_offset <= offset);
     assert(offset + bytes <= req->overlap_offset + req->overlap_bytes);
-    assert(child->perm & BLK_PERM_WRITE);
-    assert(end_sector <= bs->total_sectors || child->perm & BLK_PERM_RESIZE);
+    /* FIXME: Block migration uses the BlockBackend of the guest device at a
+     *        point when it has not yet taken write permissions. This will be
+     *        fixed by a future patch, but for now we have to bypass this
+     *        assertion for block migration to work. */
+    // assert(child->perm & BLK_PERM_WRITE);
+    /* FIXME: Because of the above, we also cannot guarantee that all format
+     *        BDS take the BLK_PERM_RESIZE permission on their file BDS, since
+     *        they are not obligated to do so if they do not have any parent
+     *        that has taken the permission to write to them. */
+    // assert(end_sector <= bs->total_sectors || child->perm & BLK_PERM_RESIZE);
 
     ret = notifier_with_return_list_notify(&bs->before_write_notifiers, req);
 
-- 
2.12.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions
  2017-04-11 14:50 [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions Max Reitz
@ 2017-04-11 14:52 ` Max Reitz
  2017-04-11 14:58 ` Kevin Wolf
  2017-04-11 15:47 ` Peter Maydell
  2 siblings, 0 replies; 7+ messages in thread
From: Max Reitz @ 2017-04-11 14:52 UTC (permalink / raw)
  To: qemu-block
  Cc: qemu-devel, Kevin Wolf, Laurent Vivier, Stefan Hajnoczi, Fam Zheng

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

On 11.04.2017 16:50, Max Reitz wrote:
> In case of block migration, there may be writes to BlockBackends that do
> not have the write permission taken. Before this issue is fixed (which
> is not going to happen in 2.9), we therefore cannot assert that this is
> the case.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c    |  6 +++++-
>  block/io.c | 12 ++++++++++--
>  2 files changed, 15 insertions(+), 3 deletions(-)

I'll send a revert for 2.10 if/when this is merged.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions
  2017-04-11 14:50 [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions Max Reitz
  2017-04-11 14:52 ` Max Reitz
@ 2017-04-11 14:58 ` Kevin Wolf
  2017-04-11 15:07   ` Laurent Vivier
  2017-04-11 15:47 ` Peter Maydell
  2 siblings, 1 reply; 7+ messages in thread
From: Kevin Wolf @ 2017-04-11 14:58 UTC (permalink / raw)
  To: Max Reitz
  Cc: qemu-block, qemu-devel, Laurent Vivier, Stefan Hajnoczi, Fam Zheng

Am 11.04.2017 um 16:50 hat Max Reitz geschrieben:
> In case of block migration, there may be writes to BlockBackends that do
> not have the write permission taken. Before this issue is fixed (which
> is not going to happen in 2.9), we therefore cannot assert that this is
> the case.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

I tested block migration (migrate -b). Leaving postcopy migration, which
is apparently also broken, to Laurent.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Tested-by: Kevin Wolf <kwolf@redhat.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions
  2017-04-11 14:58 ` Kevin Wolf
@ 2017-04-11 15:07   ` Laurent Vivier
  2017-04-12 13:18     ` Kevin Wolf
  0 siblings, 1 reply; 7+ messages in thread
From: Laurent Vivier @ 2017-04-11 15:07 UTC (permalink / raw)
  To: Kevin Wolf, Max Reitz; +Cc: qemu-block, qemu-devel, Stefan Hajnoczi, Fam Zheng

On 11/04/2017 16:58, Kevin Wolf wrote:
> Am 11.04.2017 um 16:50 hat Max Reitz geschrieben:
>> In case of block migration, there may be writes to BlockBackends that do
>> not have the write permission taken. Before this issue is fixed (which
>> is not going to happen in 2.9), we therefore cannot assert that this is
>> the case.
>>
>> Suggested-by: Kevin Wolf <kwolf@redhat.com>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> I tested block migration (migrate -b). Leaving postcopy migration, which
> is apparently also broken, to Laurent.
> 
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Tested-by: Kevin Wolf <kwolf@redhat.com>
> 

With postcopy migration

Tested-by: Laurent Vivier <lvivier@redhat.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions
  2017-04-11 14:50 [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions Max Reitz
  2017-04-11 14:52 ` Max Reitz
  2017-04-11 14:58 ` Kevin Wolf
@ 2017-04-11 15:47 ` Peter Maydell
  2 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2017-04-11 15:47 UTC (permalink / raw)
  To: Max Reitz
  Cc: Qemu-block, Kevin Wolf, Laurent Vivier, Fam Zheng,
	QEMU Developers, Stefan Hajnoczi

On 11 April 2017 at 15:50, Max Reitz <mreitz@redhat.com> wrote:
> In case of block migration, there may be writes to BlockBackends that do
> not have the write permission taken. Before this issue is fixed (which
> is not going to happen in 2.9), we therefore cannot assert that this is
> the case.
>
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions
  2017-04-11 15:07   ` Laurent Vivier
@ 2017-04-12 13:18     ` Kevin Wolf
  2017-04-12 13:28       ` Laurent Vivier
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Wolf @ 2017-04-12 13:18 UTC (permalink / raw)
  To: Laurent Vivier
  Cc: Max Reitz, qemu-block, qemu-devel, Stefan Hajnoczi, Fam Zheng,
	dgilbert, quintela

Am 11.04.2017 um 17:07 hat Laurent Vivier geschrieben:
> On 11/04/2017 16:58, Kevin Wolf wrote:
> > Am 11.04.2017 um 16:50 hat Max Reitz geschrieben:
> >> In case of block migration, there may be writes to BlockBackends that do
> >> not have the write permission taken. Before this issue is fixed (which
> >> is not going to happen in 2.9), we therefore cannot assert that this is
> >> the case.
> >>
> >> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> > 
> > I tested block migration (migrate -b). Leaving postcopy migration, which
> > is apparently also broken, to Laurent.
> > 
> > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> > Tested-by: Kevin Wolf <kwolf@redhat.com>
> 
> With postcopy migration
> 
> Tested-by: Laurent Vivier <lvivier@redhat.com>

I think the following is the real fix for postcopy migration, in case
someone wants to give it a test before I send it as a proper patch (the
bug is a result of duplicating code between precopy/postcopy migration
instead of sharing it - commit d35ff5e6 only updated one of both).

Kevin


diff --git a/migration/savevm.c b/migration/savevm.c
index 3b19a4a..43fa9bf 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1623,6 +1623,14 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
         error_report_err(local_err);
     }
 
+    /* If we get an error here, just don't restart the VM yet. */
+    blk_resume_after_migration(&local_err);
+    if (local_err) {
+        error_free(local_err);
+        local_err = NULL;
+        autostart = false;
+    }
+
     trace_loadvm_postcopy_handle_run_cpu_sync();
     cpu_synchronize_all_post_init();

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions
  2017-04-12 13:18     ` Kevin Wolf
@ 2017-04-12 13:28       ` Laurent Vivier
  0 siblings, 0 replies; 7+ messages in thread
From: Laurent Vivier @ 2017-04-12 13:28 UTC (permalink / raw)
  To: Kevin Wolf
  Cc: Max Reitz, qemu-block, qemu-devel, Stefan Hajnoczi, Fam Zheng,
	dgilbert, quintela

On 12/04/2017 15:18, Kevin Wolf wrote:
> Am 11.04.2017 um 17:07 hat Laurent Vivier geschrieben:
>> On 11/04/2017 16:58, Kevin Wolf wrote:
>>> Am 11.04.2017 um 16:50 hat Max Reitz geschrieben:
>>>> In case of block migration, there may be writes to BlockBackends that do
>>>> not have the write permission taken. Before this issue is fixed (which
>>>> is not going to happen in 2.9), we therefore cannot assert that this is
>>>> the case.
>>>>
>>>> Suggested-by: Kevin Wolf <kwolf@redhat.com>
>>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>>
>>> I tested block migration (migrate -b). Leaving postcopy migration, which
>>> is apparently also broken, to Laurent.
>>>
>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>>> Tested-by: Kevin Wolf <kwolf@redhat.com>
>>
>> With postcopy migration
>>
>> Tested-by: Laurent Vivier <lvivier@redhat.com>
> 
> I think the following is the real fix for postcopy migration, in case
> someone wants to give it a test before I send it as a proper patch (the
> bug is a result of duplicating code between precopy/postcopy migration
> instead of sharing it - commit d35ff5e6 only updated one of both).

Tested with my test case and it works fine.

Thanks,
Laurent

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-04-12 13:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11 14:50 [Qemu-devel] [PATCH for-2.9?] block/io: Comment out permission assertions Max Reitz
2017-04-11 14:52 ` Max Reitz
2017-04-11 14:58 ` Kevin Wolf
2017-04-11 15:07   ` Laurent Vivier
2017-04-12 13:18     ` Kevin Wolf
2017-04-12 13:28       ` Laurent Vivier
2017-04-11 15:47 ` Peter Maydell

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.