Re: [PATCH v2 14/15] iotests: Add qsd-migrate case

[Kevin Wolf <kwolf@redhat.com>]

Am 03.02.2025 um 20:35 hat Eric Blake geschrieben:
> On Fri, Jan 31, 2025 at 10:50:50AM +0100, Kevin Wolf wrote:
> > Test that it's possible to migrate a VM that uses an image on shared
> > storage through qemu-storage-daemon.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  tests/qemu-iotests/tests/qsd-migrate     | 132 +++++++++++++++++++++++
> >  tests/qemu-iotests/tests/qsd-migrate.out |  51 +++++++++
> >  2 files changed, 183 insertions(+)
> >  create mode 100755 tests/qemu-iotests/tests/qsd-migrate
> >  create mode 100644 tests/qemu-iotests/tests/qsd-migrate.out
> > 
> > diff --git a/tests/qemu-iotests/tests/qsd-migrate b/tests/qemu-iotests/tests/qsd-migrate
> > new file mode 100755
> > index 0000000000..687bda6f93
> > --- /dev/null
> > +++ b/tests/qemu-iotests/tests/qsd-migrate
> > @@ -0,0 +1,132 @@
> > +#!/usr/bin/env python3
> > +# group: rw quick
> 
> > +
> > +with iotests.FilePath('disk.img') as path, \
> > +     iotests.FilePath('nbd-src.sock', base_dir=iotests.sock_dir) as nbd_src, \
> > +     iotests.FilePath('nbd-dst.sock', base_dir=iotests.sock_dir) as nbd_dst, \
> > +     iotests.FilePath('migrate.sock', base_dir=iotests.sock_dir) as mig_sock, \
> > +     iotests.VM(path_suffix="-src") as vm_src, \
> > +     iotests.VM(path_suffix="-dst") as vm_dst:
> > +
> 
> > +
> > +    iotests.log('\nTest I/O on the source')
> > +    vm_src.hmp_qemu_io('virtio0/virtio-backend', 'write -P 0x11 0 4k',
> > +                       use_log=True, qdev=True)
> > +    vm_src.hmp_qemu_io('virtio0/virtio-backend', 'read -P 0x11 0 4k',
> > +                       use_log=True, qdev=True)
> > +
> > +    iotests.log('\nStarting migration...')
> 
> 
> Is it worth adding a test that qemu_io fails to write on the
> destination while it is inactive (to ensure we are properly rejecting
> modification of an inactive image)?

The problem with that is that the failure mode for qemu_io (which acts
as if it were a device, not an external interface) is an assertion
failure.

The other test (in patch 15) tests writes on the NBD export, which fails
gracefully.

> > +
> > +    mig_caps = [
> > +        {'capability': 'events', 'state': True},
> > +        {'capability': 'pause-before-switchover', 'state': True},
> > +    ]
> > +    vm_src.qmp_log('migrate-set-capabilities', capabilities=mig_caps)
> > +    vm_dst.qmp_log('migrate-set-capabilities', capabilities=mig_caps)
> > +    vm_src.qmp_log('migrate', uri=f'unix:{mig_sock}',
> > +                   filters=[iotests.filter_qmp_testfiles])
> > +
> > +    vm_src.event_wait('MIGRATION',
> > +                      match={'data': {'status': 'pre-switchover'}})
> > +
> > +    iotests.log('\nPre-switchover: Reconfigure QSD instances')
> > +
> > +    iotests.log(qsd_src.qmp('blockdev-set-active', {'active': False}))
> > +    iotests.log(qsd_dst.qmp('blockdev-set-active', {'active': True}))
> 
> Also, should you attempt a read on both src and dst while both sides
> are inactive, to prove that reads can take a snapshot in the middle of
> the handover?

I think this could be done without any problems.

Kevin