qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without io


From: Stefano Stabellini
Subject: Re: [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages
Date: Fri, 12 Aug 2016 15:09:01 -0700 (PDT)
User-agent: Alpine 2.10 (DEB 1266 2009-07-14)

On Fri, 12 Aug 2016, Paul Durrant wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:address@hidden
> > Sent: 11 August 2016 21:07
> > To: Paul Durrant
> > Cc: Anthony Perard; address@hidden; qemu-
> > address@hidden; Stefano Stabellini
> > Subject: RE: [PATCH] xen: handle inbound migration of VMs without ioreq
> > server pages
> > 
> > On Tue, 9 Aug 2016, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Anthony PERARD [mailto:address@hidden
> > > > Sent: 09 August 2016 16:19
> > > > To: Paul Durrant
> > > > Cc: address@hidden; address@hidden; 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 <address@hidden>
> > > > > Cc: Stefano Stabellini <address@hidden>
> > > > > Cc: Anthony Perard <address@hidden>
> > > >
> > > > 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
> > <address@hidden>
> > > >
> > > > 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?
> 
> Personally I don't think it's worth it. This is not a performance critical 
> codepath but if you insist I will re-factor the code.

All right, given that Anthony already acked it, it's two vs. one. I'll
commit it as is.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]