[{"id":1771961,"web_url":"http://patchwork.ozlabs.org/comment/1771961/","msgid":"<20170920155746.1192d2fa.cohuck@redhat.com>","list_archive_url":null,"date":"2017-09-20T13:57:46","subject":"Re: [Qemu-devel] [PATCH 1/5] migration: pre_save return int","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:34 +0100\n\"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com> wrote:\n\n> From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n> \n> Modify the pre_save method on VMStateDescription to return an int\n> rather than void so that it potentially can fail.\n> \n> Changed zillions of devices to make them return 0; the only\n> case I've made it return non-0 is hw/intc/s390_flic_kvm.c that already\n> had an error_report/return case.\n\nNever thought that this device would be at the bleeding edge ;)\n\n> \n> Note: If you add an error exit in your pre_save you must emit\n> an error_report to say why.\n\nWould it make sense to relay an error object? For example,\ncpu_pre_save() in target/s390x/machine.c calls\nkvm_s390_vcpu_interrupt_pre_save() which already does an error report.\nIf we relay that error instead, we would avoid saying \"oops, this\ndidn't work\" several times with decreasing amount of information.\n\nOn the other hand, that change would be more invasive.\n\n> \n> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>\n> ---\n\n>  hw/intc/s390_flic_kvm.c        |  6 ++++--\n\n>  hw/s390x/css.c                 | 10 +++++++---\n>  hw/s390x/virtio-ccw.c          |  4 +++-\n\n>  target/s390x/machine.c         |  4 +++-\n\nThat said, the changes in s390-related code look fine.","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=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 3xy30q68Cqz9sPt\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 21 Sep 2017 01:03:55 +1000 (AEST)","from localhost ([::1]:48686 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 1dugXZ-0000hL-Qw\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 11:03:53 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:50968)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dufVs-0004QJ-S2\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:58:05 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <cohuck@redhat.com>) id 1dufVn-0006gH-Or\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:58:04 -0400","from mx1.redhat.com ([209.132.183.28]:33852)\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 1dufVn-0006fu-JD\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 09:57:59 -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 83CEE7F7D0;\n\tWed, 20 Sep 2017 13:57:58 +0000 (UTC)","from gondolin (ovpn-117-98.ams2.redhat.com [10.36.117.98])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id E497B60604;\n\tWed, 20 Sep 2017 13:57:49 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 83CEE7F7D0","Date":"Wed, 20 Sep 2017 15:57:46 +0200","From":"Cornelia Huck <cohuck@redhat.com>","To":"\"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com>","Message-ID":"<20170920155746.1192d2fa.cohuck@redhat.com>","In-Reply-To":"<20170919180038.26056-2-dgilbert@redhat.com>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-2-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.13","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 13:57:58 +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 1/5] migration: pre_save return int","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":1772015,"web_url":"http://patchwork.ozlabs.org/comment/1772015/","msgid":"<20170920143226.GF2449@work-vm>","list_archive_url":null,"date":"2017-09-20T14:32:27","subject":"Re: [Qemu-devel] [PATCH 1/5] migration: pre_save return int","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:34 +0100\n> \"Dr. David Alan Gilbert (git)\" <dgilbert@redhat.com> wrote:\n> \n> > From: \"Dr. David Alan Gilbert\" <dgilbert@redhat.com>\n> > \n> > Modify the pre_save method on VMStateDescription to return an int\n> > rather than void so that it potentially can fail.\n> > \n> > Changed zillions of devices to make them return 0; the only\n> > case I've made it return non-0 is hw/intc/s390_flic_kvm.c that already\n> > had an error_report/return case.\n> \n> Never thought that this device would be at the bleeding edge ;)\n\nIt's the one case that had bothered to do a proper error.\n\n> > \n> > Note: If you add an error exit in your pre_save you must emit\n> > an error_report to say why.\n> \n> Would it make sense to relay an error object? For example,\n> cpu_pre_save() in target/s390x/machine.c calls\n> kvm_s390_vcpu_interrupt_pre_save() which already does an error report.\n> If we relay that error instead, we would avoid saying \"oops, this\n> didn't work\" several times with decreasing amount of information.\n> \n> On the other hand, that change would be more invasive.\n\nRight, and it's very very verbose.\n\n> \n> > \n> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>\n> > ---\n> \n> >  hw/intc/s390_flic_kvm.c        |  6 ++++--\n> \n> >  hw/s390x/css.c                 | 10 +++++++---\n> >  hw/s390x/virtio-ccw.c          |  4 +++-\n> \n> >  target/s390x/machine.c         |  4 +++-\n> \n> That said, the changes in s390-related code look fine.\n\nThanks.\n\nDave\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-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=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 3xy49C3kk5z9s7c\n\tfor <incoming@patchwork.ozlabs.org>;\n\tThu, 21 Sep 2017 01:56:15 +1000 (AEST)","from localhost ([::1]:49152 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 1duhMD-0003mN-Gz\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 11:56:13 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:32957)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <dgilbert@redhat.com>) id 1dug3M-0003h5-2D\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:32:41 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <dgilbert@redhat.com>) id 1dug3I-00042b-Ro\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:32:40 -0400","from mx1.redhat.com ([209.132.183.28]:37148)\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 1dug3I-00042R-LI\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:32:36 -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 98CF9883DF;\n\tWed, 20 Sep 2017 14:32:35 +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 95A36614C0;\n\tWed, 20 Sep 2017 14:32:29 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 98CF9883DF","Date":"Wed, 20 Sep 2017 15:32:27 +0100","From":"\"Dr. David Alan Gilbert\" <dgilbert@redhat.com>","To":"Cornelia Huck <cohuck@redhat.com>","Message-ID":"<20170920143226.GF2449@work-vm>","References":"<20170919180038.26056-1-dgilbert@redhat.com>\n\t<20170919180038.26056-2-dgilbert@redhat.com>\n\t<20170920155746.1192d2fa.cohuck@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170920155746.1192d2fa.cohuck@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.26]);\n\tWed, 20 Sep 2017 14:32: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 1/5] migration: pre_save return int","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>"}}]