From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [PATCH] virtio-net: Fix save/load Date: Thu, 15 Jan 2009 11:39:59 -0700 Message-ID: <1232044799.20605.22.camel@bling> References: <1232042731.20605.9.camel@bling> <1232043670.24287.5.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Cc: Avi Kivity , kvm-devel To: Mark McLoughlin Return-path: Received: from g1t0028.austin.hp.com ([15.216.28.35]:21430 "EHLO g1t0028.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1762832AbZAOSkX (ORCPT ); Thu, 15 Jan 2009 13:40:23 -0500 In-Reply-To: <1232043670.24287.5.camel@localhost.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: Hi Mark, On Thu, 2009-01-15 at 18:21 +0000, Mark McLoughlin wrote: > Actually, we really need to handle VLANClientState:link_down for all > vlan clients, not just virtio-net and then update status on load > according to link_down. > > It's not critically important, though - if we neglect to save/load this > the only side effect is that "set link down" would have to be called > again. Does need to be fixed, though. The link status may be able to get away w/o a save/load, but what else is going to get added to there later. Seems like we might as well add it now. > > +/* > > + * Anything added here should cause a bump in VIRTIO_NET_VM_VERSION > > + * and appropriate conditionalized load with sane defaults for older > > + * images should be added to virtio_net_load(). > > + */ > > This is true for all savevm() code, so I don't think we need the comment > here. Doesn't hurt and it's obviously easy to forget ;) > > static int virtio_net_load(QEMUFile *f, void *opaque, int > version_id) > > { > > VirtIONet *n = opaque; > > > > - if (version_id != 2) > > + if (version_id < 3 || version_id > VIRTIO_NET_VM_VERSION) > > This bit isn't right - how can this code load e.g. version 4? It can't. There's no way to load a save image for a version_id greater than VIRTIO_NET_VM_VERSION, because we don't know how much data to pop off or what state we'd be missing. We can only load older save images on equal or newer versions of qemu. As far as know... Thanks, Alex -- Alex Williamson HP Open Source & Linux Org.