Re: [PATCH v7 08/10] tools/misc: Add xen-vmtrace tool

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 26/01/2021 13:32, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [PATCH v7 08/10] tools/misc: Add xen-vmtrace tool"):
>> On 26/01/2021 11:59, Ian Jackson wrote:
>>> [Andrew:]
>>>> This is example code, not a production utility.
>>> Perhaps the utility could print some kind of health warning about it
>>> possibly leaving this perf-impacting feature enabled, and how to clean
>>> up ?
>> Why?  The theoretical fallout here is not conceptually different to
>> libxl or qemu segfaulting, or any of the myriad other random utilities
>> we have.
>>
>> Printing "Warning - this program, just like everything else in the Xen
>> tree, might in exceptional circumstances segfault and leave the domain
>> in a weird state" is obvious, and doesn't need stating.
>>
>> The domain is stuffed. `xl destroy` may or may not make the problem go away.
> Firstly, I don't agree with this pessimistic analysis of our current
> tooling.  Secondly, I would consider many such behaviours bugs;
> certainly we have bugs but we shouldn't introduce more of them.
>
> You are justifying the poor robustness of this tool on the grounds
> that it's "example code, not a production utility".
>
> But we are shipping it to bin/ and there is nothing telling anyone
> that trying to use it (perhaps wrapped in something of their own
> devising) is a bad idea.
>
> Either this is code users might be expected to run in production in
> which we need to make it at least have a minimal level of engineering
> robustness (which is perhaps too difficult at this stage), or we need
> to communicate to our users that it's a programming example, not a
> useful utility.
>
> Note that *even if it is a programming example*, we should highlight
> its most important deficiencies.  Otherwise it is a hazardously
> misleading example.
>
> I hope that makes sense.

First of all - I'm not the author of this code.  I'm merely the person
who did the latest round of cleanup, and sent the series.

This code is a damn sight more robust than the other utilities, because
in the case that something does go wrong, the domain will still function
correctly.  Almost everything else, qemu included, will leave the VM
totally broken, as it becomes paused waiting on a request which has
escaped into the ether.


I also don't feel that now is an appropriate time to be insisting that
the goalpost's move when it comes to submissions into tools/,
particularly as you were happy (well - didn't comment on) with this
pattern back in v3.

What exact colour do you want this bikeshed?  Anything non-trivial is
going to miss 4.15.

~Andrew