From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXwG2-0005Ay-Gb for qemu-devel@nongnu.org; Thu, 11 Aug 2016 16:07:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bXwFy-0003bx-9i for qemu-devel@nongnu.org; Thu, 11 Aug 2016 16:07:14 -0400 Received: from mail.kernel.org ([198.145.29.136]:41842) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bXwFx-0003as-W9 for qemu-devel@nongnu.org; Thu, 11 Aug 2016 16:07:10 -0400 Date: Thu, 11 Aug 2016 13:07:05 -0700 (PDT) From: Stefano Stabellini In-Reply-To: <339fd6631cba40518264719ba8cf2018@AMSPEX02CL03.citrite.net> Message-ID: References: <1470042985-680-1-git-send-email-paul.durrant@citrix.com> <20160809151912.GC1835@perard.uk.xensource.com> <339fd6631cba40518264719ba8cf2018@AMSPEX02CL03.citrite.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Subject: Re: [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paul Durrant Cc: Anthony Perard , "xen-devel@lists.xenproject.org" , "qemu-devel@nongnu.org" , Stefano Stabellini On Tue, 9 Aug 2016, Paul Durrant wrote: > > -----Original Message----- > > From: Anthony PERARD [mailto:anthony.perard@citrix.com] > > Sent: 09 August 2016 16:19 > > To: Paul Durrant > > Cc: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org; Stefano > > Stabellini > > Subject: Re: [PATCH] xen: handle inbound migration of VMs without ioreq > > server pages > > > > On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote: > > > VMs created on older versions on Xen will not have been provisioned with > > > pages to support creation of non-default ioreq servers. In this case > > > the ioreq server API is not supported and QEMU's only option is to fall > > > back to using the default ioreq server pages as it did prior to > > > commit 3996e85c ("Xen: Use the ioreq-server API when available"). > > > > > > This patch therefore changes the code in xen_common.h to stop > > considering > > > a failure of xc_hvm_create_ioreq_server() as a hard failure but simply > > > as an indication that the guest is too old to support the ioreq server > > > API. Instead a boolean is set to cause reversion to old behaviour such > > > that the default ioreq server is then used. > > > > > > Signed-off-by: Paul Durrant > > > Cc: Stefano Stabellini > > > Cc: Anthony Perard > > > > There are just two lines too long: > > > > WARNING: line over 80 characters > > #31: FILE: include/hw/xen/xen_common.h:110: > > +static inline int xen_get_default_ioreq_server_info(xc_interface *xc, > > domid_t dom, > > > > WARNING: line over 80 characters > > #34: FILE: include/hw/xen/xen_common.h:113: > > + evtchn_port_t *bufioreq_evtchn) > > > > With that fixed: Acked-by: Anthony PERARD > > > > Thanks, > > > > Ok, I'll post v2 with those fixes and your ack. Thanks, Thank you for fixing this bug and Thanks Anthony for the review. I was about to commit it but my sense of style rebelled: I really don't like all the if statements. Too many! Sorry for coming in so late in commenting on a patch, I realize that it is unfair. Would you be up for rewriting the fix so that instead of introducing if (use_default_ioreq_server) { return; } in many functions, we turn xc_hvm_* calls into function pointers calls that get set to the right behavior depending on the success of xc_hvm_create_ioreq_server? The call sites would be something like: ioreq_server->unmap_io_range_from_ioreq_server(xc, dom, ioservid, 0, start_addr, end_addr); At boot time, if xc_hvm_create_ioreq_server returns error: ioreq_server = empty_stubs_functions; else ioreq_server = useful_functions; What do you guys think?