From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57774) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wv2bK-0003yG-1V for qemu-devel@nongnu.org; Thu, 12 Jun 2014 06:51:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wv2bC-00048T-6h for qemu-devel@nongnu.org; Thu, 12 Jun 2014 06:51:21 -0400 Received: from e06smtp13.uk.ibm.com ([195.75.94.109]:34048) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wv2bB-00047u-UQ for qemu-devel@nongnu.org; Thu, 12 Jun 2014 06:51:14 -0400 Received: from /spool/local by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 12 Jun 2014 11:51:11 +0100 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id EF5701B08070 for ; Thu, 12 Jun 2014 11:51:35 +0100 (BST) Received: from d06av12.portsmouth.uk.ibm.com (d06av12.portsmouth.uk.ibm.com [9.149.37.247]) by b06cxnps4075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s5CAp95326673312 for ; Thu, 12 Jun 2014 10:51:09 GMT Received: from d06av12.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av12.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s5CAp7FJ006214 for ; Thu, 12 Jun 2014 04:51:08 -0600 Date: Thu, 12 Jun 2014 12:50:56 +0200 From: Greg Kurz Message-ID: <20140612125056.0c5c5dca@bahia.local> In-Reply-To: References: <20140514154130.10746.1412.stgit@bahia.local> <20140514154230.10746.56297.stgit@bahia.local> <5384A8D2.8050104@redhat.com> <20140529111253.4ff55199@bahia.local> <538708FA.4070309@redhat.com> <20140612094351.6295fd38@bahia.local> <53996B0E.4040808@redhat.com> <20140612110601.3fe5626e@bahia.local> <539970B3.3040304@redhat.com> <20140612093712.GA22565@redhat.com> <5399755E.10902@redhat.com> <53997638.6020403@suse.de> <20140612121440.6ce62e77@bahia.local> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC 8/8] virtio: add endian-ambivalent support to VirtIODevice List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Kevin Wolf , Fam Zheng , Anthony Liguori , Juan Quintela , "Michael S. Tsirkin" , "qemu-devel@nongnu.org" , Stefan Hajnoczi , Amit Shah , Paolo Bonzini , Andreas =?UTF-8?B?RsOkcmJlcg==?= On Thu, 12 Jun 2014 12:39:27 +0200 Alexander Graf wrote: > > > > Am 12.06.2014 um 12:14 schrieb Greg Kurz : > > > > On Thu, 12 Jun 2014 11:43:20 +0200 > > Alexander Graf wrote: > > > >> > >>> On 12.06.14 11:39, Paolo Bonzini wrote: > >>> Il 12/06/2014 11:37, Michael S. Tsirkin ha scritto: > >>>> Maybe just drop unnecessary stuff for new machine types? > >>>> Then we won't need hacks to migrate it. > >>> > >>> For any machine type it's trivial to migrate it. All machine types > >>> including old ones can disregard the migrated values. > >> > >> How about a patch like this before the actual LE awareness ones? With > >> this we should make virtio-serial config space completely independent of > >> live migration. > >> > >> Also since QEMU versions that do read these swapped values during > >> migration are not bi-endian aware, we can never get into a case where a > >> cross-endian save needs to be considered ;). > >> > >> > >> Alex > >> > >> > >> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c > >> index 2b647b6..73cb9b7 100644 > >> --- a/hw/char/virtio-serial-bus.c > >> +++ b/hw/char/virtio-serial-bus.c > >> @@ -670,6 +670,7 @@ static int virtio_serial_load(QEMUFile *f, void > >> *opaque, int version_id) > >> uint32_t max_nr_ports, nr_active_ports, ports_map; > >> unsigned int i; > >> int ret; > >> + uint32_t tmp; > >> > >> if (version_id > 3) { > >> return -EINVAL; > >> @@ -685,17 +686,12 @@ static int virtio_serial_load(QEMUFile *f, void > >> *opaque, int version_id) > >> return 0; > >> } > >> > >> - /* The config space */ > >> - qemu_get_be16s(f, &s->config.cols); > >> - qemu_get_be16s(f, &s->config.rows); > >> - > >> - qemu_get_be32s(f, &max_nr_ports); > >> - tswap32s(&max_nr_ports); > >> - if (max_nr_ports > tswap32(s->config.max_nr_ports)) { > >> - /* Source could have had more ports than us. Fail migration. */ > >> - return -EINVAL; > >> - } > >> + /* Unused */ > >> + qemu_get_be16s(f, &tmp); > >> + qemu_get_be16s(f, &tmp); > >> + qemu_get_be32s(f, &tmp); > >> > >> + max_nr_ports = tswap32(s->config.max_nr_ports); > >> for (i = 0; i < (max_nr_ports + 31) / 32; i++) { > >> qemu_get_be32s(f, &ports_map); > > > > For the moment, we have 0 < max_nr_ports < 32 so the source > > machine only stores a single 32 bit value... If this limit > > gets raised, we can end up sending more than that... and > > only the source machine max_nr_ports value can give the > > information... > > Why? This value only ever gets set in realize, so it will not change during the lifetime of the device - which means we don't need to migrate it. > I agree with the fact that the value does not change and should not be migrated in the first place. I am just worried about the size of the active ports bitmap that is streamed in the for loop... it is only 32 bit as of today, because we are limited to 32 ports. How would this work if the limit is raised ? How can the destination machine know how many bits have to be read ? -- Gregory Kurz kurzgreg@fr.ibm.com gkurz@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 "Anarchy is about taking complete responsibility for yourself." Alan Moore.