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

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.

> 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? :-)