[libvirt] [PATCH] Add persistence of PCI addresses to QEMU

[Daniel Veillard]

On Fri, Feb 12, 2010 at 11:09:41AM +0000, Daniel P. Berrange wrote:
> On Fri, Feb 12, 2010 at 12:05:39PM +0100, Daniel Veillard wrote:
> > On Thu, Feb 11, 2010 at 04:40:43PM +0000, Daniel P. Berrange wrote:
> > > Current PCI addresses are allocated at time of VM startup.
> > > To make them truely persistent, it is neccessary to do this
> > > at time of virDomainDefine/virDomainCreate. The code in
> > > qemuStartVMDaemon still remains in order to cope with upgrades
> > > from older libvirt releases
> > > 
> > > * src/qemu/qemu_driver.c: Rename existing qemuAssignPCIAddresses
> > >   to qemuDetectPCIAddresses. Add new qemuAssignPCIAddresses which
> > >   does auto-allocation upfront. Call qemuAssignPCIAddresses from
> > >   qemuDomainDefine and qemuDomainCreate to assign PCI addresses that
> > >   can then be persisted. Don't clear PCI addresses at shutdown if
> > >   they are intended to be persistent
> > > ---
> > >  src/qemu/qemu_driver.c |   84 +++++++++++++++++++++++++++++++++++++++++++++---
> > >  1 files changed, 79 insertions(+), 5 deletions(-)
> 
> 
> > > @@ -857,6 +858,7 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq
> > >      virDomainObjPtr obj = payload;
> > >      struct qemud_driver *driver = opaque;
> > >      qemuDomainObjPrivatePtr priv;
> > > +    unsigned long long qemuCmdFlags;
> > >  
> > >      virDomainObjLock(obj);
> > >  
> > > @@ -872,6 +874,15 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq
> > >          goto error;
> > >      }
> > >  
> > > +    /* XXX we should be persisting the original flags in the XML
> > > +     * not re-detecting them, since the binary may have changed
> > > +     * since launch time */
> > 
> >   But where would we store them ? It sounds a bit strange, it's emulator
> > properties, not really domain ones, and I think we sound only store in
> > the domain what specific flags might be needed from the emulator, not
> > the full set (and this could change over time as a domain is being
> > modified).
> 
> We would put them in the secret status file
> 
>    /var/lib/libvirt/qemu/$GUEST.xml
> 
> where we already store the $PID and a few other bits of internal state
> we need to preserve across daemon restarts.
> 
> The key issue is that you might boot the guest with /usr/bin/qemu
> pointing to QEMU 0.11, and when we later hotplug a device, or restart
> libvirtd, that path might be pointing to 0.12. So we'd detect 0.12
> features which would then fail because the guest is still running
> with 0.11.  
> 
> I'm not planning to fix this for this patch, since the problem already
> existed for a long time. I just wanted to put in this comment to 
> remind me for later :-)

  okay

> > > @@ -2141,6 +2152,44 @@ static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) {
> > >      return -1;
> > >  }
> > >  
> > > +
> > > +static int
> > > +qemuAssignPCIAddresses(virDomainDefPtr def)
> > > +{
> > > +    int ret = -1;
> > > +    unsigned long long qemuCmdFlags = 0;
> > > +    qemuDomainPCIAddressSetPtr addrs = NULL;
> > > +    struct stat sb;
> > > +
> > > +    if (stat(def->emulator, &sb) < 0) {
> > > +        virReportSystemError(errno,
> > > +                             _("Cannot find QEMU binary %s"),
> > > +                             def->emulator);
> > > +        goto cleanup;
> > > +    }
> > 
> >   do we really need to update that every time ? We can't cache forever
> > but it's not like the emulator is changing every second. Maybe we need
> > to put a watch on the emulator at the driver level and keep this in
> > the driver.
> 
> The only reason for the 'stat()' call is that the next method invoked
> qemudExtractVersionInfo() does nto give very nice error messages
> if the binary is missing. We should probably just move the stat()
> call into the qemudExtractVersionInfo() itself, rather than duplicating
> it everywhere that we invoke qemudExtractVersionInfo()

  Well I wasn't worried about the stat, more about fork'ing and exec'ing
qemu each time we want to see the capabilities flags.

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/