Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]

On 05/02/2016 07:26 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Rather than using a QJSON object and converting the QString result
>> to a char *, we can use the new JSON output visitor and get directly
>> to a char *.
>>
>> The conversions are a bit tricky in place (in places, we have to
>> copy an integer to an int64_t temporary to get the right pointer for
>> visit_type_int(); and for several strings, we have to copy to a
>> temporary variable so we can take an address (&char[] is not the
>> same as &char*) and cast away const), but overall still fairly
>> mechanical.
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>

>> -static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdesc)
>> +static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se,
>> +                                   Visitor *vmdesc)
>>  {
>>      int64_t old_offset, size;
>> +    const char *tmp;
>>
>>      old_offset = qemu_ftell_fast(f);
>>      se->ops->save_state(f, se->opaque);
>>      size = qemu_ftell_fast(f) - old_offset;
>>
>>      if (vmdesc) {
> 
> Conditionals could be avoided: use a null visitor.  Not sure it's worth
> it, though.

We could just teach qapi-visit-core.c to be a no-op for v==NULL (thus
hiding the conditionals in the core code, but that then slows down the
common case for more conditionals on every caller.  Maybe a null visitor
is reasonable, after all?

>> +        tmp = "data";
>> +        visit_type_str(vmdesc, "name", (char **)&tmp, &error_abort);
> 
> The Visitor interface is the same for input and for output.  Convenient
> when the code is direction-agnostic.  Inconvenient when it's output: you
> have to pass the value by reference even though it's only read.  In
> particular, literals need a temporary, and types have to be adjusted via
> cast or temporary more frequently than for by-value.
> 
> If that bothers us, we can add by-value wrappers to the interface.
> 
> Are there other output-only visitor uses?

qom-get is output-only, just as qom-set is input-only.  Maybe it's worth
an experiment to see how difficult it would be.


> Well, it doesn't exactly make this code prettier, but having a stupid
> wrapper just to hide the ugliness isn't so hot, either.

And now you see why I posted two alternatives, to see which way we want
to go.  Having convenient wrappers for output-only visits may swing the
vote.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]