[{"id":1771536,"web_url":"http://patchwork.ozlabs.org/comment/1771536/","msgid":"<20170920033134.GQ3617@pxdev.xzpeter.org>","list_archive_url":null,"date":"2017-09-20T03:31:34","subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","submitter":{"id":67717,"url":"http://patchwork.ozlabs.org/api/people/67717/","name":"Peter Xu","email":"peterx@redhat.com"},"content":"On Tue, Sep 19, 2017 at 07:00:38PM +0100, Dr. David Alan Gilbert (git) wrote:\n> From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n> \n> vmstate_save is called in a few places, and vmstate_save_state is\n> called in lots of places.\n> \n> Route error returns from the easier cases back up;  there are lots\n> of more complex cases where there own error paths need fixing.\n> \n> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>\n\nIIUC we are touching up the put()s hooks.  Then do we need this as\nwell below?\n\n-----\ndiff --git a/migration/vmstate.c b/migration/vmstate.c\nindex 3226e8e..70e1e14 100644\n--- a/migration/vmstate.c\n+++ b/migration/vmstate.c\n@@ -355,7 +355,10 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,\n                 } else if (field->flags & VMS_STRUCT) {\n                     vmstate_save_state(f, field->vmsd, curr_elem, vmdesc_loop);\n                 } else {\n-                    field->info->put(f, curr_elem, size, field, vmdesc_loop);\n+                    ret = field->info->put(f, curr_elem, size, field, vmdesc_loop);\n+                    if (ret) {\n+                        return ret;\n+                    }\n                 }\n \n                 written_bytes = qemu_ftell_fast(f) - old_offset;\n-----\n\nOr these errors won't really stop the migration?\n\n> ---\n>  hw/display/virtio-gpu.c    |  4 +---\n>  hw/virtio/virtio.c         | 13 +++++++------\n>  include/hw/virtio/virtio.h |  2 +-\n>  migration/vmstate-types.c  | 11 ++++++++---\n>  tests/test-vmstate.c       |  6 +++---\n>  5 files changed, 20 insertions(+), 16 deletions(-)\n> \n> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c\n> index 622ee300f9..c9494e8a79 100644\n> --- a/hw/display/virtio-gpu.c\n> +++ b/hw/display/virtio-gpu.c\n> @@ -996,9 +996,7 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,\n>      }\n>      qemu_put_be32(f, 0); /* end of list */\n>  \n> -    vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL);\n> -\n> -    return 0;\n> +    return vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL);\n>  }\n>  \n>  static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,\n> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c\n> index 464947f76d..860333788b 100644\n> --- a/hw/virtio/virtio.c\n> +++ b/hw/virtio/virtio.c\n> @@ -1899,7 +1899,7 @@ static const VMStateDescription vmstate_virtio = {\n>      }\n>  };\n>  \n> -void virtio_save(VirtIODevice *vdev, QEMUFile *f)\n> +int virtio_save(VirtIODevice *vdev, QEMUFile *f)\n>  {\n>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));\n>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);\n> @@ -1949,20 +1949,21 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)\n>      }\n>  \n>      if (vdc->vmsd) {\n> -        vmstate_save_state(f, vdc->vmsd, vdev, NULL);\n> +        int ret = vmstate_save_state(f, vdc->vmsd, vdev, NULL);\n> +        if (ret) {\n> +            return ret;\n> +        }\n>      }\n>  \n>      /* Subsections */\n> -    vmstate_save_state(f, &vmstate_virtio, vdev, NULL);\n> +    return vmstate_save_state(f, &vmstate_virtio, vdev, NULL);\n>  }\n>  \n>  /* A wrapper for use as a VMState .put function */\n>  static int virtio_device_put(QEMUFile *f, void *opaque, size_t size,\n>                                VMStateField *field, QJSON *vmdesc)\n>  {\n> -    virtio_save(VIRTIO_DEVICE(opaque), f);\n> -\n> -    return 0;\n> +    return virtio_save(VIRTIO_DEVICE(opaque), f);\n>  }\n>  \n>  /* A wrapper for use as a VMState .get function */\n> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h\n> index 80c45c321e..5abada6966 100644\n> --- a/include/hw/virtio/virtio.h\n> +++ b/include/hw/virtio/virtio.h\n> @@ -188,7 +188,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,\n>  void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq);\n>  void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);\n>  \n> -void virtio_save(VirtIODevice *vdev, QEMUFile *f);\n> +int virtio_save(VirtIODevice *vdev, QEMUFile *f);\n>  \n>  extern const VMStateInfo virtio_vmstate_info;\n>  \n> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c\n> index c056c98bdb..48184c380d 100644\n> --- a/migration/vmstate-types.c\n> +++ b/migration/vmstate-types.c\n> @@ -550,13 +550,14 @@ static int put_tmp(QEMUFile *f, void *pv, size_t size, VMStateField *field,\n>  {\n>      const VMStateDescription *vmsd = field->vmsd;\n>      void *tmp = g_malloc(size);\n> +    int ret;\n>  \n>      /* Writes the parent field which is at the start of the tmp */\n>      *(void **)tmp = pv;\n> -    vmstate_save_state(f, vmsd, tmp, vmdesc);\n> +    ret = vmstate_save_state(f, vmsd, tmp, vmdesc);\n>      g_free(tmp);\n>  \n> -    return 0;\n> +    return ret;\n>  }\n>  \n>  const VMStateInfo vmstate_info_tmp = {\n> @@ -657,12 +658,16 @@ static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size,\n>      /* offset of the QTAILQ entry in a QTAILQ element*/\n>      size_t entry_offset = field->start;\n>      void *elm;\n> +    int ret;\n>  \n>      trace_put_qtailq(vmsd->name, vmsd->version_id);\n>  \n>      QTAILQ_RAW_FOREACH(elm, pv, entry_offset) {\n>          qemu_put_byte(f, true);\n> -        vmstate_save_state(f, vmsd, elm, vmdesc);\n> +        ret = vmstate_save_state(f, vmsd, elm, vmdesc);\n> +        if (ret) {\n> +            return ret;\n> +        }\n>      }\n>      qemu_put_byte(f, false);\n>  \n> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c\n> index e643ac662b..ab3e430c2c 100644\n> --- a/tests/test-vmstate.c\n> +++ b/tests/test-vmstate.c\n> @@ -70,7 +70,7 @@ static void save_vmstate(const VMStateDescription *desc, void *obj)\n>      QEMUFile *f = open_test_file(true);\n>  \n>      /* Save file with vmstate */\n> -    vmstate_save_state(f, desc, obj, NULL);\n> +    g_assert(!vmstate_save_state(f, desc, obj, NULL));\n>      qemu_put_byte(f, QEMU_VM_EOF);\n>      g_assert(!qemu_file_get_error(f));\n>      qemu_fclose(f);\n> @@ -381,7 +381,7 @@ static void test_save_noskip(void)\n>      QEMUFile *fsave = open_test_file(true);\n>      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,\n>                         .skip_c_e = false };\n> -    vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));\n>      g_assert(!qemu_file_get_error(fsave));\n>  \n>      uint8_t expected[] = {\n> @@ -402,7 +402,7 @@ static void test_save_skip(void)\n>      QEMUFile *fsave = open_test_file(true);\n>      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,\n>                         .skip_c_e = true };\n> -    vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));\n>      g_assert(!qemu_file_get_error(fsave));\n>  \n>      uint8_t expected[] = {\n> -- \n> 2.13.5\n>","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=peterx@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxlpb3Q7Hz9s7h\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 20 Sep 2017 13:39:03 +1000 (AEST)","from localhost ([::1]:46501 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duVqn-0000XK-JD\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 23:39:01 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:47913)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <peterx@redhat.com>) id 1duVjo-0004PR-Ix\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 23:31:49 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <peterx@redhat.com>) id 1duVjh-0000Kt-Fk\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 23:31:48 -0400","from mx1.redhat.com ([209.132.183.28]:42548)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <peterx@redhat.com>) id 1duVjh-0000IY-64\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 23:31:41 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 2367180468;\n\tWed, 20 Sep 2017 03:31:40 +0000 (UTC)","from pxdev.xzpeter.org (dhcp-15-224.nay.redhat.com [10.66.15.224])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id D21BF5C542;\n\tWed, 20 Sep 2017 03:31:37 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 2367180468","Date":"Wed, 20 Sep 2017 11:31:34 +0800","From":"Peter Xu <peterx@redhat.com>","To":"\"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com>","Message-ID":"<20170920033134.GQ3617@pxdev.xzpeter.org>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-6-dgilbert@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170919180038.26056-6-dgilbert@redhat.com>","User-Agent":"Mutt/1.5.24 (2015-08-30)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tWed, 20 Sep 2017 03:31:40 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"peter.maydell@linaro.org, rth@twiddle.net, cohuck@redhat.com,\n\tqemu-devel@nongnu.org, quintela@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771546,"web_url":"http://patchwork.ozlabs.org/comment/1771546/","msgid":"<20170920035512.GF18491@lemon>","list_archive_url":null,"date":"2017-09-20T03:55:12","subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","submitter":{"id":24872,"url":"http://patchwork.ozlabs.org/api/people/24872/","name":"Fam Zheng","email":"famz@redhat.com"},"content":"On Tue, 09/19 19:00, Dr. David Alan Gilbert (git) wrote:\n> From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n> \n> vmstate_save is called in a few places, and vmstate_save_state is\n> called in lots of places.\n> \n> Route error returns from the easier cases back up;  there are lots\n> of more complex cases where there own error paths need fixing.\n\nDid you mean s/there/their/ ?\n\n<snip>\n\n> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c\n> index e643ac662b..ab3e430c2c 100644\n> --- a/tests/test-vmstate.c\n> +++ b/tests/test-vmstate.c\n> @@ -70,7 +70,7 @@ static void save_vmstate(const VMStateDescription *desc, void *obj)\n>      QEMUFile *f = open_test_file(true);\n>  \n>      /* Save file with vmstate */\n> -    vmstate_save_state(f, desc, obj, NULL);\n> +    g_assert(!vmstate_save_state(f, desc, obj, NULL));\n\nThough this is test code, isn't putting anything with a side effect into an\nassert expression a very bad pattern in general?\n\n>      qemu_put_byte(f, QEMU_VM_EOF);\n>      g_assert(!qemu_file_get_error(f));\n>      qemu_fclose(f);\n> @@ -381,7 +381,7 @@ static void test_save_noskip(void)\n>      QEMUFile *fsave = open_test_file(true);\n>      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,\n>                         .skip_c_e = false };\n> -    vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));\n>      g_assert(!qemu_file_get_error(fsave));\n>  \n>      uint8_t expected[] = {\n> @@ -402,7 +402,7 @@ static void test_save_skip(void)\n>      QEMUFile *fsave = open_test_file(true);\n>      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,\n>                         .skip_c_e = true };\n> -    vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));\n>      g_assert(!qemu_file_get_error(fsave));\n>  \n>      uint8_t expected[] = {\n> -- \n> 2.13.5\n> \n> \n\nFam","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=famz@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxmB56NRTz9s2G\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 20 Sep 2017 13:55:56 +1000 (AEST)","from localhost ([::1]:46545 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duW78-0004eP-Ds\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 23:55:54 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:32797)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1duW6e-0004e9-SG\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 23:55:25 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <famz@redhat.com>) id 1duW6b-0000ey-Ql\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 23:55:24 -0400","from mx1.redhat.com ([209.132.183.28]:39824)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <famz@redhat.com>) id 1duW6b-0000aw-Lf\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 23:55:21 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 79D8D5F73B;\n\tWed, 20 Sep 2017 03:55:20 +0000 (UTC)","from localhost (ovpn-12-90.pek2.redhat.com [10.72.12.90])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id E7C0A6062F;\n\tWed, 20 Sep 2017 03:55:14 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 79D8D5F73B","Date":"Wed, 20 Sep 2017 11:55:12 +0800","From":"Fam Zheng <famz@redhat.com>","To":"\"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com>","Message-ID":"<20170920035512.GF18491@lemon>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-6-dgilbert@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170919180038.26056-6-dgilbert@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tWed, 20 Sep 2017 03:55:20 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"peter.maydell@linaro.org, quintela@redhat.com, cohuck@redhat.com,\n\tqemu-devel@nongnu.org, peterx@redhat.com, rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771857,"web_url":"http://patchwork.ozlabs.org/comment/1771857/","msgid":"<5b02462e-99c3-eb72-3481-93a26fc405e4@redhat.com>","list_archive_url":null,"date":"2017-09-20T13:26:22","subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 09/19/2017 10:55 PM, Fam Zheng wrote:\n> On Tue, 09/19 19:00, Dr. David Alan Gilbert (git) wrote:\n>> From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n>>\n>> vmstate_save is called in a few places, and vmstate_save_state is\n>> called in lots of places.\n>>\n>> Route error returns from the easier cases back up;  there are lots\n>> of more complex cases where there own error paths need fixing.\n> \n\n>> +++ b/tests/test-vmstate.c\n>> @@ -70,7 +70,7 @@ static void save_vmstate(const VMStateDescription *desc, void *obj)\n>>      QEMUFile *f = open_test_file(true);\n>>  \n>>      /* Save file with vmstate */\n>> -    vmstate_save_state(f, desc, obj, NULL);\n>> +    g_assert(!vmstate_save_state(f, desc, obj, NULL));\n> \n> Though this is test code, isn't putting anything with a side effect into an\n> assert expression a very bad pattern in general?\n\nIndeed - although we don't disable asserts (as of commit 262a69f4), this\nshould still be separated into running vmstate_save_state()\nunconditionally, then asserting that the result stored into a temporary\nvariable matches expectations.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy18s3Gvwz9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 20 Sep 2017 23:40:45 +1000 (AEST)","from localhost ([::1]:48146 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1dufF5-00051q-Gv\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 09:40:43 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:36435)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1duf1p-0002vF-A8\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:27:20 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1duf1M-00042b-3p\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:27:01 -0400","from mx1.redhat.com ([209.132.183.28]:52302)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <eblake@redhat.com>) id 1duf1L-00042C-R9\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:26:32 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id E943D883C8;\n\tWed, 20 Sep 2017 13:26:30 +0000 (UTC)","from [10.10.124.97] (ovpn-124-97.rdu2.redhat.com [10.10.124.97])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 439E25D6A8;\n\tWed, 20 Sep 2017 13:26:23 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com E943D883C8","To":"Fam Zheng <famz@redhat.com>,\n\t\"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-6-dgilbert@redhat.com>\n\t<20170920035512.GF18491@lemon>","From":"Eric Blake <eblake@redhat.com>","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<5b02462e-99c3-eb72-3481-93a26fc405e4@redhat.com>","Date":"Wed, 20 Sep 2017 08:26:22 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170920035512.GF18491@lemon>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"jj3e063sv0Gnj0TIdrqOqKiCqqMdS7KnO\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tWed, 20 Sep 2017 13:26:31 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"peter.maydell@linaro.org, quintela@redhat.com, cohuck@redhat.com,\n\tqemu-devel@nongnu.org, peterx@redhat.com, rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771977,"web_url":"http://patchwork.ozlabs.org/comment/1771977/","msgid":"<20170920142040.GE2449@work-vm>","list_archive_url":null,"date":"2017-09-20T14:20:41","subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","submitter":{"id":48102,"url":"http://patchwork.ozlabs.org/api/people/48102/","name":"Dr. David Alan Gilbert","email":"dgilbert@redhat.com"},"content":"* Fam Zheng (famz@redhat.com) wrote:\n> On Tue, 09/19 19:00, Dr. David Alan Gilbert (git) wrote:\n> > From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n> > \n> > vmstate_save is called in a few places, and vmstate_save_state is\n> > called in lots of places.\n> > \n> > Route error returns from the easier cases back up;  there are lots\n> > of more complex cases where there own error paths need fixing.\n> \n> Did you mean s/there/their/ ?\n\nI do; thanks.\n\n> <snip>\n> \n> > diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c\n> > index e643ac662b..ab3e430c2c 100644\n> > --- a/tests/test-vmstate.c\n> > +++ b/tests/test-vmstate.c\n> > @@ -70,7 +70,7 @@ static void save_vmstate(const VMStateDescription *desc, void *obj)\n> >      QEMUFile *f = open_test_file(true);\n> >  \n> >      /* Save file with vmstate */\n> > -    vmstate_save_state(f, desc, obj, NULL);\n> > +    g_assert(!vmstate_save_state(f, desc, obj, NULL));\n> \n> Though this is test code, isn't putting anything with a side effect into an\n> assert expression a very bad pattern in general?\n\nHmm; ok I've changed this but I'm not really convinced; the whole point\nof an asser in a test is to actually run it, and I think the g_assert\nprints the text that failed, so it gives you a much better error inline.\n\nDave\n\n> >      qemu_put_byte(f, QEMU_VM_EOF);\n> >      g_assert(!qemu_file_get_error(f));\n> >      qemu_fclose(f);\n> > @@ -381,7 +381,7 @@ static void test_save_noskip(void)\n> >      QEMUFile *fsave = open_test_file(true);\n> >      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,\n> >                         .skip_c_e = false };\n> > -    vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> > +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));\n> >      g_assert(!qemu_file_get_error(fsave));\n> >  \n> >      uint8_t expected[] = {\n> > @@ -402,7 +402,7 @@ static void test_save_skip(void)\n> >      QEMUFile *fsave = open_test_file(true);\n> >      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,\n> >                         .skip_c_e = true };\n> > -    vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> > +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));\n> >      g_assert(!qemu_file_get_error(fsave));\n> >  \n> >      uint8_t expected[] = {\n> > -- \n> > 2.13.5\n> > \n> > \n> \n> Fam\n--\nDr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=dgilbert@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy3KC0t9fz9s7B\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 21 Sep 2017 01:18:07 +1000 (AEST)","from localhost ([::1]:48785 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duglJ-0005SG-4v\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 11:18:05 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:58092)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <dgilbert@redhat.com>) id 1dufrx-0000Yp-3m\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:20:54 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <dgilbert@redhat.com>) id 1dufrt-0007KY-2l\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:20:53 -0400","from mx1.redhat.com ([209.132.183.28]:59414)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <dgilbert@redhat.com>) id 1dufrs-0007Jr-Oq\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:20:48 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id CC0B7C00DBBD;\n\tWed, 20 Sep 2017 14:20:47 +0000 (UTC)","from work-vm (ovpn-117-183.ams2.redhat.com [10.36.117.183])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 3437A5C8B0;\n\tWed, 20 Sep 2017 14:20:43 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com CC0B7C00DBBD","Date":"Wed, 20 Sep 2017 15:20:41 +0100","From":"\"Dr. David Alan Gilbert\" <dgilbert@redhat.com>","To":"Fam Zheng <famz@redhat.com>","Message-ID":"<20170920142040.GE2449@work-vm>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-6-dgilbert@redhat.com>\n\t<20170920035512.GF18491@lemon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170920035512.GF18491@lemon>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tWed, 20 Sep 2017 14:20:47 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"peter.maydell@linaro.org, quintela@redhat.com, cohuck@redhat.com,\n\tqemu-devel@nongnu.org, peterx@redhat.com, rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1771989,"web_url":"http://patchwork.ozlabs.org/comment/1771989/","msgid":"<20170920163059.2ddd456c.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-20T14:30:59","subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Tue, 19 Sep 2017 19:00:38 +0100\n\"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com> wrote:\n\n> From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n> \n> vmstate_save is called in a few places, and vmstate_save_state is\n> called in lots of places.\n> \n> Route error returns from the easier cases back up;  there are lots\n> of more complex cases where there own error paths need fixing.\n> \n> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>\n> ---\n>  hw/display/virtio-gpu.c    |  4 +---\n>  hw/virtio/virtio.c         | 13 +++++++------\n>  include/hw/virtio/virtio.h |  2 +-\n>  migration/vmstate-types.c  | 11 ++++++++---\n>  tests/test-vmstate.c       |  6 +++---\n>  5 files changed, 20 insertions(+), 16 deletions(-)\n> \n\n> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c\n> index 464947f76d..860333788b 100644\n> --- a/hw/virtio/virtio.c\n> +++ b/hw/virtio/virtio.c\n> @@ -1899,7 +1899,7 @@ static const VMStateDescription vmstate_virtio = {\n>      }\n>  };\n>  \n> -void virtio_save(VirtIODevice *vdev, QEMUFile *f)\n> +int virtio_save(VirtIODevice *vdev, QEMUFile *f)\n>  {\n>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));\n>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);\n\nWould it make sense to touch up the save_config callback as well? Else\nvirtio_save() looks a bit lopsided.\n\n[For virtio-ccw, the callback can simply pass on any return code from\nvmstate_save_state(). For virtio-pci, we can probably make\npci_device_save() restore the interrupt state in any case.]","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy3fk63NRz9sPt\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 21 Sep 2017 01:33:18 +1000 (AEST)","from localhost ([::1]:48899 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duh00-0001WU-SQ\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 11:33:16 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:60943)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dug21-0002TI-L1\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:31:25 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dug1w-0003h9-41\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:31:17 -0400","from mx1.redhat.com ([209.132.183.28]:59052)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <cohuck@redhat.com>) id 1dug1v-0003gw-Ur\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:31:12 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 091F6C047B74;\n\tWed, 20 Sep 2017 14:31:11 +0000 (UTC)","from gondolin (ovpn-117-98.ams2.redhat.com [10.36.117.98])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 09AF45D75A;\n\tWed, 20 Sep 2017 14:31:02 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 091F6C047B74","Date":"Wed, 20 Sep 2017 16:30:59 +0200","From":"Cornelia Huck <cohuck@redhat.com>","To":"\"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com>","Message-ID":"<20170920163059.2ddd456c.cohuck@redhat.com>","In-Reply-To":"<20170919180038.26056-6-dgilbert@redhat.com>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-6-dgilbert@redhat.com>","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tWed, 20 Sep 2017 14:31:11 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"peter.maydell@linaro.org, rth@twiddle.net, qemu-devel@nongnu.org,\n\tpeterx@redhat.com, quintela@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1772022,"web_url":"http://patchwork.ozlabs.org/comment/1772022/","msgid":"<80ea7936-97b1-7d82-1373-149d33a811f7@redhat.com>","list_archive_url":null,"date":"2017-09-20T15:32:26","subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","submitter":{"id":6591,"url":"http://patchwork.ozlabs.org/api/people/6591/","name":"Eric Blake","email":"eblake@redhat.com"},"content":"On 09/20/2017 09:20 AM, Dr. David Alan Gilbert wrote:\n\n>>>      /* Save file with vmstate */\n>>> -    vmstate_save_state(f, desc, obj, NULL);\n>>> +    g_assert(!vmstate_save_state(f, desc, obj, NULL));\n>>\n>> Though this is test code, isn't putting anything with a side effect into an\n>> assert expression a very bad pattern in general?\n> \n> Hmm; ok I've changed this but I'm not really convinced; the whole point\n> of an asser in a test is to actually run it, and I think the g_assert\n> prints the text that failed, so it gives you a much better error inline.\n\nglib doesn't make life any easier by having two different flavors of\nassertions.\n\ng_assert() can be disabled at compile-time (side effects are lost).  It\nis roughly akin to assert(); and the output when an assertion fails\nisn't that much more useful than what you get with plain assert().\n\ng_assert_cmpint() cannot be disabled at compile-time, but can have a\nruntime switch on whether it abort()s or continues on.  Here, the output\nwhen something fails is MUCH more pleasant to read.\n\nBut the fact that there are two separate families of g_assert* behaviors\njust makes it easier to live by the mantra that side-effects don't\nbelong inside assertions.","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx09.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eblake@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy4LJ6nSrz9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 21 Sep 2017 02:04:08 +1000 (AEST)","from localhost ([::1]:49229 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duhTq-0002NA-UO\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 12:04:06 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:58754)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1dugzN-0001cJ-IU\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 11:32:38 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <eblake@redhat.com>) id 1dugzK-0001NE-8f\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 11:32:37 -0400","from mx1.redhat.com ([209.132.183.28]:38418)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <eblake@redhat.com>) id 1dugzJ-0001MA-NP\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 11:32:34 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id C12C44A708;\n\tWed, 20 Sep 2017 15:32:32 +0000 (UTC)","from [10.10.124.97] (ovpn-124-97.rdu2.redhat.com [10.10.124.97])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 0C3AD60C9A;\n\tWed, 20 Sep 2017 15:32:26 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com C12C44A708","To":"\"Dr. David Alan Gilbert\" <dgilbert@redhat.com>,\n\tFam Zheng <famz@redhat.com>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-6-dgilbert@redhat.com>\n\t<20170920035512.GF18491@lemon> <20170920142040.GE2449@work-vm>","From":"Eric Blake <eblake@redhat.com>","Openpgp":"url=http://people.redhat.com/eblake/eblake.gpg","Organization":"Red Hat, Inc.","Message-ID":"<80ea7936-97b1-7d82-1373-149d33a811f7@redhat.com>","Date":"Wed, 20 Sep 2017 10:32:26 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170920142040.GE2449@work-vm>","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"AxhdUuIgcAmLbbf8daeRbni1khqlCDxSr\"","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.38]);\n\tWed, 20 Sep 2017 15:32:32 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","X-Content-Filtered-By":"Mailman/MimeDel 2.1.21","Subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"peter.maydell@linaro.org, quintela@redhat.com, cohuck@redhat.com,\n\tqemu-devel@nongnu.org, peterx@redhat.com, rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1772026,"web_url":"http://patchwork.ozlabs.org/comment/1772026/","msgid":"<20170920180032.567c49a7.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-20T16:00:32","subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","submitter":{"id":71914,"url":"http://patchwork.ozlabs.org/api/people/71914/","name":"Cornelia Huck","email":"cohuck@redhat.com"},"content":"On Wed, 20 Sep 2017 16:57:39 +0100\n\"Dr. David Alan Gilbert\" <dgilbert@redhat.com> wrote:\n\n> * Cornelia Huck (cohuck@redhat.com) wrote:\n> > On Tue, 19 Sep 2017 19:00:38 +0100\n> > \"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com> wrote:\n> >   \n> > > From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n> > > \n> > > vmstate_save is called in a few places, and vmstate_save_state is\n> > > called in lots of places.\n> > > \n> > > Route error returns from the easier cases back up;  there are lots\n> > > of more complex cases where there own error paths need fixing.\n> > > \n> > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>\n> > > ---\n> > >  hw/display/virtio-gpu.c    |  4 +---\n> > >  hw/virtio/virtio.c         | 13 +++++++------\n> > >  include/hw/virtio/virtio.h |  2 +-\n> > >  migration/vmstate-types.c  | 11 ++++++++---\n> > >  tests/test-vmstate.c       |  6 +++---\n> > >  5 files changed, 20 insertions(+), 16 deletions(-)\n> > >   \n> >   \n> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c\n> > > index 464947f76d..860333788b 100644\n> > > --- a/hw/virtio/virtio.c\n> > > +++ b/hw/virtio/virtio.c\n> > > @@ -1899,7 +1899,7 @@ static const VMStateDescription vmstate_virtio = {\n> > >      }\n> > >  };\n> > >  \n> > > -void virtio_save(VirtIODevice *vdev, QEMUFile *f)\n> > > +int virtio_save(VirtIODevice *vdev, QEMUFile *f)\n> > >  {\n> > >      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));\n> > >      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);  \n> > \n> > Would it make sense to touch up the save_config callback as well? Else\n> > virtio_save() looks a bit lopsided.\n> > \n> > [For virtio-ccw, the callback can simply pass on any return code from\n> > vmstate_save_state(). For virtio-pci, we can probably make\n> > pci_device_save() restore the interrupt state in any case.]  \n> \n> What I'd really love to do with save_config is turn it into a\n> VMStateDescription* - I think I've figured out how to\n> do that for ccw and mmio - but the pci version is a much harder\n> mess of casts and stuff.\n\nSounds even better, but even more complicated as well...","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=cohuck@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy4TF4qNyz9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 21 Sep 2017 02:10:09 +1000 (AEST)","from localhost ([::1]:49355 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duhZf-0007JU-Kb\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 12:10:07 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:43403)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1duhQd-00005c-Oa\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 12:00:50 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1duhQX-0007BA-C6\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 12:00:47 -0400","from mx1.redhat.com ([209.132.183.28]:28270)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <cohuck@redhat.com>) id 1duhQX-00078y-5x\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 12:00:41 -0400","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id EC7EB7F3E2;\n\tWed, 20 Sep 2017 16:00:39 +0000 (UTC)","from gondolin (ovpn-116-108.ams2.redhat.com [10.36.116.108])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id DA36960472;\n\tWed, 20 Sep 2017 16:00:35 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com EC7EB7F3E2","Date":"Wed, 20 Sep 2017 18:00:32 +0200","From":"Cornelia Huck <cohuck@redhat.com>","To":"\"Dr. David Alan Gilbert\" <dgilbert@redhat.com>","Message-ID":"<20170920180032.567c49a7.cohuck@redhat.com>","In-Reply-To":"<20170920155739.GI2449@work-vm>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-6-dgilbert@redhat.com>\n\t<20170920163059.2ddd456c.cohuck@redhat.com>\n\t<20170920155739.GI2449@work-vm>","Organization":"Red Hat GmbH","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tWed, 20 Sep 2017 16:00:40 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"peter.maydell@linaro.org, rth@twiddle.net, qemu-devel@nongnu.org,\n\tpeterx@redhat.com, quintela@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1772031,"web_url":"http://patchwork.ozlabs.org/comment/1772031/","msgid":"<20170920155739.GI2449@work-vm>","list_archive_url":null,"date":"2017-09-20T15:57:39","subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","submitter":{"id":48102,"url":"http://patchwork.ozlabs.org/api/people/48102/","name":"Dr. David Alan Gilbert","email":"dgilbert@redhat.com"},"content":"* Cornelia Huck (cohuck@redhat.com) wrote:\n> On Tue, 19 Sep 2017 19:00:38 +0100\n> \"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com> wrote:\n> \n> > From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n> > \n> > vmstate_save is called in a few places, and vmstate_save_state is\n> > called in lots of places.\n> > \n> > Route error returns from the easier cases back up;  there are lots\n> > of more complex cases where there own error paths need fixing.\n> > \n> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>\n> > ---\n> >  hw/display/virtio-gpu.c    |  4 +---\n> >  hw/virtio/virtio.c         | 13 +++++++------\n> >  include/hw/virtio/virtio.h |  2 +-\n> >  migration/vmstate-types.c  | 11 ++++++++---\n> >  tests/test-vmstate.c       |  6 +++---\n> >  5 files changed, 20 insertions(+), 16 deletions(-)\n> > \n> \n> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c\n> > index 464947f76d..860333788b 100644\n> > --- a/hw/virtio/virtio.c\n> > +++ b/hw/virtio/virtio.c\n> > @@ -1899,7 +1899,7 @@ static const VMStateDescription vmstate_virtio = {\n> >      }\n> >  };\n> >  \n> > -void virtio_save(VirtIODevice *vdev, QEMUFile *f)\n> > +int virtio_save(VirtIODevice *vdev, QEMUFile *f)\n> >  {\n> >      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));\n> >      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);\n> \n> Would it make sense to touch up the save_config callback as well? Else\n> virtio_save() looks a bit lopsided.\n> \n> [For virtio-ccw, the callback can simply pass on any return code from\n> vmstate_save_state(). For virtio-pci, we can probably make\n> pci_device_save() restore the interrupt state in any case.]\n\nWhat I'd really love to do with save_config is turn it into a\nVMStateDescription* - I think I've figured out how to\ndo that for ccw and mmio - but the pci version is a much harder\nmess of casts and stuff.\n\nDave\n\n--\nDr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=dgilbert@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy4Zr3jJ4z9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 21 Sep 2017 02:15:00 +1000 (AEST)","from localhost ([::1]:49391 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duheM-00032t-IZ\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 12:14:58 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:39150)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <dgilbert@redhat.com>) id 1duhNo-0005Z7-OS\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 11:57:53 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <dgilbert@redhat.com>) id 1duhNk-0001TV-Nm\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 11:57:52 -0400","from mx1.redhat.com ([209.132.183.28]:37056)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <dgilbert@redhat.com>) id 1duhNk-0001Qu-DF\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 11:57:48 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 2E8FC51165;\n\tWed, 20 Sep 2017 15:57:47 +0000 (UTC)","from work-vm (ovpn-117-183.ams2.redhat.com [10.36.117.183])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 216AD5C670;\n\tWed, 20 Sep 2017 15:57:41 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 2E8FC51165","Date":"Wed, 20 Sep 2017 16:57:39 +0100","From":"\"Dr. David Alan Gilbert\" <dgilbert@redhat.com>","To":"Cornelia Huck <cohuck@redhat.com>","Message-ID":"<20170920155739.GI2449@work-vm>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-6-dgilbert@redhat.com>\n\t<20170920163059.2ddd456c.cohuck@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170920163059.2ddd456c.cohuck@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tWed, 20 Sep 2017 15:57:47 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"peter.maydell@linaro.org, rth@twiddle.net, qemu-devel@nongnu.org,\n\tpeterx@redhat.com, quintela@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}},{"id":1772043,"web_url":"http://patchwork.ozlabs.org/comment/1772043/","msgid":"<20170920154558.GH2449@work-vm>","list_archive_url":null,"date":"2017-09-20T15:45:59","subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","submitter":{"id":48102,"url":"http://patchwork.ozlabs.org/api/people/48102/","name":"Dr. David Alan Gilbert","email":"dgilbert@redhat.com"},"content":"* Peter Xu (peterx@redhat.com) wrote:\n> On Tue, Sep 19, 2017 at 07:00:38PM +0100, Dr. David Alan Gilbert (git) wrote:\n> > From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n> > \n> > vmstate_save is called in a few places, and vmstate_save_state is\n> > called in lots of places.\n> > \n> > Route error returns from the easier cases back up;  there are lots\n> > of more complex cases where there own error paths need fixing.\n> > \n> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>\n> \n> IIUC we are touching up the put()s hooks.  Then do we need this as\n> well below?\n\nYou're right - 'put' has returned int since Jianjun's patches but\nI seem to have missed that it never wired the check up.\n\n> -----\n> diff --git a/migration/vmstate.c b/migration/vmstate.c\n> index 3226e8e..70e1e14 100644\n> --- a/migration/vmstate.c\n> +++ b/migration/vmstate.c\n> @@ -355,7 +355,10 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,\n>                  } else if (field->flags & VMS_STRUCT) {\n>                      vmstate_save_state(f, field->vmsd, curr_elem, vmdesc_loop);\n>                  } else {\n> -                    field->info->put(f, curr_elem, size, field, vmdesc_loop);\n> +                    ret = field->info->put(f, curr_elem, size, field, vmdesc_loop);\n> +                    if (ret) {\n> +                        return ret;\n> +                    }\n>                  }\n>  \n>                  written_bytes = qemu_ftell_fast(f) - old_offset;\n> -----\n> \n> Or these errors won't really stop the migration?\n\nAdded as a separate patch (with an error report in the if)\n\nThanks,\n\nDave\n\n> \n> > ---\n> >  hw/display/virtio-gpu.c    |  4 +---\n> >  hw/virtio/virtio.c         | 13 +++++++------\n> >  include/hw/virtio/virtio.h |  2 +-\n> >  migration/vmstate-types.c  | 11 ++++++++---\n> >  tests/test-vmstate.c       |  6 +++---\n> >  5 files changed, 20 insertions(+), 16 deletions(-)\n> > \n> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c\n> > index 622ee300f9..c9494e8a79 100644\n> > --- a/hw/display/virtio-gpu.c\n> > +++ b/hw/display/virtio-gpu.c\n> > @@ -996,9 +996,7 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,\n> >      }\n> >      qemu_put_be32(f, 0); /* end of list */\n> >  \n> > -    vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL);\n> > -\n> > -    return 0;\n> > +    return vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL);\n> >  }\n> >  \n> >  static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,\n> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c\n> > index 464947f76d..860333788b 100644\n> > --- a/hw/virtio/virtio.c\n> > +++ b/hw/virtio/virtio.c\n> > @@ -1899,7 +1899,7 @@ static const VMStateDescription vmstate_virtio = {\n> >      }\n> >  };\n> >  \n> > -void virtio_save(VirtIODevice *vdev, QEMUFile *f)\n> > +int virtio_save(VirtIODevice *vdev, QEMUFile *f)\n> >  {\n> >      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));\n> >      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);\n> > @@ -1949,20 +1949,21 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)\n> >      }\n> >  \n> >      if (vdc->vmsd) {\n> > -        vmstate_save_state(f, vdc->vmsd, vdev, NULL);\n> > +        int ret = vmstate_save_state(f, vdc->vmsd, vdev, NULL);\n> > +        if (ret) {\n> > +            return ret;\n> > +        }\n> >      }\n> >  \n> >      /* Subsections */\n> > -    vmstate_save_state(f, &vmstate_virtio, vdev, NULL);\n> > +    return vmstate_save_state(f, &vmstate_virtio, vdev, NULL);\n> >  }\n> >  \n> >  /* A wrapper for use as a VMState .put function */\n> >  static int virtio_device_put(QEMUFile *f, void *opaque, size_t size,\n> >                                VMStateField *field, QJSON *vmdesc)\n> >  {\n> > -    virtio_save(VIRTIO_DEVICE(opaque), f);\n> > -\n> > -    return 0;\n> > +    return virtio_save(VIRTIO_DEVICE(opaque), f);\n> >  }\n> >  \n> >  /* A wrapper for use as a VMState .get function */\n> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h\n> > index 80c45c321e..5abada6966 100644\n> > --- a/include/hw/virtio/virtio.h\n> > +++ b/include/hw/virtio/virtio.h\n> > @@ -188,7 +188,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,\n> >  void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq);\n> >  void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);\n> >  \n> > -void virtio_save(VirtIODevice *vdev, QEMUFile *f);\n> > +int virtio_save(VirtIODevice *vdev, QEMUFile *f);\n> >  \n> >  extern const VMStateInfo virtio_vmstate_info;\n> >  \n> > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c\n> > index c056c98bdb..48184c380d 100644\n> > --- a/migration/vmstate-types.c\n> > +++ b/migration/vmstate-types.c\n> > @@ -550,13 +550,14 @@ static int put_tmp(QEMUFile *f, void *pv, size_t size, VMStateField *field,\n> >  {\n> >      const VMStateDescription *vmsd = field->vmsd;\n> >      void *tmp = g_malloc(size);\n> > +    int ret;\n> >  \n> >      /* Writes the parent field which is at the start of the tmp */\n> >      *(void **)tmp = pv;\n> > -    vmstate_save_state(f, vmsd, tmp, vmdesc);\n> > +    ret = vmstate_save_state(f, vmsd, tmp, vmdesc);\n> >      g_free(tmp);\n> >  \n> > -    return 0;\n> > +    return ret;\n> >  }\n> >  \n> >  const VMStateInfo vmstate_info_tmp = {\n> > @@ -657,12 +658,16 @@ static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size,\n> >      /* offset of the QTAILQ entry in a QTAILQ element*/\n> >      size_t entry_offset = field->start;\n> >      void *elm;\n> > +    int ret;\n> >  \n> >      trace_put_qtailq(vmsd->name, vmsd->version_id);\n> >  \n> >      QTAILQ_RAW_FOREACH(elm, pv, entry_offset) {\n> >          qemu_put_byte(f, true);\n> > -        vmstate_save_state(f, vmsd, elm, vmdesc);\n> > +        ret = vmstate_save_state(f, vmsd, elm, vmdesc);\n> > +        if (ret) {\n> > +            return ret;\n> > +        }\n> >      }\n> >      qemu_put_byte(f, false);\n> >  \n> > diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c\n> > index e643ac662b..ab3e430c2c 100644\n> > --- a/tests/test-vmstate.c\n> > +++ b/tests/test-vmstate.c\n> > @@ -70,7 +70,7 @@ static void save_vmstate(const VMStateDescription *desc, void *obj)\n> >      QEMUFile *f = open_test_file(true);\n> >  \n> >      /* Save file with vmstate */\n> > -    vmstate_save_state(f, desc, obj, NULL);\n> > +    g_assert(!vmstate_save_state(f, desc, obj, NULL));\n> >      qemu_put_byte(f, QEMU_VM_EOF);\n> >      g_assert(!qemu_file_get_error(f));\n> >      qemu_fclose(f);\n> > @@ -381,7 +381,7 @@ static void test_save_noskip(void)\n> >      QEMUFile *fsave = open_test_file(true);\n> >      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,\n> >                         .skip_c_e = false };\n> > -    vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> > +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));\n> >      g_assert(!qemu_file_get_error(fsave));\n> >  \n> >      uint8_t expected[] = {\n> > @@ -402,7 +402,7 @@ static void test_save_skip(void)\n> >      QEMUFile *fsave = open_test_file(true);\n> >      TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4, .e = 5, .f = 6,\n> >                         .skip_c_e = true };\n> > -    vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> > +    g_assert(!vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL));\n> >      g_assert(!qemu_file_get_error(fsave));\n> >  \n> >      uint8_t expected[] = {\n> > -- \n> > 2.13.5\n> > \n> \n> -- \n> Peter Xu\n--\nDr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx07.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=dgilbert@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy4sZ3Th8z9t2Q\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 21 Sep 2017 02:27:46 +1000 (AEST)","from localhost ([::1]:49571 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duhqi-0004nf-9C\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 12:27:44 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:48362)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <dgilbert@redhat.com>) id 1duhCY-0004er-M7\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 11:46:16 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <dgilbert@redhat.com>) id 1duhCT-0008K5-Ax\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 11:46:14 -0400","from mx1.redhat.com ([209.132.183.28]:51404)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <dgilbert@redhat.com>) id 1duhCS-0008GB-2D\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 11:46:08 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id B8570C071886;\n\tWed, 20 Sep 2017 15:46:05 +0000 (UTC)","from work-vm (ovpn-117-183.ams2.redhat.com [10.36.117.183])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 0F7181755A;\n\tWed, 20 Sep 2017 15:46:01 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com B8570C071886","Date":"Wed, 20 Sep 2017 16:45:59 +0100","From":"\"Dr. David Alan Gilbert\" <dgilbert@redhat.com>","To":"Peter Xu <peterx@redhat.com>","Message-ID":"<20170920154558.GH2449@work-vm>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-6-dgilbert@redhat.com>\n\t<20170920033134.GQ3617@pxdev.xzpeter.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170920033134.GQ3617@pxdev.xzpeter.org>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tWed, 20 Sep 2017 15:46:06 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"peter.maydell@linaro.org, rth@twiddle.net, cohuck@redhat.com,\n\tqemu-devel@nongnu.org, quintela@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]