Re: [PATCH v2 15/15] iotests: Add (NBD-based) tests for inactive nodes

[Kevin Wolf <kwolf@redhat.com>]

Am 03.02.2025 um 20:49 hat Eric Blake geschrieben:
> On Fri, Jan 31, 2025 at 10:50:51AM +0100, Kevin Wolf wrote:
> > This tests different types of operations on inactive block nodes
> > (including graph changes, block jobs and NBD exports) to make sure that
> > users manually activating and inactivating nodes doesn't break things.
> > 
> > Support for inactive nodes in other export types will have to come with
> > separate test cases because they have different dependencies like blkio
> > or root permissions and we don't want to disable this basic test when
> > they are not fulfilled.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  tests/qemu-iotests/iotests.py                 |   4 +
> >  tests/qemu-iotests/tests/inactive-node-nbd    | 303 ++++++++++++++++++
> >  .../qemu-iotests/tests/inactive-node-nbd.out  | 239 ++++++++++++++
> >  3 files changed, 546 insertions(+)
> >  create mode 100755 tests/qemu-iotests/tests/inactive-node-nbd
> >  create mode 100644 tests/qemu-iotests/tests/inactive-node-nbd.out
> > 
> 
> > +    iotests.log('\nMirror from active source to inactive target')
> > +
> > +    iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt'))
> > +    iotests.log('snap-fmt active: %s' % node_is_active(vm, 'snap-fmt'))
> > +    iotests.log('snap2-fmt active: %s' % node_is_active(vm, 'snap2-fmt'))
> > +    iotests.log('target-fmt active: %s' % node_is_active(vm, 'target-fmt'))
> > +
> > +    # Activating snap2-fmt recursively activates the whole backing chain
> > +    vm.qmp_log('blockdev-set-active', node_name='snap2-fmt', active=True)
> > +    vm.qmp_log('blockdev-set-active', node_name='target-fmt', active=False)
> 
> Here, you have "Activating ... recursively activates"...
> 
> > +
> > +    iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt'))
> > +    iotests.log('snap-fmt active: %s' % node_is_active(vm, 'snap-fmt'))
> > +    iotests.log('snap2-fmt active: %s' % node_is_active(vm, 'snap2-fmt'))
> > +    iotests.log('target-fmt active: %s' % node_is_active(vm, 'target-fmt'))
> > +
> > +    vm.qmp_log('blockdev-mirror', job_id='job0', device='snap2-fmt',
> > +               target='target-fmt', sync='full',
> > +               filters=[iotests.filter_qmp_generated_node_ids])
> > +
> > +    iotests.log('\nBackup from active source to inactive target')
> > +
> > +    vm.qmp_log('blockdev-backup', job_id='job0', device='snap2-fmt',
> > +               target='target-fmt', sync='full',
> > +               filters=[iotests.filter_qmp_generated_node_ids])
> > +
> > +    iotests.log('\nBackup from inactive source to active target')
> > +
> > +    # Activating snap2-fmt recursively inactivates the whole backing chain
> > +    vm.qmp_log('blockdev-set-active', node_name='snap2-fmt', active=False)
> > +    vm.qmp_log('blockdev-set-active', node_name='target-fmt', active=True)
> 
> ...but here, "Activating ... recursively inactivates".  Is one of
> these statements wrong?

Yes, looks like I updated only half of the comment after copying it. As
you can see in the line after the comment, it sets active=False for
snap2-fmt, so the comment should say "Inactivating ... recursively
inactivates".

Kevin

> Overall a nice barrage of tests, and I can see how adding this many
> tests caused your v2 to fix some bugs that it discovered in v1.
> 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.
> Virtualization:  qemu.org | libguestfs.org
>