From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v2] tools/libxc: Prevent erroneous success from xc_domain_restore Date: Wed, 5 Feb 2014 14:55:35 +0000 Message-ID: <52F250E7.1090008@eu.citrix.com> References: <1391535813.6497.61.camel@kazak.uk.xensource.com> <1391536870-22809-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1391536870-22809-1-git-send-email-andrew.cooper3@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , Xen-devel Cc: Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 02/04/2014 06:01 PM, Andrew Cooper wrote: > The variable 'rc' is set to 1 at the top of xc_domain_restore, and for the > most part is left alone until success, at which point it is set to 0. > > There is a separate 'frc' which for the most part is used to check function > calls, keeping errors separate from 'rc'. > > For a toolstack which sets callbacks->toolstack_restore(), and the function > returns 0, any subsequent error will end up with code flow going to "out;", > resulting in the migration being declared a success. > > For consistency, update the callsites of xc_dom_gnttab{,_hvm}_seed() to use > 'frc', even though their use of 'rc' is currently safe. > > Signed-off-by: Andrew Cooper > CC: Ian Campbell > CC: Ian Jackson > CC: George Dunlap > > --- > > Changes in v2: > * Dont drop rc = -1 from toolstack_restore(). > > Regarding 4.4: If the two "for consistency" changes to > xc_dom_gnttab{,_hvm}_seed() are considered too risky, they can be dropped > without affecting the bugfix nature of the patch, but I would argue that > leaving some examples of "rc = function_call()" leaves a bad precident which > is likely to lead to similar bugs in the future. Yes, these are all pretty clear bug fixes. Release-acked-by: George Dunlap