From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53151) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKpW4-0003A1-8l for qemu-devel@nongnu.org; Thu, 30 Jul 2015 11:13:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZKpVz-0003ug-AV for qemu-devel@nongnu.org; Thu, 30 Jul 2015 11:13:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54860) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKpVz-0003uc-4c for qemu-devel@nongnu.org; Thu, 30 Jul 2015 11:12:59 -0400 Message-ID: <55BA3EF5.3030705@redhat.com> Date: Thu, 30 Jul 2015 17:12:53 +0200 From: Thomas Huth MIME-Version: 1.0 References: <1437400198-25382-1-git-send-email-cornelia.huck@de.ibm.com> <1437400198-25382-8-git-send-email-cornelia.huck@de.ibm.com> <55ADFE0B.1090407@redhat.com> <20150721123718.6ab0b668@thinkpad-w530> <55BA3C11.2060202@linux.vnet.ibm.com> In-Reply-To: <55BA3C11.2060202@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.5 7/8] s390x: Migrate guest storage keys (initial memory only) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: jjherne@linux.vnet.ibm.com, David Hildenbrand Cc: Cornelia Huck , borntraeger@de.ibm.com, jfrei@linux.vnet.ibm.com, qemu-devel@nongnu.org, agraf@suse.de On 30/07/15 17:00, Jason J. Herne wrote: > On 07/21/2015 06:37 AM, David Hildenbrand wrote: >>> >>> So if I've got this code right, you send here a "header" that announces >>> a packet with all pages ... >>> >>>> + while (handled_count < total_count) { >>>> + cur_count = MIN(total_count - handled_count, >>>> S390_SKEYS_BUFFER_SIZE); >>>> + >>>> + ret = skeyclass->get_skeys(ss, cur_gfn, cur_count, buf); >>>> + if (ret < 0) { >>>> + error_report("S390_GET_KEYS error %d\n", ret); >>>> + break; >>> >>> ... but when an error occurs here, you suddenly stop in the middle of >>> that "packet" with all pages ... >> >> Indeed, although that should never fail, we never know. >> We don't want to overengineer the protocol but still abort migration >> at least >> on the loading side in that (theoretical) case. >> > > I don't have a strong opinion on this either way. I think it is fine > just the way > it is (for the reasons David described above). However, if people are > worried I > can see about writing some code that sends fake keys to the destination as > described below. Thoughts? If David is right and the skeyclass->get_skeys() really never fails (I did not check), then simply do an "assert (ret == 0)" afterwards - that way you can be sure that it really never fails. And if it ever fails, you notice it immediately - and that's certainly way much better than debugging the currently-wrong error handling code. Thomas