From patchwork Tue May 9 17:05:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Halil Pasic X-Patchwork-Id: 760243 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 3wMm3b1mYdz9s0Z for ; Wed, 10 May 2017 03:06:03 +1000 (AEST) Received: from localhost ([::1]:38285 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d88aG-0008P5-Or for incoming@patchwork.ozlabs.org; Tue, 09 May 2017 13:06:00 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32841) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d88Zv-0008Mc-H9 for qemu-devel@nongnu.org; Tue, 09 May 2017 13:05:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d88Zr-0004Xf-FE for qemu-devel@nongnu.org; Tue, 09 May 2017 13:05:39 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:35141 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d88Zr-0004XV-84 for qemu-devel@nongnu.org; Tue, 09 May 2017 13:05:35 -0400 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v49GrwK6064237 for ; Tue, 9 May 2017 13:05:34 -0400 Received: from e06smtp12.uk.ibm.com (e06smtp12.uk.ibm.com [195.75.94.108]) by mx0b-001b2d01.pphosted.com with ESMTP id 2abatwkykv-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 09 May 2017 13:05:34 -0400 Received: from localhost by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 9 May 2017 18:05:32 +0100 Received: from b06cxnps4075.portsmouth.uk.ibm.com (9.149.109.197) by e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 9 May 2017 18:05:29 +0100 Received: from d06av24.portsmouth.uk.ibm.com (d06av24.portsmouth.uk.ibm.com [9.149.105.60]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v49H5T9T21430334; Tue, 9 May 2017 17:05:29 GMT Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6881D4203F; Tue, 9 May 2017 18:04:00 +0100 (BST) Received: from d06av24.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 22EE442041; Tue, 9 May 2017 18:04:00 +0100 (BST) Received: from oc3836556865.ibm.com (unknown [9.152.224.122]) by d06av24.portsmouth.uk.ibm.com (Postfix) with ESMTP; Tue, 9 May 2017 18:04:00 +0100 (BST) To: "Dr. David Alan Gilbert" References: <20170505173507.74077-1-pasic@linux.vnet.ibm.com> <20170505173507.74077-7-pasic@linux.vnet.ibm.com> <20170508165501.GH2120@work-vm> From: Halil Pasic Date: Tue, 9 May 2017 19:05:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170508165501.GH2120@work-vm> X-TM-AS-GCONF: 00 x-cbid: 17050917-0008-0000-0000-000004417FD2 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17050917-0009-0000-0000-00001D998C58 Message-Id: <52177e87-8d46-e1f6-49cf-397f296dd366@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-05-09_14:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1705090091 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy] X-Received-From: 148.163.158.5 Subject: Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Cornelia Huck , qemu-devel@nongnu.org, "Michael S. Tsirkin" Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" On 05/08/2017 06:55 PM, Dr. David Alan Gilbert wrote: > * Halil Pasic (pasic@linux.vnet.ibm.com) wrote: >> Let us use the freshly introduced vmstate migration helpers instead of >> saving/loading the config manually. >> >> To achieve this we need to hack the config_vector which is a common >> VirtIO state in the middle of the VirtioCcwDevice state representation. >> This somewhat ugly but we have no choice because the stream format needs >> to be preserved. >> >> Still no changes in behavior, but the dead code we added previously is >> finally awakening to life. >> >> Signed-off-by: Halil Pasic >> --- >> --- >> hw/s390x/virtio-ccw.c | 116 +++++++++++++++++++------------------------------- >> 1 file changed, 44 insertions(+), 72 deletions(-) >> >> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c >> index c2badfe..8ab655c 100644 >> --- a/hw/s390x/virtio-ccw.c >> +++ b/hw/s390x/virtio-ccw.c >> @@ -57,6 +57,32 @@ static int virtio_ccw_dev_post_load(void *opaque, int version_id) >> return 0; >> } >> >> +static int get_config_vector(QEMUFile *f, void *pv, size_t size, >> + VMStateField *field) >> +{ >> + VirtioCcwDevice *dev = pv; >> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >> + >> + qemu_get_be16s(f, &vdev->config_vector); >> + return 0; >> +} >> + >> +static int put_config_vector(QEMUFile *f, void *pv, size_t size, >> + VMStateField *field, QJSON *vmdesc) >> +{ >> + VirtioCcwDevice *dev = pv; >> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); >> + >> + qemu_put_be16(f, vdev->config_vector); >> + return 0; >> +} > Again that should be doable using WITH_TMP. > (I do wonder if we need a macro for cases where it's just a casting > operation to another type). > Yeah this one can be done with WITH_TMP. Below is the patch on top of this patch set. It's a bit more verbose (+6 lines) but it looks a bit nicer and probably also safer in (terms of symmetric read and write). If you think its the way to go I will squash it into this patch for the next version. -----------------------------8<----------------------------------------- From e92135590ab95cc565b37913de77a9ed17012933 Mon Sep 17 00:00:00 2001 From: Halil Pasic Date: Tue, 9 May 2017 16:01:50 +0200 Subject: [PATCH 1/2] virtio-ccw: replace info with VMSTATE_WITH_TMP Convert s VMSatateInfo based solution manipulating the migration stream directly to VMSTATE_WITH_TMP witch keeps IO and transformation logic separate. Signed-off-by: Halil Pasic --- hw/s390x/virtio-ccw.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index c611b6f..6ebc78a 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -57,30 +57,38 @@ static int virtio_ccw_dev_post_load(void *opaque, int version_id) return 0; } -static int get_config_vector(QEMUFile *f, void *pv, size_t size, - VMStateField *field) +typedef struct VirtioCcwDeviceTmp { + VirtioCcwDevice *parent; + uint16_t config_vector; +} VirtioCcwDeviceTmp; + +static void virtio_ccw_dev_tmp_pre_save(void *opaque) { - VirtioCcwDevice *dev = pv; + VirtioCcwDeviceTmp *tmp = opaque; + VirtioCcwDevice *dev = tmp->parent; VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); - qemu_get_be16s(f, &vdev->config_vector); - return 0; + tmp->config_vector = vdev->config_vector; } -static int put_config_vector(QEMUFile *f, void *pv, size_t size, - VMStateField *field, QJSON *vmdesc) +static int virtio_ccw_dev_tmp_post_load(void *opaque, int version_id) { - VirtioCcwDevice *dev = pv; + VirtioCcwDeviceTmp *tmp = opaque; + VirtioCcwDevice *dev = tmp->parent; VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); - qemu_put_be16(f, vdev->config_vector); + vdev->config_vector = tmp->config_vector; return 0; } -const VMStateInfo vmstate_info_config_vector = { - .name = "config_vector", - .get = get_config_vector, - .put = put_config_vector, +const VMStateDescription vmstate_virtio_ccw_dev_tmp = { + .name = "s390_virtio_ccw_dev_tmp", + .pre_save = virtio_ccw_dev_tmp_pre_save, + .post_load = virtio_ccw_dev_tmp_post_load, + .fields = (VMStateField[]) { + VMSTATE_UINT16(config_vector, VirtioCcwDeviceTmp), + VMSTATE_END_OF_LIST() + } }; const VMStateDescription vmstate_virtio_ccw_dev = { @@ -93,14 +101,12 @@ const VMStateDescription vmstate_virtio_ccw_dev = { VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), - { /* * Ugly hack because VirtIODevice does not migrate itself. * This also makes legacy via vmstate_save_state possible. */ - .name = "virtio/config_vector", - .info = &vmstate_info_config_vector, - }, + VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp, + vmstate_virtio_ccw_dev_tmp), VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes, \ AdapterRoutes), VMSTATE_UINT8(thinint_isc, VirtioCcwDevice),