[{"id":1775105,"web_url":"http://patchwork.ozlabs.org/comment/1775105/","msgid":"<20170926030647.GK19505@pxdev.xzpeter.org>","list_archive_url":null,"date":"2017-09-26T03:06:47","subject":"Re: [Qemu-devel] [PATCH v2 6/6] 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 Mon, Sep 25, 2017 at 12:29:17PM +0100, Dr. David Alan Gilbert (git) wrote:\n> From: \"Dr. David Alan Gilbert\" \n> \n> vmstate_save is called in a few places, and vmstate_save_state is\n> called in lots of places.\n\nvmstate_save() should have been processed with previous patch?\n\n> \n> Route error returns from the easier cases back up; there are lots\n> of more complex cases where their own error paths need fixing.\n> \n> Signed-off-by: Dr. David Alan Gilbert \n\nThe patch content wires quite a few vmstate_save_state() callers, so\nthe patch itself makes sense to me.\n\nReviewed-by: Peter Xu \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 | 9 ++++++---\n> 5 files changed, 23 insertions(+), 16 deletions(-)\n> \n> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c\n> index 3a8f1e1a2d..2becbfda59 100644\n> --- a/hw/display/virtio-gpu.c\n> +++ b/hw/display/virtio-gpu.c\n> @@ -1050,9 +1050,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 3129d25c00..311929e9df 100644\n> --- a/hw/virtio/virtio.c\n> +++ b/hw/virtio/virtio.c\n> @@ -1897,7 +1897,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> @@ -1947,20 +1947,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..087844b6c8 100644\n> --- a/tests/test-vmstate.c\n> +++ b/tests/test-vmstate.c\n> @@ -70,7 +70,8 @@ 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> + int ret = vmstate_save_state(f, desc, obj, NULL);\n> + g_assert(!ret);\n> qemu_put_byte(f, QEMU_VM_EOF);\n> g_assert(!qemu_file_get_error(f));\n> qemu_fclose(f);\n> @@ -381,7 +382,8 @@ 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> + int ret = vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> + g_assert(!ret);\n> g_assert(!qemu_file_get_error(fsave));\n> \n> uint8_t expected[] = {\n> @@ -402,7 +404,8 @@ 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> + int ret = vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> + g_assert(!ret);\n> g_assert(!qemu_file_get_error(fsave));\n> \n> uint8_t expected[] = {\n> -- \n> 2.13.5\n>","headers":{"Return-Path":"","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=)","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=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 3y1QqN5MBWz9t3F\n\tfor ;\n\tTue, 26 Sep 2017 13:07:25 +1000 (AEST)","from localhost ([::1]:45348 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dwgDR-00069z-KE\n\tfor incoming@patchwork.ozlabs.org; Mon, 25 Sep 2017 23:07:21 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:57025)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dwgD4-00069K-Sg\n\tfor qemu-devel@nongnu.org; Mon, 25 Sep 2017 23:07:00 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dwgD1-0002ba-Lo\n\tfor qemu-devel@nongnu.org; Mon, 25 Sep 2017 23:06:58 -0400","from mx1.redhat.com ([209.132.183.28]:34532)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dwgD1-0002aJ-BR\n\tfor qemu-devel@nongnu.org; Mon, 25 Sep 2017 23:06:55 -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 53767C04B946;\n\tTue, 26 Sep 2017 03:06:54 +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 D51F777D66;\n\tTue, 26 Sep 2017 03:06:49 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 53767C04B946","Date":"Tue, 26 Sep 2017 11:06:47 +0800","From":"Peter Xu ","To":"\"Dr. David Alan Gilbert (git)\" ","Message-ID":"<20170926030647.GK19505@pxdev.xzpeter.org>","References":"<20170925112917.21340-1-dgilbert@redhat.com>\n\t<20170925112917.21340-7-dgilbert@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20170925112917.21340-7-dgilbert@redhat.com>","User-Agent":"Mutt/1.5.24 (2015-08-30)","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.31]);\n\tTue, 26 Sep 2017 03:06:54 +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 v2 6/6] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"peter.maydell@linaro.org, famz@redhat.com, quintela@redhat.com,\n\tcohuck@redhat.com, qemu-devel@nongnu.org, rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1775328,"web_url":"http://patchwork.ozlabs.org/comment/1775328/","msgid":"<20170926100315.GC2689@work-vm>","list_archive_url":null,"date":"2017-09-26T10:03:16","subject":"Re: [Qemu-devel] [PATCH v2 6/6] 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 Mon, Sep 25, 2017 at 12:29:17PM +0100, Dr. David Alan Gilbert (git) wrote:\n> > From: \"Dr. David Alan Gilbert\" \n> > \n> > vmstate_save is called in a few places, and vmstate_save_state is\n> > called in lots of places.\n> \n> vmstate_save() should have been processed with previous patch?\n\nAh yes, it was just vmstate_save_state in the end fixed in this one.\n\n> > \n> > Route error returns from the easier cases back up; there are lots\n> > of more complex cases where their own error paths need fixing.\n> > \n> > Signed-off-by: Dr. David Alan Gilbert \n> \n> The patch content wires quite a few vmstate_save_state() callers, so\n> the patch itself makes sense to me.\n> \n> Reviewed-by: Peter Xu \n\nThanks!\n\nDave\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 | 9 ++++++---\n> > 5 files changed, 23 insertions(+), 16 deletions(-)\n> > \n> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c\n> > index 3a8f1e1a2d..2becbfda59 100644\n> > --- a/hw/display/virtio-gpu.c\n> > +++ b/hw/display/virtio-gpu.c\n> > @@ -1050,9 +1050,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 3129d25c00..311929e9df 100644\n> > --- a/hw/virtio/virtio.c\n> > +++ b/hw/virtio/virtio.c\n> > @@ -1897,7 +1897,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> > @@ -1947,20 +1947,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..087844b6c8 100644\n> > --- a/tests/test-vmstate.c\n> > +++ b/tests/test-vmstate.c\n> > @@ -70,7 +70,8 @@ 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> > + int ret = vmstate_save_state(f, desc, obj, NULL);\n> > + g_assert(!ret);\n> > qemu_put_byte(f, QEMU_VM_EOF);\n> > g_assert(!qemu_file_get_error(f));\n> > qemu_fclose(f);\n> > @@ -381,7 +382,8 @@ 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> > + int ret = vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> > + g_assert(!ret);\n> > g_assert(!qemu_file_get_error(fsave));\n> > \n> > uint8_t expected[] = {\n> > @@ -402,7 +404,8 @@ 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> > + int ret = vmstate_save_state(fsave, &vmstate_skipping, &obj, NULL);\n> > + g_assert(!ret);\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":"","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=)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.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 3y1c4B0Mm0z9tXc\n\tfor ;\n\tTue, 26 Sep 2017 20:04:10 +1000 (AEST)","from localhost ([::1]:46438 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dwmim-0006iR-6t\n\tfor incoming@patchwork.ozlabs.org; Tue, 26 Sep 2017 06:04:08 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:39995)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dwmiD-0006hG-2V\n\tfor qemu-devel@nongnu.org; Tue, 26 Sep 2017 06:03:34 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dwmi7-0002CK-14\n\tfor qemu-devel@nongnu.org; Tue, 26 Sep 2017 06:03:33 -0400","from mx1.redhat.com ([209.132.183.28]:37492)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dwmi6-0002Ba-PE\n\tfor qemu-devel@nongnu.org; Tue, 26 Sep 2017 06:03:26 -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 D0E0B356F2;\n\tTue, 26 Sep 2017 10:03:25 +0000 (UTC)","from work-vm (ovpn-117-148.ams2.redhat.com [10.36.117.148])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 4553069AC3;\n\tTue, 26 Sep 2017 10:03:18 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com D0E0B356F2","Date":"Tue, 26 Sep 2017 11:03:16 +0100","From":"\"Dr. David Alan Gilbert\" ","To":"Peter Xu ","Message-ID":"<20170926100315.GC2689@work-vm>","References":"<20170925112917.21340-1-dgilbert@redhat.com>\n\t<20170925112917.21340-7-dgilbert@redhat.com>\n\t<20170926030647.GK19505@pxdev.xzpeter.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170926030647.GK19505@pxdev.xzpeter.org>","User-Agent":"Mutt/1.8.3 (2017-05-23)","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.30]);\n\tTue, 26 Sep 2017 10:03:25 +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 v2 6/6] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"peter.maydell@linaro.org, famz@redhat.com, quintela@redhat.com,\n\tcohuck@redhat.com, qemu-devel@nongnu.org, rth@twiddle.net","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1775516,"web_url":"http://patchwork.ozlabs.org/comment/1775516/","msgid":"<20170926153930.65f10c2f.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-26T13:39:30","subject":"Re: [Qemu-devel] [PATCH v2 6/6] 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 Mon, 25 Sep 2017 12:29:17 +0100\n\"Dr. David Alan Gilbert (git)\" wrote:\n\n> From: \"Dr. David Alan Gilbert\" \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 their own error paths need fixing.\n> \n> Signed-off-by: Dr. David Alan Gilbert \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 | 9 ++++++---\n> 5 files changed, 23 insertions(+), 16 deletions(-)\n\nIf you fix up the description to only refer to vmstate_save_state:\n\nReviewed-by: Cornelia Huck ","headers":{"Return-Path":"","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=)","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 3y1hvP6MLFz9s7M\n\tfor ;\n\tTue, 26 Sep 2017 23:41:53 +1000 (AEST)","from localhost ([::1]:47614 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dwq7T-0000oH-VS\n\tfor incoming@patchwork.ozlabs.org; Tue, 26 Sep 2017 09:41:52 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:49912)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dwq5P-0007yV-8Y\n\tfor qemu-devel@nongnu.org; Tue, 26 Sep 2017 09:39:44 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dwq5M-0001mY-4l\n\tfor qemu-devel@nongnu.org; Tue, 26 Sep 2017 09:39:43 -0400","from mx1.redhat.com ([209.132.183.28]:36878)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dwq5L-0001lr-VX\n\tfor qemu-devel@nongnu.org; Tue, 26 Sep 2017 09:39:40 -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 8B96DC047B6D;\n\tTue, 26 Sep 2017 13:39:35 +0000 (UTC)","from gondolin (dhcp-192-215.str.redhat.com [10.33.192.215])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id C9DDB600CA;\n\tTue, 26 Sep 2017 13:39:32 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 8B96DC047B6D","Date":"Tue, 26 Sep 2017 15:39:30 +0200","From":"Cornelia Huck ","To":"\"Dr. David Alan Gilbert (git)\" ","Message-ID":"<20170926153930.65f10c2f.cohuck@redhat.com>","In-Reply-To":"<20170925112917.21340-7-dgilbert@redhat.com>","References":"<20170925112917.21340-1-dgilbert@redhat.com>\n\t<20170925112917.21340-7-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.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.31]);\n\tTue, 26 Sep 2017 13:39:35 +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 v2 6/6] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"peter.maydell@linaro.org, famz@redhat.com, quintela@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"}},{"id":1776159,"web_url":"http://patchwork.ozlabs.org/comment/1776159/","msgid":"<87fub8bjco.fsf@secure.laptop>","list_archive_url":null,"date":"2017-09-27T09:07:19","subject":"Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths","submitter":{"id":2643,"url":"http://patchwork.ozlabs.org/api/people/2643/","name":"Juan Quintela","email":"quintela@redhat.com"},"content":"\"Dr. David Alan Gilbert (git)\" wrote:\n> From: \"Dr. David Alan Gilbert\" \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 their own error paths need fixing.\n>\n> Signed-off-by: Dr. David Alan Gilbert \n\nReviewed-by: Juan Quintela \n\nBut .....\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> @@ -1947,20 +1947,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\nWe add error code, good.\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\nAnd we add the error code. But are the callers ready to use it?\n\n...\n } else {\n field->info->put(f, curr_elem, size, field, vmdesc_loop);\n }\n\nNo, we need to fix it on the callers also. Yeap, that is can be fixed\nin a posterios series.\n\nLater, Juan.","headers":{"Return-Path":"","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=)","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=quintela@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 3y2Bmq417Vz9tXT\n\tfor ;\n\tWed, 27 Sep 2017 19:07:55 +1000 (AEST)","from localhost ([::1]:53561 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dx8Jt-0000uV-K6\n\tfor incoming@patchwork.ozlabs.org; Wed, 27 Sep 2017 05:07:53 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:50817)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dx8JN-0000tn-BY\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 05:07:23 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dx8JJ-0000rB-Ch\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 05:07:21 -0400","from mx1.redhat.com ([209.132.183.28]:44332)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dx8JJ-0000qq-6H\n\tfor qemu-devel@nongnu.org; Wed, 27 Sep 2017 05:07:17 -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 4E81FC047B78;\n\tWed, 27 Sep 2017 09:07:16 +0000 (UTC)","from localhost (ovpn-117-142.ams2.redhat.com [10.36.117.142])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 87CA177E88;\n\tWed, 27 Sep 2017 09:07:13 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 4E81FC047B78","From":"Juan Quintela ","To":"\"Dr. David Alan Gilbert \\(git\\)\" ","In-Reply-To":"<20170925112917.21340-7-dgilbert@redhat.com> (David Alan\n\tGilbert's message of \"Mon, 25 Sep 2017 12:29:17 +0100\")","References":"<20170925112917.21340-1-dgilbert@redhat.com>\n\t<20170925112917.21340-7-dgilbert@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)","Date":"Wed, 27 Sep 2017 11:07:19 +0200","Message-ID":"<87fub8bjco.fsf@secure.laptop>","MIME-Version":"1.0","Content-Type":"text/plain","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, 27 Sep 2017 09:07:16 +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 v2 6/6] migration: Route more error paths","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"quintela@redhat.com","Cc":"peter.maydell@linaro.org, famz@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"}}]