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. Best regards, Wolfgang -- Siemens AG, Corporate Technology CT T DE IT 1 Corporate Competence Centre Embedded Linux
On Wed, Dec 02, 2009 at 11:04:25PM +0100, Wolfgang Mauerer wrote:
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.
I actually think I might be able to get all this including hot-removal 'for free' as part of other work I'm doing to convert libvirt over to use '-device' arg / 'device_add', 'device_remove' monitor commands for all device configuration. So don't spend time on it just yet. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Daniel P. Berrange -
Wolfgang Mauerer