Am 13.09.2019 um 21:54 hat John Snow geschrieben: > > > On 9/13/19 11:25 AM, Sergio Lopez wrote: > > do_drive_backup() already acquires the AioContext, so release it > > before the call. > > > > Signed-off-by: Sergio Lopez <s...@redhat.com> > > --- > > blockdev.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/blockdev.c b/blockdev.c > > index fbef6845c8..3927fdab80 100644 > > --- a/blockdev.c > > +++ b/blockdev.c > > @@ -1783,20 +1783,16 @@ static void drive_backup_prepare(BlkActionState > > *common, Error **errp) > > > > aio_context = bdrv_get_aio_context(bs); > > aio_context_acquire(aio_context); > > -
Are you removing this unrelated empty line intentionally? > > /* Paired with .clean() */ > > bdrv_drained_begin(bs); > > Do we need to make this change to blockdev_backup_prepare as well? Actually, the whole structure feels a bit wrong. We get the bs here and take its lock, then release the lock again and forget the reference, only to do both things again inside do_drive_backup(). The way snapshots work is that the "normal" snapshot commands are wrappers that turn it into a single-entry transaction. Then you have only one code path where you can resolve the ID and take the lock just once. So maybe backup should work like this, too? Kevin
