Re: [Qemu-devel] [PATCH v4 3/5] iotests: change qmp_log filters to expect QMP objects only

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

19.12.2018 14:07, Vladimir Sementsov-Ogievskiy wrote:
> 19.12.2018 4:52, John Snow wrote:
>> log() treats filters as if they can always filter its primary argument.
>> qmp_log treats filters as if they're always text.
>>
>> Change qmp_log to treat filters as if they're always qmp object filters,
>> then change the logging call to rely on log()'s ability to serialize QMP
>> objects, so we're not duplicating that effort.
> 
> As I understand, there still no use for qmp-object based filters (even after the
> series), do we really need them? I'm afraid it's premature complication.

aha, sorry, missed that you use it in 206.
But still not sure that it worth it. Isn't it better to just remove fields from dict,
which are unpredictable, instead of substituting them..

The other idea here: if we want
automatically logged qmp commands (qmp_log(), actually), it should filter unpredictable
things from output automatically, just by command, which is the first argument. Caller
should not care about it, as it may be derived from command, how to filter it's output.
And then, we just need a kind of dict of functions, which do not do something like generic
recursion, but specifically prepares common-test-output for the concrete command...

> 
> Why not to keep all filters text based? If we need to filter some concrete fields,
> not the whole thing, I doubt that recursion and defining functions inside
> functions is a true way for it. Instead in concrete case, like in you test, it's
> better to select fields that should be in output, and output only them.
> 
>>
>> Because kwargs have been sorted already, the order is preserved.
>>
>> Edit the only caller who uses filters on qmp_log to use a qmp version,
>> also added in this patch.
>> ---
>>   tests/qemu-iotests/206        |  4 ++--
>>   tests/qemu-iotests/iotests.py | 24 +++++++++++++++++++++---
>>   2 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206
>> index e92550fa59..5bb738bf23 100755
>> --- a/tests/qemu-iotests/206
>> +++ b/tests/qemu-iotests/206
>> @@ -27,7 +27,7 @@ iotests.verify_image_format(supported_fmts=['qcow2'])
>>   def blockdev_create(vm, options):
>>       result = vm.qmp_log('blockdev-create',
>> -                        filters=[iotests.filter_testfiles],
>> +                        filters=[iotests.filter_qmp_testfiles],
>>                           job_id='job0', options=options)
>>       if 'return' in result:
>> @@ -55,7 +55,7 @@ with iotests.FilePath('t.qcow2') as disk_path, \
>>                             'size': 0 })
>>       vm.qmp_log('blockdev-add',
>> -               filters=[iotests.filter_testfiles],
>> +               filters=[iotests.filter_qmp_testfiles],
>>                  driver='file', filename=disk_path,
>>                  node_name='imgfile')
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 55fb60e039..812302538d 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -246,10 +246,29 @@ def filter_qmp_event(event):
>>           event['timestamp']['microseconds'] = 'USECS'
>>       return event
>> +def filter_qmp(qmsg, filter_fn):
>> +    '''Given a string filter, filter a QMP object's values.
>> +    filter_fn takes a (key, value) pair.'''
>> +    for key in qmsg:
>> +        if isinstance(qmsg[key], list):
>> +            qmsg[key] = [filter_qmp(atom, filter_fn) for atom in qmsg[key]]
>> +        elif isinstance(qmsg[key], dict):
>> +            qmsg[key] = filter_qmp(qmsg[key], filter_fn)
>> +        else:
>> +            qmsg[key] = filter_fn(key, qmsg[key])
>> +    return qmsg
>> +
>>   def filter_testfiles(msg):
>>       prefix = os.path.join(test_dir, "%s-" % (os.getpid()))
>>       return msg.replace(prefix, 'TEST_DIR/PID-')
>> +def filter_qmp_testfiles(qmsg):
>> +    def _filter(key, value):
>> +        if key == 'filename' or key == 'backing-file':
>> +            return filter_testfiles(value)
>> +        return value
>> +    return filter_qmp(qmsg, _filter)
>> +
>>   def filter_generated_node_ids(msg):
>>       return re.sub("#block[0-9]+", "NODE_NAME", msg)
>> @@ -462,10 +481,9 @@ class VM(qtest.QEMUQtestMachine):
>>       def qmp_log(self, cmd, filters=[], **kwargs):
>>           full_cmd = OrderedDict({"execute": cmd,
>>                                   "arguments": ordered_kwargs(kwargs)})
>> -        logmsg = json.dumps(full_cmd)
>> -        log(logmsg, filters)
>> +        log(full_cmd, filters)
>>           result = self.qmp(cmd, **kwargs)
>> -        log(json.dumps(result, sort_keys=True), filters)
>> +        log(result, filters)
>>           return result
>>       def run_job(self, job, auto_finalize=True, auto_dismiss=False):
>>
> 
> 


-- 
Best regards,
Vladimir