From patchwork Thu Jan 26 12:14:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cornelia Huck X-Patchwork-Id: 720134 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3v8LWd1PhJz9t2D for ; Thu, 26 Jan 2017 23:17:01 +1100 (AEDT) Received: from localhost ([::1]:37919 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cWiz4-0005nq-P4 for incoming@patchwork.ozlabs.org; Thu, 26 Jan 2017 07:16:58 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42233) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cWiwQ-0003r2-Nk for qemu-devel@nongnu.org; Thu, 26 Jan 2017 07:14:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cWiwN-0005f5-Fr for qemu-devel@nongnu.org; Thu, 26 Jan 2017 07:14:14 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:43420 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cWiwN-0005ei-Ar for qemu-devel@nongnu.org; Thu, 26 Jan 2017 07:14:11 -0500 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v0QCDrph065623 for ; Thu, 26 Jan 2017 07:14:08 -0500 Received: from e06smtp07.uk.ibm.com (e06smtp07.uk.ibm.com [195.75.94.103]) by mx0b-001b2d01.pphosted.com with ESMTP id 287ccma8ck-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 26 Jan 2017 07:14:08 -0500 Received: from localhost by e06smtp07.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 26 Jan 2017 12:14:06 -0000 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp07.uk.ibm.com (192.168.101.137) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 26 Jan 2017 12:14:04 -0000 Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 7EC4A1B08079; Thu, 26 Jan 2017 12:16:51 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v0QCE38T63766726; Thu, 26 Jan 2017 12:14:03 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 8CEC911C04C; Thu, 26 Jan 2017 11:11:55 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 62AF011C04A; Thu, 26 Jan 2017 11:11:55 +0000 (GMT) Received: from gondolin (unknown [9.152.224.55]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Thu, 26 Jan 2017 11:11:55 +0000 (GMT) Date: Thu, 26 Jan 2017 13:14:02 +0100 From: Cornelia Huck To: "Dr. David Alan Gilbert" In-Reply-To: <20170125144420.GE2096@work-vm> References: <20170124184742.1639-1-dgilbert@redhat.com> <20170124184742.1639-3-dgilbert@redhat.com> <20170125114620.GA9699@lemon.Home> <20170125120053.GA2096@work-vm> <20170125131917.32df62ef.cornelia.huck@de.ibm.com> <20170125132254.GB2096@work-vm> <20170125152014.2ab6f901.cornelia.huck@de.ibm.com> <20170125144420.GE2096@work-vm> Organization: IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz =?UTF-8?B?R2VzY2jDpGZ0c2bDvGhydW5nOg==?= Dirk Wittkopp Sitz der Gesellschaft: =?UTF-8?B?QsO2Ymxpbmdlbg==?= Registergericht: Amtsgericht Stuttgart, HRB 243294 X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17012612-0028-0000-0000-0000029E7656 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17012612-0029-0000-0000-0000222A3A7E Message-Id: <20170126131402.547f1004.cornelia.huck@de.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-01-26_07:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1701260124 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [generic] [fuzzy] X-Received-From: 148.163.158.5 Subject: Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jianjun Duan , Fam Zheng , qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" On Wed, 25 Jan 2017 14:44:20 +0000 "Dr. David Alan Gilbert" wrote: > * Cornelia Huck (cornelia.huck@de.ibm.com) wrote: > > On Wed, 25 Jan 2017 13:22:55 +0000 > > "Dr. David Alan Gilbert" wrote: > > > > > * Cornelia Huck (cornelia.huck@de.ibm.com) wrote: > > > > On Wed, 25 Jan 2017 12:00:53 +0000 > > > > "Dr. David Alan Gilbert" wrote: > > > > > OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make > > > > > sense there. > > > > > > > > Just saw this. I don't think we want -ENOMEM, as that would change the > > > > actual state being saved, no? > > > > > > But isn't that the intention of this function? > > > > > > buf = g_try_malloc0(len); > > > if (!buf) { > > > /* Storing FLIC_FAILED into the count field here will cause the > > > * target system to fail when attempting to load irqs from the > > > * migration state */ > > > error_report("flic: couldn't allocate memory"); > > > qemu_put_be64(f, FLIC_FAILED); > > > return; > > > } > > > > > > What should happen on the destination - should the migration fail? > > > If we want the migration to fail then we should now return an error > > > status rather than 0, and then we see a failed migration on the source > > > as well. > > > > Yes. There's also another error case below where we should return an > > error instead of putting FLIC_FAILED, then. > > > > The problem is that this is rather hard to test: So I'd prefer to fix > > the compile for now and introduce error return codes in a separate > > patch. > > OK, that's fair. I've coded something up and tried to test it with error injection to trigger the failed case, but I can't really see an improvement :( Before: source logs error, target fails to load the flic with 'invalid argument' After: source logs error, target fails to load the flic with 'could not allocate memory' The migration code does not seem to do anything with the return code of put methods for now, so that's not too surprising. Is anything in the works? For now, I'd prefer to keep the old behaviour as 'invalid argument' seems like a more obvious error on the target. diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c index e86a84e49a..3c62ef8258 100644 --- a/hw/intc/s390_flic_kvm.c +++ b/hw/intc/s390_flic_kvm.c @@ -293,27 +293,21 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size, int len = FLIC_SAVE_INITIAL_SIZE; void *buf; int count; + int r = 0; flic_disable_wait_pfault((struct KVMS390FLICState *) opaque); buf = g_try_malloc0(len); if (!buf) { - /* Storing FLIC_FAILED into the count field here will cause the - * target system to fail when attempting to load irqs from the - * migration state */ error_report("flic: couldn't allocate memory"); - qemu_put_be64(f, FLIC_FAILED); - return 0; + return -ENOMEM; } count = __get_all_irqs(flic, &buf, len); if (count < 0) { error_report("flic: couldn't retrieve irqs from kernel, rc %d", count); - /* Storing FLIC_FAILED into the count field here will cause the - * target system to fail when attempting to load irqs from the - * migration state */ - qemu_put_be64(f, FLIC_FAILED); + r = count; } else { qemu_put_be64(f, count); qemu_put_buffer(f, (uint8_t *) buf, @@ -321,7 +315,7 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size, } g_free(buf); - return 0; + return r; } /**