From patchwork Wed Jun 3 11:59:15 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Borntraeger X-Patchwork-Id: 479901 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BD7EA14027F for ; Wed, 3 Jun 2015 21:59:49 +1000 (AEST) Received: from localhost ([::1]:34818 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z07Kl-0004Bi-En for incoming@patchwork.ozlabs.org; Wed, 03 Jun 2015 07:59:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43277) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z07KR-0003v1-5b for qemu-devel@nongnu.org; Wed, 03 Jun 2015 07:59:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z07KN-0000U5-47 for qemu-devel@nongnu.org; Wed, 03 Jun 2015 07:59:27 -0400 Received: from e06smtp12.uk.ibm.com ([195.75.94.108]:35173) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z07KM-0000Sj-PV for qemu-devel@nongnu.org; Wed, 03 Jun 2015 07:59:23 -0400 Received: from /spool/local by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 3 Jun 2015 12:59:19 +0100 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Wed, 3 Jun 2015 12:59:18 +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 7AB961B08023 for ; Wed, 3 Jun 2015 13:00:12 +0100 (BST) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t53BxHpo23527510 for ; Wed, 3 Jun 2015 11:59:17 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t53BxGHQ015937 for ; Wed, 3 Jun 2015 05:59:17 -0600 Received: from oc1450873852.ibm.com (dyn-9-152-224-118.boeblingen.de.ibm.com [9.152.224.118]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id t53BxFAO015899; Wed, 3 Jun 2015 05:59:15 -0600 Message-ID: <556EEC13.2050204@de.ibm.com> Date: Wed, 03 Jun 2015 13:59:15 +0200 From: Christian Borntraeger User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Cornelia Huck , qemu-devel@nongnu.org References: <20150513165438-mutt-send-email-mst@redhat.com> <20150513170335.2d662124.cornelia.huck@de.ibm.com> <20150513180005-mutt-send-email-mst@redhat.com> <55539E7C.90004@de.ibm.com> <20150513234516-mutt-send-email-mst@redhat.com> <55546945.4050400@de.ibm.com> <20150514112845-mutt-send-email-mst@redhat.com> <55547938.4020201@de.ibm.com> <20150514170035.GJ2576@work-vm> <55559B57.5020704@de.ibm.com> <20150515091307-mutt-send-email-mst@redhat.com> <20150518132635.57971731.cornelia.huck@de.ibm.com> <20150518172928.43300b77.cornelia.huck@de.ibm.com> In-Reply-To: <20150518172928.43300b77.cornelia.huck@de.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15060311-0009-0000-0000-00000463ECB4 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 195.75.94.108 Cc: David Hildenbrand , quintela@redhat.com, "Dr. David Alan Gilbert" , jjherne@linux.vnet.ibm.com, "Michael S. Tsirkin" Subject: Re: [Qemu-devel] [PATCH RFC 1/1] virtio: migrate config_vector X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Am 18.05.2015 um 17:29 schrieb Cornelia Huck: > On Mon, 18 May 2015 13:26:35 +0200 > Cornelia Huck wrote: Had a discussion with Juan in irc regarding compatibility. As v2.3.0 is still on v2 for the machine and v2.4.0 will have v4 we could simply go with Jasons first approach that boild down to we only have to provide compatibility checks between releases. As there is no production envrionment yet we can break compatibility and the migration code will reject 2.3->2.4 and vice versa. In the future we have to be more careful, though. Juan, Conny virtio-ccw has no version as virtio has no vmstate. I am tempted to convert subch_device_save to use a vmstate_save_state instead of qemu_put*.... This would allow us to have a version for the virtio-ccw stuff only. This would be an incompatible change, though. Maybe we should continue to bump the machine version even if only the virtio migration format changes (which should rarely happen). Christian > >> Would the subsection approach be more acceptable if I default needed to >> false and have only virtio-ccw override it in a callback? > > Smth like the following: > > From 773a753475d9c89fb8dc789005a694d07b0cfac2 Mon Sep 17 00:00:00 2001 > From: Cornelia Huck > Date: Tue, 12 May 2015 09:14:45 +0200 > Subject: [PATCH] virtio: migrate config_vector > > Currently, config_vector is managed in VirtIODevice, but migration is > handled by the transport; this led to one transport using config_vector > migrating correctly (pci), while the other forgot it (ccw). > > We don't want to simply add a value to the virtio-ccw save data (it > may make migration failures hard to debug), and unfortunately we can't > add optional vmstate subsections to a transport only. So let's add > a new subsection for config_vector to the core and only let virtio-ccw > signal via an optional callback that it wants the config_vector to be > saved. > > Reported-by: "Jason J. Herne" > Signed-off-by: Cornelia Huck > --- > hw/s390x/virtio-ccw.c | 10 ++++++++++ > hw/virtio/virtio.c | 24 ++++++++++++++++++++++++ > include/hw/virtio/virtio-bus.h | 5 +++++ > 3 files changed, 39 insertions(+) > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index c96101a..dfb4514 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -1436,6 +1436,15 @@ static void virtio_ccw_device_unplugged(DeviceState *d) > > virtio_ccw_stop_ioeventfd(dev); > } > + > +static bool virtio_ccw_needs_confvec(DeviceState *d) > +{ > + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); > + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); > + > + return vdev && (vdev->config_vector != VIRTIO_NO_VECTOR); > +} > + > /**************** Virtio-ccw Bus Device Descriptions *******************/ > > static Property virtio_ccw_net_properties[] = { > @@ -1760,6 +1769,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data) > k->load_config = virtio_ccw_load_config; > k->device_plugged = virtio_ccw_device_plugged; > k->device_unplugged = virtio_ccw_device_unplugged; > + k->needs_confvec = virtio_ccw_needs_confvec; > } > > static const TypeInfo virtio_ccw_bus_info = { > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 6985e76..627be0d 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -903,6 +903,26 @@ static const VMStateDescription vmstate_virtio_device_endian = { > } > }; > > +static bool virtio_device_confvec_needed(void *opaque) > +{ > + VirtIODevice *vdev = opaque; > + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); > + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > + > + return k && k->needs_confvec ? > + k->needs_confvec(qbus->parent) : false; > +} > + > +static const VMStateDescription vmstate_virtio_device_confvec = { > + .name = "virtio/config_vector", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT16(config_vector, VirtIODevice), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static const VMStateDescription vmstate_virtio = { > .name = "virtio", > .version_id = 1, > @@ -916,6 +936,10 @@ static const VMStateDescription vmstate_virtio = { > .vmsd = &vmstate_virtio_device_endian, > .needed = &virtio_device_endian_needed > }, > + { > + .vmsd = &vmstate_virtio_device_confvec, > + .needed = &virtio_device_confvec_needed, > + }, > { 0 } > } > }; > diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h > index a4588ca..79e6e8b 100644 > --- a/include/hw/virtio/virtio-bus.h > +++ b/include/hw/virtio/virtio-bus.h > @@ -69,6 +69,11 @@ typedef struct VirtioBusClass { > * Note that changing this will break migration for this transport. > */ > bool has_variable_vring_alignment; > + /* > + * (optional) Does the bus want the core to handle config_vector > + * migration? This is for backwards compatibility only. > + */ > + bool (*needs_confvec)(DeviceState *d); > } VirtioBusClass; > > struct VirtioBusState { > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 9ef1059..13d9dda 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -1332,6 +1332,7 @@ static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) { VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); SubchDev *s = dev->sch; + VirtIODevice *vdev = virtio_ccw_get_vdev(s); subch_device_save(s, f); if (dev->indicators != NULL) { @@ -1355,6 +1356,7 @@ static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) qemu_put_be32(f, 0); qemu_put_be64(f, 0UL); } + qemu_put_be16(f, vdev->config_vector); qemu_put_be64(f, dev->routes.adapter.ind_offset); qemu_put_byte(f, dev->thinint_isc); } @@ -1363,6 +1365,7 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) { VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); SubchDev *s = dev->sch; + VirtIODevice *vdev = virtio_ccw_get_vdev(s); int len; s->driver_data = dev; @@ -1388,6 +1391,7 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) qemu_get_be64(f); dev->summary_indicator = NULL; } + qemu_get_be16s(f, &vdev->config_vector); dev->routes.adapter.ind_offset = qemu_get_be64(f); dev->thinint_isc = qemu_get_byte(f); if (s->thinint_active) {