Re: [libvirt] [PATCH v2] Disk- and Controller Hotplug

Hi, Some time ago, Daniel P. Berrange wrote: > On Tue, Nov 24, 2009 at 02:15:58PM +0100, Wolfgang Mauerer wrote: >> Daniel P. Berrange wrote: >>> On Mon, Nov 23, 2009 at 02:15:06PM +0100, Wolfgang Mauerer wrote: >>>> Daniel P. Berrange wrote: >>>>> Wolfgang Mauerer wrote: >> okay, to avoid flooding the mailing list with nearly identical >> patches, I've put the rebased versions into a repository at >> git://git.kiszka.org/libvirt.git queue > > I've been taking a closer look at this, and I think the final state > of patch series as a whole looks good. The minor issue is that the > intermediate patches don't compile, since they rely on compile fixes > at the end of the series. Rather than ask you to re-format the > patches yet again, I'm going to just merge the patches into a smaller > set myself. yes, I've sacrificed individual compilability when I did the forward porting because I the amount of time I can dedicate to this right now is a bit restricted for various reasons. Thanks for doing the missing cleanups! > I've got just one question I'd like to understand before doing this. > On the <disk> element's new <controller> element, you are allowing > two mutually exclusive ways of specifying the controller > > <disk> ... <controller name="<identifier>" bus="<number>" > unit="<number>"/> </disk> > > and > > <disk> ... <controller pci_addr="<addr>" bus="<number>" > unit="<number>"/> </disk> > > I think it is going to cause unneccessary pain for applications to > have to cope with two different ways of specifying the same > relationship. So my intention is to remove the 'pci_addr' variant, > since that information can easily be got directly from the separate > top level <controller> device itself Getting rid of pci_addr is perfectly okay. I've kept it in the current series for setups that don't provide <controller>s, but it's supposed to be eliminated by design. Just go ahead if you want to do this it already now ;-) > On a side note, once we've applied this initial series, I think we'll > also need to be automatically adding a <controller> element in all > guest domains which have IDE or SCSI controllers present by default. > This is going to also force us to hook into QEMU's 'info pci' monitor > command to figure out their PCI address. This is long overdue though > and needed for NIC & Disk devices added at startup too, so this is > good motivation :-) agreed. I can try to address this when I bring back hot-removal.