[libvirt] [PATCH 02/14] Retain disk PCI address across libvirtd restarts

[Daniel Veillard]

On Mon, Jul 20, 2009 at 03:05:31PM +0100, Mark McLoughlin wrote:
> On Mon, 2009-07-20 at 14:42 +0100, Daniel P. Berrange wrote:
> > On Mon, Jul 20, 2009 at 12:51:12PM +0100, Mark McLoughlin wrote:
> > > When we hot-plug a disk device into a qemu guest, we need to retain its
> > > PCI address so that it can be removed again later. Currently, we do
> > > retain the slot number, but not across libvirtd restarts.
> > > 
> > > Add <state devaddr="xxxx:xx:xx"/> to the disk device XML config when the
> > > VIR_DOMAIN_XML_INTERNAL_STATUS flag is used. We still don't parse the
> > > domain and bus number, but the format allows us to do that in future.
> > > 
> > > * src/domain_conf.h: replace slotnum with pci_addr
> > > 
> > 
> > 
> > > diff --git a/src/domain_conf.h b/src/domain_conf.h
> > > index 6e111fa..1766b61 100644
> > > --- a/src/domain_conf.h
> > > +++ b/src/domain_conf.h
> > > @@ -106,7 +106,7 @@ struct _virDomainDiskDef {
> > >      int cachemode;
> > >      unsigned int readonly : 1;
> > >      unsigned int shared : 1;
> > > -    int slotnum; /* pci slot number for unattach */
> > > +    char *pci_addr; /* for detach */
> > >  };
> > 
> > I think it'd be nicer to store the parsed address here as a
> > nested struct with domain, bus, slot.

  I understand dan 'here' as in the C struct not in the XML

> > It is not really saving us trouble by using a string, since most
> > of the places using this field end up asprintf'ing it into another
> > string, or even having to extract pieces out of it again.
> 
> It's saving us trouble because you don't have to code the equivalent of
> virDomainHostdevSubsysPciDefParseXML() and have e.g.
> 
>   <state>
>     <address domain="0" bus="0" slot="5"/>
>   </state>
> 
> I just now started to do it and then realized how much extra hassle the
> XML parsing was going to be. All for some internal data that we use in
> textual format anyway. Are you sure? :-)

  Well a single string in the XML is fine, but in the parsed Def let's
keep the bits as fully parsed, i.e. the set of ints we extract in patch
12/14
  Agreed with Dan,
  Agreed that separating them in the XML will make the code way more
complex especially for error handling.

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/