[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor |
Date: |
Mon, 2 May 2016 08:23:22 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 05/02/2016 07:26 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> 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 <address@hidden>
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> -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
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Markus Armbruster, 2016/05/02
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor,
Eric Blake <=
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Dr. David Alan Gilbert, 2016/05/03
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Markus Armbruster, 2016/05/03
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Eric Blake, 2016/05/03
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Dr. David Alan Gilbert, 2016/05/03
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Markus Armbruster, 2016/05/04
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Dr. David Alan Gilbert, 2016/05/04
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Paolo Bonzini, 2016/05/24
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Dr. David Alan Gilbert, 2016/05/03
- Re: [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Markus Armbruster, 2016/05/04