Re: [PATCH v4 14/15] qemu-iotests: add option to show qemu binary logs on stdout

From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Cleber Rosa <crosa@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH v4 14/15] qemu-iotests: add option to show qemu binary logs on stdout
Date: Fri, 28 May 2021 22:32:11 +0200	[thread overview]
Message-ID: <b943927f-9cb6-cee3-45b3-f1c591bc7e13@redhat.com> (raw)
In-Reply-To: <21fd6fc2-0715-34dd-c070-696f5f106f87@virtuozzo.com>

>> +
>>   imgfmt = os.environ.get('IMGFMT', 'raw')
>>   imgproto = os.environ.get('IMGPROTO', 'file')
>>   output_dir = os.environ.get('OUTPUT_DIR', '.')
>> @@ -614,6 +616,13 @@ def _post_shutdown(self) -> None:
>>           super()._post_shutdown()
>>           self.subprocess_check_valgrind(qemu_valgrind)
>> +    def _pre_launch(self) -> None:
>> +        super()._pre_launch()
>> +        if qemu_print and self._qemu_log_file is not None:
>> +            # set QEMU binary output to stdout
>> +            self._qemu_log_file.close()
>> +            self._qemu_log_file = None
>> +
> 
> So, many use of _private members actually show that proper way of doing 
> this is adding an option to __init__ instead

And then add yet another bool variable in __init__ just to mark when use 
the log file or not? At this point, if we really don't want this here we 
can just create a public function in machine.py and call that...
This can also be shared with machine.py's _post_shutdown().

> 
> Interesting will pylint complain on using _private members outside of 
> the home class?

No, test 297 tests it and prints no warning or error.

Thank you,
Emanuele