Re: [Qemu-devel] [PATCH v2 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting

[Peter Crosthwaite]

On Wed, Dec 18, 2013 at 11:48 PM, Alexander Graf <address@hidden> wrote:
>
> On 11.11.2013, at 09:16, address@hidden wrote:
>
>> From: Peter Crosthwaite <address@hidden>
>>
>> There are a mix of usages of the qemu_fdt_* API calls, some which
>> wish to assert and abort QEMU on failure and some of which wish to do
>> their own error handling. The latter in more correct, but the former
>> is in the majority and more pragmatic. However the asserting clients
>> are usually doing large numbers fdt ops and only want to assert if any
>> one of them breaks. So the cleanest compromising solution is:
>>
>> 1. Require an Error ** to be passes to all fdt ops.
>> 2. And perform no action if its already non-null (error condition).
>> 3. If no Error ** is given report errors to stderr
>>
>> Step one allows clients to do their own error handling if they wish
>> too, which is the most flexible:
>>
>> Error *err = NULL;
>> fdt_foo( ... , &err);
>> if (err) {
>>    /* handle error my special way */
>> }
>>
>> Step two allows you to do a large number of fdt ops then check them
>> all only the once at the end:
>>
>> Error *err = NULL;
>> fdt_foo( ... , &err);
>> fdt_bar( ... , &err);
>> fdt_baz( ... , &err);
>> if (err) {
>>    /* First encountered error will be in err */
>> }
>>
>> Step 3, handles the common case where the error is not an issue, but
>> just needs to make noise at the user (via stderr). This seems common for
>> bootloader code that sets chosen and initrd props etc (rather than machine
>> description).
>>
>> All error reporting is stylistically udpated to use Error ** instead of
>> integer return codes and no more exit(1) on failures.
>>
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>
> Thanks a lot. I've applied the first two patches to ppc-next with some 
> adjustments to also cover hw/arm/virt.c.

Thanks,

That make my respin of this one much easier.

> But this patch is too much work to rebase to current master :).

This patch is going to see a major change pattern once the
&error_abort series goes through, as the assert_no_error scheme is now
gone. This would have conflicted with the qmp queue that holds those
patches (I probably should have put a note on list to retract this
patch).

>
> Please repost a version that also covers virt.c. I'd also like to see an Ack 
> from Peter at least.
>

I'll wait for both yours and Luiz' queues to flush and spin it.

Regards,
Peter

>
> Alex
>
>