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

[Gerd Hoffmann]

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