From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=47997 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PFoOJ-0000ZJ-F7 for qemu-devel@nongnu.org; Tue, 09 Nov 2010 08:37:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PFoOD-0001NQ-9N for qemu-devel@nongnu.org; Tue, 09 Nov 2010 08:37:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:29047) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PFoOD-0001NH-17 for qemu-devel@nongnu.org; Tue, 09 Nov 2010 08:37:33 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id oA9DbWns019299 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 9 Nov 2010 08:37:32 -0500 Date: Tue, 9 Nov 2010 15:37:28 +0200 From: "Michael S. Tsirkin" Message-ID: <20101109133728.GH22705@redhat.com> References: <1288623114-14439-1-git-send-email-kraxel@redhat.com> <20101108174752.GC8498@redhat.com> <4CD91661.6030102@redhat.com> <20101109113453.GA22705@redhat.com> <4CD93573.7040009@redhat.com> <20101109130530.GD22705@redhat.com> <4CD94C85.9060008@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4CD94C85.9060008@redhat.com> Subject: [Qemu-devel] Re: [PATCH] add VMSTATE_BOOL List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org 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