Re: [Qemu-devel] [PATCH 0/2] qom: detect attempts to add a property that already exists

[Peter Maydell]

On 22 October 2012 16:35, Luiz Capitulino <address@hidden> wrote:
> Peter Maydell <address@hidden> wrote:
>
>> The aim of this patch series is to make QEMU exit with a helpful
>> error message for bugs where multiple properties of the same name
>> are accidentally added to a QOM object.
>
> Does this happen only at build-time or can it happen at command-line
> too? What about QMP/HMP?

Anything that cares about not abort()ing needs to pass a valid
Error** in.

>> In order to achieve this
>> for static properties whilst still allowing the hotplug case
>> to gracefully fail without killing QEMU, we add the concept
>> of a 'critical' error. A critical error is one which must be
>> handled somehow -- if we encounter a NULL Error** either when
>> the error is raised or later when it is propagated, we will
>> abort() rather than throwing the error away.
>
> This gives me the impression that we're fixing it in the wrong layer.
> Besides, all code calling error_propagate() today can now abort
> (at least in theory), but that's something we really don't want to happen
> in QMP.

That's why QMP gets to pass in an Error ** and handle the result.

I'm open to better ways to handle this. Perhaps all Errors should
be critical? :-)

Mostly what this patch is trying to do is deal with the fact that
huge amounts of code using the Error interface just throws away
the error. I'd rather not have lots of boilerplate at the device
and board model level that's just sitting there asserting "this
will always succeed". So instead we say "if you pass in a NULL
Error ** then you are saying that you know this won't fail".

> An alternative would be to let users set is_critical, but add a
> error_is_critical() function and let the code that wants to abort
> to check for it.
>
> But, how difficult it's to add a flag to QPM objects to allow/disallow
> multiple properties?

Why would you want to have multiple properties with the same name?
It's not meaningful because property lookup is by name so there's
no way to distinguish them. It's always an error, either in QEMU
itself or an error by the user on the command line.

-- PMM