From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [PATCH 0/6] Save state error handling (kill off no_migrate) Date: Mon, 08 Nov 2010 10:20:46 -0700 Message-ID: <1289236846.28165.24.camel@x201> References: <20101006204546.32127.70109.stgit@s20.home> <20101108114043.GB1075@redhat.com> <1289228397.19902.18.camel@x201> <20101108165406.GE7962@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org, cam@cs.ualberta.ca, quintela@redhat.com, anthony@codemonkey.ws To: "Michael S. Tsirkin" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:50602 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755150Ab0KHRUw (ORCPT ); Mon, 8 Nov 2010 12:20:52 -0500 In-Reply-To: <20101108165406.GE7962@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > On Wed, Oct 06, 2010 at 02:58:57PM -0600, Alex Williamson wrote: > > > > Our code paths for saving or migrating a VM are full of functions that > > > > return void, leaving no opportunity for a device to cancel a migration, > > > > either from error or incompatibility. The ivshmem driver attempted to > > > > solve this with a no_migrate flag on the save state entry. I think the > > > > more generic and flexible way to solve this is to allow driver save > > > > functions to fail. This series implements that and converts ivshmem > > > > to uses a set_params function to NAK migration much earlier in the > > > > processes. This touches a lot of files, but bulk of those changes are > > > > simply s/void/int/ and tacking a "return 0" to the end of functions. > > > > Thanks, > > > > > > > > Alex > > > > > > Well error handling is always tricky: it seems easier to > > > require save handlers to never fail. > > > > Sure it's easier, but does that make it robust? > > More robust in the face of wwhat kind of failure? I really don't understand why we're having a discussion about whether providing a means to return an error is a good thing or not. These patches touch a lot of files, but the change is dead simple. > > > So there's a bunch of code here but what exactly is the benefit? > > > Since save handlers have no idea what does the remote do, > > > what is the compatibility you mention? > > > > There are two users I currently have in mind. ivshmem currently makes > > use of the register_device_unmigratable() because it makes use of host > > specific resources and connections (aiui). This sets the no_migrate > > flag, which is not dynamic and a bit of a band-aide. > > The other is > > device assignment, which needs a way to NAK a migration since physical > > devices are never migratable. > > Well since all these can't be migrated ever, a fixed property actually seems > a good match. Sure it's not dynamic but all the easier to debug. > > > I imagine we could at some point have > > devices with state tied to other features that can't always be detached > > from the host, this tries to provide the infrastructure for that to > > happen. > > > > Alex > > Let guest control whether you can migrate? > Sounds like something that is more likely to be abused > than used constructively. s/guest/device/ So you would rather the migration failed on the incoming side where it may not be detected or it may be detected too late to stop the migration? Alex