From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH] Fix save/restore for HVM domains with viridian=1 Date: Fri, 25 Nov 2011 15:25:06 +0000 Message-ID: <20111125152506.GC6475@ocelot.phlegethon.org> References: <291EDFCB1E9E224A99088639C4762022B5988E4CE3@LONPMAILBOX01.citrite.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <291EDFCB1E9E224A99088639C4762022B5988E4CE3@LONPMAILBOX01.citrite.net> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Paul Durrant Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org At 15:14 +0000 on 25 Nov (1322234058), Paul Durrant wrote: > > > @@ -411,6 +417,8 @@ static int viridian_load_domain_ctxt(str > > > if ( hvm_load_entry(VIRIDIAN_DOMAIN, h, &ctxt) != 0 ) > > > return -EINVAL; > > > > > > + ASSERT(is_viridian_domain(d)); > > > + > > > > I don't think it's appropriate to crash Xen if the save file is > > bogus. > > > > There's a similar ASSERT in the hypercall function anyway; would you rather I turned that into a rate limited warning too? > If you mean the one in viridian_hypercall(), no - if that function is called for a non-viridian VM that's a bug in Xen so the ASSERT() is correct. The viridian_load_*_ctxt() functions are called based on the HVM save record the tools gave us, so we should at most return an error if they lead us astray. And I think it should be OK to load the HVM state and then set the HVM params, so the current (lack of) response is correct. Cheers, Tim.