Re: [PATCH 06/14] rust: qemu-api: add bindings to Error

[Markus Armbruster <armbru@redhat.com>]

Zhao Liu <zhao1.liu@intel.com> writes:
> Markus Armbruster <armbru@redhat.com> writes:

[...]

>> Let's examine the other aspect: how exactly "storing" behaves.
>> 
>> error_setg() according to its contract:
>> 
>>     If @errp is NULL, the error is ignored.  [...]
>> 
>>     If @errp is &error_abort, print a suitable message and abort().
>> 
>>     If @errp is &error_fatal, print a suitable message and exit(1).
>> 
>>     If @errp is anything else, *@errp must be NULL.
>> 
>> error_propagate() according to its contract:
>> 
>>     [...] if @dst_errp is NULL, errors are being ignored.  Free the
>>     error object.
>> 
>>     Else, if @dst_errp is &error_abort, print a suitable message and
>>     abort().
>> 
>>     Else, if @dst_errp is &error_fatal, print a suitable message and
>>     exit(1).
>> 
>>     Else, if @dst_errp already contains an error, ignore this one: free
>>     the error object.
>> 
>>     Else, move the error object from @local_err to *@dst_errp.
>> 
>> The second to last clause is where its storing differs from
>> error_setg().
>> 
>> What does errp.write(err) do?  I *guess* it simply stores @err in @errp.
>> Matches neither behavior.
>> 
>> If that's true, then passing &error_abort or &error_fatal to Rust does
>> not work, and neither does error accumulation.  Not equivalent of C
>> error_propagate().
>
> I did some simple tests. yes, &error_abort or &error_fatal doesn't work.
> Current @errp of realize() can work because @errp points to @local_err
> in device_set_realized().

Thank you!

>> Is "propagate" semantics what you want here?
>> 
>> If not, use another name.
>
> I guess here we should call C version's error_propagate() instead of
> write():
>
> diff --git a/rust/qemu-api/src/error.rs b/rust/qemu-api/src/error.rs
> index a91ce6fefaf4..56622065ad22 100644
> --- a/rust/qemu-api/src/error.rs
> +++ b/rust/qemu-api/src/error.rs
> @@ -205,7 +205,7 @@ pub unsafe fn propagate(self, errp: *mut *mut bindings::Error) {
>
>          // SAFETY: caller guarantees errp is valid
>          unsafe {
> -            errp.write(err);
> +            bindings::error_propagate(errp, err);
>          }
>      }
>
> ---
>
> Then Rust's propagate has the same behavior as C (Of course, here Rust
> is actually using C's error_propagate, so the two are equivalent.)

*If* we want propagate semantics.  I'm not sure we do.

If we don't: use error_handle()?