[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [RFC PATCH 15/56] migration: Make XBZRLE c
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-block] [Qemu-devel] [RFC PATCH 15/56] migration: Make XBZRLE cache size unsigned in QAPI/QMP |
Date: |
Tue, 08 Aug 2017 17:57:12 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Juan Quintela <address@hidden> writes:
> Markus Armbruster <address@hidden> wrote:
>> Sizes should use QAPI type 'size' (uint64_t). migrate-set-cache-size
>> parameter @value is 'int' (int64_t). qmp_migrate_set_cache_size()
>> ensures it fits into size_t. page_cache.c implicitly converts the
>> signed size to unsigned types (it can't quite decide whether to use
>> uint64_t or size_t for cache offsets, but that's not something I can
>> tackle now).
>>
>> XBZRLECacheStats member @cache-size and query-migrate-cache-size's
>> result are also 'int'.
>>
>> Change the migrate-set-cache-size parameter and the XBZRLECacheStats
>> members to 'size', fix up hmp_migrate_set_cache_size(), and tweak a
>> few variable types to reduce implicit conversions.
>>
>> migrate-set-cache-size now accepts size values between 2^63 and
>> 2^64-1. It accepts negative values as before, because that's how the
>> QObject input visitor works for backward compatibility.
>>
>> So does HMP's migrate_set_cache_size.
>>
>> query-migrate and query-migrate-cache-size now report cache sizes
>> above 2^63-1 correctly instead of their (negative) two's complement.
>>
>> So does HMP's "info migrate_cache_size". HMP's "info migrate" already
>> reported the cache size correctly, because it printed the signed
>> integer with PRIu32.
>>
>
> Reviewed-by: Juan Quintela <address@hidden>
>
>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index c8cceb9..ecabff6 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -646,7 +646,7 @@
>> # Since: 1.2
>> ##
>> { 'struct': 'XBZRLECacheStats',
>> - 'data': {'cache-size': 'int', 'bytes': 'int', 'pages': 'int',
>> + 'data': {'cache-size': 'size', 'bytes': 'int', 'pages': 'int',
>> 'cache-miss': 'int', 'cache-miss-rate': 'number',
>> 'overflow': 'int' } }
>>
>> @@ -2875,7 +2875,7 @@
>> # <- { "return": {} }
>> #
>> ##
>> -{ 'command': 'migrate-set-cache-size', 'data': {'value': 'int'} }
>> +{ 'command': 'migrate-set-cache-size', 'data': {'value': 'size'} }
>>
>> ##
>> # @query-migrate-cache-size:
>> @@ -2892,7 +2892,7 @@
>> # <- { "return": 67108864 }
>> #
>> ##
>> -{ 'command': 'query-migrate-cache-size', 'returns': 'int' }
>> +{ 'command': 'query-migrate-cache-size', 'returns': 'size' }
>>
>> ##
>> # @ObjectPropertyInfo:
>
> I am ussming this bits are backward compatible (I don't understand QMP
> to assure this)
I guess I should've explained this in the cover letter.
Until recent commit 5923f85, integers between INT64_MAX+1 and UINT64_MAX
did not work in QMP. QEMU sent and accepted integers between INT64_MIN
and -1 instead.
The fix maintains strict compatibility for QMP input: negative integers
are accepted as before for backward compatibility. Perhaps we can get
rid of this wart some day.
It does not maintain strict compatibility for QMP output: we now output
the correct integer. We figure that's tolerable, because the obvious
way to parse the old output is strtoull(), and that does the right thing
for the new output when it does the right thing for the old output.
Fixing a QAPI type from 'int' to 'size' has the same compatibility
impact.
Questions?
- [Qemu-block] [RFC PATCH 22/56] block: Mix up signed and unsigned less in bdrv_img_create(), (continued)
- [Qemu-block] [RFC PATCH 22/56] block: Mix up signed and unsigned less in bdrv_img_create(), Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 25/56] block/qcow2: Change qcow2_calc_prealloc_size() to uint64_t, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 02/56] qdict: New helpers to put and get unsigned integers, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 06/56] char: Don't truncate -chardev and HMP chardev-add ringbuf size, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 18/56] migration: Make parameter max-bandwidth unsigned in QAPI/QMP, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 15/56] migration: Make XBZRLE cache size unsigned in QAPI/QMP, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 08/56] dump: Make sizes and addresses unsigned in QAPI/QMP, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 24/56] block/qcow2: Change align_offset() to operate on uint64_t, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 10/56] hmp: Make balloon's argument unsigned, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 23/56] option: Fix type of qemu_opt_set_number() parameter @val, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 07/56] cpus: Make memsave, pmemsave sizes, addresses unsigned in QAPI/QMP, Markus Armbruster, 2017/08/07
- [Qemu-block] [RFC PATCH 12/56] pc-dimm: Make size and address unsigned in QAPI/QMP, Markus Armbruster, 2017/08/07