From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32957) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dug3M-0003h5-2D for qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:32:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dug3I-00042b-Ro for qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:32:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37148) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dug3I-00042R-LI for qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:32:36 -0400 Date: Wed, 20 Sep 2017 15:32:27 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20170920143226.GF2449@work-vm> References: <20170919180038.26056-1-dgilbert@redhat.com> <20170919180038.26056-2-dgilbert@redhat.com> <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> Subject: Re: [Qemu-devel] [PATCH 1/5] migration: pre_save return int List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: qemu-devel@nongnu.org, quintela@redhat.com, peterx@redhat.com, peter.maydell@linaro.org, rth@twiddle.net * Cornelia Huck (cohuck@redhat.com) wrote: > On Tue, 19 Sep 2017 19:00:34 +0100 > "Dr. David Alan Gilbert (git)" wrote: > > > From: "Dr. David Alan Gilbert" > > > > Modify the pre_save method on VMStateDescription to return an int > > rather than void so that it potentially can fail. > > > > Changed zillions of devices to make them return 0; the only > > case I've made it return non-0 is hw/intc/s390_flic_kvm.c that already > > had an error_report/return case. > > Never thought that this device would be at the bleeding edge ;) It's the one case that had bothered to do a proper error. > > > > Note: If you add an error exit in your pre_save you must emit > > an error_report to say why. > > Would it make sense to relay an error object? For example, > cpu_pre_save() in target/s390x/machine.c calls > kvm_s390_vcpu_interrupt_pre_save() which already does an error report. > If we relay that error instead, we would avoid saying "oops, this > didn't work" several times with decreasing amount of information. > > On the other hand, that change would be more invasive. Right, and it's very very verbose. > > > > > Signed-off-by: Dr. David Alan Gilbert > > --- > > > hw/intc/s390_flic_kvm.c | 6 ++++-- > > > hw/s390x/css.c | 10 +++++++--- > > hw/s390x/virtio-ccw.c | 4 +++- > > > target/s390x/machine.c | 4 +++- > > That said, the changes in s390-related code look fine. Thanks. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK