[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] add VMSTATE_BOOL
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH] add VMSTATE_BOOL |
Date: |
Tue, 9 Nov 2010 15:37:28 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Nov 09, 2010 at 02:28:37PM +0100, Gerd Hoffmann wrote:
> 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.
Unless there's a bug in software that writes the file, then checksum
will match.
> >What, exactly, do you want to check on save?
>
> I don't want to check anything.
Why did you suggest it above then?
> I'm just saying that *if* we are sanity-checking bool
My patch doesn't check bool. Look at it. I am sanity
checking a byte read from file. File can have any values,
there is no guarantee that it has the same value
that the same version of qemu wrote out.
> to catch bugs
> it is much more useful to do that when saving.
There's nothing we *can* check.
if (v == true || v == false)
is always true according to the language standard.
How is it useful to stick always true conditions that compiler
will likely remove in code?
> >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