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

2025-02-03 Thread Kevin Wolf
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 
> > ---
> >  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 00..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




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

2025-02-03 Thread Eric Blake
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 
> ---
>  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 00..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)?

> +
> +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?

Oveall a nice test.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org