[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] blockdev-backup: don't check aio_context too ea
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH] blockdev-backup: don't check aio_context too early |
Date: |
Tue, 7 May 2019 10:50:21 +0200 |
User-agent: |
Mutt/1.11.3 (2019-02-01) |
Am 06.05.2019 um 22:33 hat John Snow geschrieben:
> in blockdev_backup_prepare, we check to make sure that the target is
> associated with a compatible aio context. However, do_blockdev_backup is
> called later and has some logic to move the target to a compatible
> aio_context. The transaction version will fail certain commands
> needlessly early as a result.
>
> Allow blockdev_backup_prepare to simply call do_blockdev_backup, which
> will ultimately decide if the contexts are compatible or not.
>
> Note: the transaction version has always disallowed this operation since
> its initial commit bd8baecd (2014), whereas the version of
> qmp_blockdev_backup at the time, from commit c29c1dd312f, tried to
> enforce the aio_context switch instead. It's not clear, and I can't see
> from the mailing list archives at the time, why the two functions take a
> different approach. It wasn't until later in efd7556708b (2016) that the
> standalone version tried to determine if it could set the context or
> not.
>
> Reported-by: aihua liang <address@hidden>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1683498
Signed-off-by is missing, and a testcase, too. :-)
> diff --git a/blockdev.c b/blockdev.c
> index 79fbac8450..a81d88980c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1872,10 +1872,6 @@ static void blockdev_backup_prepare(BlkActionState
> *common, Error **errp)
> }
>
> aio_context = bdrv_get_aio_context(bs);
> - if (aio_context != bdrv_get_aio_context(target)) {
> - error_setg(errp, "Backup between two IO threads is not implemented");
> - return;
> - }
> aio_context_acquire(aio_context);
> state->bs = bs;
The actual change looks good to me.
Kevin