From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=52258 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OdglQ-0007V6-Ki for qemu-devel@nongnu.org; Tue, 27 Jul 2010 05:47:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OdglN-00063p-Hh for qemu-devel@nongnu.org; Tue, 27 Jul 2010 05:47:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37058) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OdglN-00063Y-AD for qemu-devel@nongnu.org; Tue, 27 Jul 2010 05:47:53 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o6R9lpZE028161 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 27 Jul 2010 05:47:52 -0400 Date: Tue, 27 Jul 2010 10:47:49 +0100 From: "Daniel P. Berrange" Message-ID: <20100727094749.GB12387@redhat.com> References: <20100723150818.69cde489@redhat.com> <20100724073124.GB5254@amit-laptop.redhat.com> <20100726112353.4771b3cd@redhat.com> <4C4DA97F.2060003@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: [Qemu-devel] Re: [PATCH] migration: Issue 'cont' only on successful incoming migration Reply-To: "Daniel P. Berrange" List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: Amit Shah , Paolo Bonzini , qemu list , Laine Stump , Luiz Capitulino On Mon, Jul 26, 2010 at 09:49:12PM +0200, Juan Quintela wrote: > Laine Stump wrote: > > On 07/26/2010 10:23 AM, Luiz Capitulino wrote: > >> On Sat, 24 Jul 2010 13:01:24 +0530 > >> Amit Shah wrote: > >> > >>> On (Fri) Jul 23 2010 [15:08:18], Luiz Capitulino wrote: > >>>>> diff --git a/monitor.c b/monitor.c > >>>>> index 45fd482..d12a7b5 100644 > >>>>> --- a/monitor.c > >>>>> +++ b/monitor.c > >>>>> @@ -1056,6 +1056,10 @@ static int do_cont(Monitor *mon, const QDict *qdict, QObject **ret_data) > >>>>> { > >>>>> struct bdrv_iterate_context context = { mon, 0 }; > >>>>> > >>>>> + if (incoming_expected&& !incoming_done) { > >>>>> + autostart = 1; > >>>> Why do we need to set autostart? We should just fail if we're unable to run. > >>>> > >>>>> + return 1; /* Waiting for incoming migration */ > >>>> You should return -1 and use qerror_report(), so that we have a meaningful > >>>> error in the user Monitor and QMP (otherwise we'll get UndefinedError). > >>> That would mean old/existing libvirt will be confused on why guests > >>> wouldn't start even though it issued cont. > >> Yes, although delaying to start could cause a problem too and this is > >> also introducing an new error in QMP already. > >> > >> I really would like to avoid adding weird semantics, specially in QMP where > >> cont will return an error but will put the VM to run later. We could fix this > >> there only, but then it will get complex w/o reason. > >> > >> We should fix it properly right now, IMO. > >> > >>> If it's not a problem for the libvirt folks, I can do that. > >> Laine, could you please check that? > > > > That should really be answered by someone who better understands the > > implications (I'm a newcomer to that part of the code). Dan Berrange > > or Chris Lalancette maybe? > > > > (I am setting up to test the current version of the patch on the > > system where I can reproduce the problem. Haven't flipped the switch > > yet, though.) > > Just to be sure, what do you want "cont" to return if we are in the > middle of a migration? NB, this is only making a difference if 'cont' is run between the time QEMU starts with -incoming and the time migration starts. Once migration starts, the entire QEMU monitor is blocked until it completes, so you'll never see 'cont' in the middle of migration. libvirt currently relies on that behaviour but as per previous discussions on the subject, we'd prefer to have some kind of async notification whether via async events, or via an async QMP command. > This patch just sets autostart=1 (i.e. user wants to start guest as soon > as possible), is that ok, or a warning/error is better? I agree with Luiz that having such 'magical' behaviour for commands is not very desirable. If 'cont' isn't valid based on the current QEMU execution state, then it should return an error. The downside is that while migration will still complete successfully, the current libvirt will unfortunately report an error in this scenario. If there is an explicit QMP error code associated with this condition though, we can catch the error and handle it appropriately in future. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|