[Qemu-devel] Re: [PATCH] add VMSTATE_BOOL

[Gerd Hoffmann <kraxel@redhat.com>]

On 11/09/10 14:05, Michael S. Tsirkin wrote:
> On Tue, Nov 09, 2010 at 12:50:11PM +0100, Gerd Hoffmann wrote:
>>    Hi,
>>
>>>>>> +static int get_bool(QEMUFile *f, void *pv, size_t size)
>>>>>> +{
>>>>>> +    bool *v = pv;
>>>>>> +    *v = qemu_get_byte(f);
>>>>>> +    return 0;
>>
>>> I think we should verify that value is 0 or 1 and fail
>>> migration otherwise, to make it more robust.
>>
>> I still think such a check doesn't belong into the migration code as
>> such a bug would exist without migration too.  And if anything we
>> should check on save not on load, otherwise qemu can write out
>> savevm images which it will refuse to load.  I wouldn't call this
>> "robust".
>>
>> cheers,
>>    Gerd
>
> I think we should verify on load: e.g. the image could have
> got corrupted.

For catching corruption checksums work much better.

> What, exactly, do you want to check on save?

I don't want to check anything.

I'm just saying that *if* we are sanity-checking bool to catch bugs it 
is much more useful to do that when saving.

> savevm: validate bool values on load
>
> We always save 0 or 1 values for booleans.  Validate on input to
> increase the chance of detecting input corruption.

NACK.

cheers,
   Gerd