On 7/5/19 10:07 AM, Stefan Hajnoczi wrote:
> On Thu, Jul 04, 2019 at 11:28:37AM +0100, Daniel P. Berrangé wrote:
>> On Thu, Jul 04, 2019 at 11:24:57AM +0100, Stefan Hajnoczi wrote:
>>> On Wed, Jul 03, 2019 at 06:25:01PM +0100, Daniel P. Berrangé wrote:
>>>> On Wed, Jul 03, 2019 at 07:10:05PM +0200, Philippe Mathieu-Daudé wrote:
>>>>> @@ -306,6 +308,13 @@ bool trace_init_backends(void)
>>>>> openlog(NULL, LOG_PID, LOG_DAEMON);
>>>>> #endif
>>>>>
>>>>> + trace_init_file(getenv("QEMU_TRACE_LOGFILE"));
>>>>> + trace_init_events(getenv("QEMU_TRACE_EVENTFILE"));
>>>>> + trace_env = getenv("QEMU_TRACE_EVENTS");
>>>>> + if (trace_env) {
>>>>> + trace_enable_events(trace_env);
>>>>> + }
>>>>> +
>>>>
>>>> I don't think it is a nice idea to add this via environment variables
>>>> to QEMU itself. Why not modify libqtest qtest_init_without_qmp_handshake
>>>> to read the env vars and then pass a suitable -trace arg when spawning
>>>> QEMU ?
>>>
>>> What is the concern about adding these environment variables to QEMU?
>>>
>>> It is convenient to be able to use tracing even if QEMU is invoked by
>>> something you cannot modify/control.
>>>
>>> The main issues I see with environment variables are:
>>>
>>> 1. Security. Is there a scenario where an attacker can use environment
>>> variables to influence the behavior of a QEMU process running at a
>>> different trust level?
>>>
>>> 2. Name collision. What is the chance that existing users already
>>> define environment variables with these names and that unexpected
>>> behavior could result?
>>
>> One of the biggest problems with QEMU in general has been poorly modelled
>> & defined interfaces for configuration. At runtime we've solved this with
>> QMP. At startup we're still fighting the horror of QemuOpts in general and
>> haven't got startup modelling to be on a par with that offered by QEMU.
>> It was even worse when Audio didn't even use QemuOpts and instead used
>> an arbitrary set of poorly defined env variables. To me adding yet another
>> way to configure QEMU via env vars is moving in the opposite direction to
>> what we want.
>
> In this case the environment variables are optional and meant for cases
> where the user cannot change the QEMU command-line. I think they serve
> a different purpose from the audio subsystem environment variables and
> I'd be happy to merge them.
>
> Philippe: Have you tried adding the environment variable to libqtest as
> Dan suggested and did it work for your use case?Yes, but we loose the ability to use this feature from linux-user and other tools: $ git grep trace_opt_parse bsd-user/main.c:851: trace_file = trace_opt_parse(optarg); linux-user/main.c:387: trace_file = trace_opt_parse(arg); qemu-img.c:5063: trace_file = trace_opt_parse(optarg); qemu-io.c:579: trace_file = trace_opt_parse(optarg); qemu-nbd.c:862: trace_file = trace_opt_parse(optarg); scsi/qemu-pr-helper.c:969: trace_file = trace_opt_parse(optarg); vl.c:3730: trace_file = trace_opt_parse(optarg); So I'm now mixed about the trade off regarding Daniel worries.
signature.asc
Description: OpenPGP digital signature
