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

[Daniel P. Berrange]

On Tue, Nov 24, 2009 at 02:15:58PM +0100, Wolfgang Mauerer wrote:
> Hi,
> 
> Daniel P. Berrange wrote:
> > On Mon, Nov 23, 2009 at 02:15:06PM +0100, Wolfgang Mauerer wrote:
> >> Daniel P. Berrange wrote:
> >>> On Tue, Nov 17, 2009 at 12:53:31AM +0100, wolfgang.mauerer at siemens.com wrote:
> >>>> this is the second revision of a patch series to improve disk
> >>>> hotadd support for libvirt. It focuses on the qemu backend, but
> >>>> is naturally designed to be compatible with other backends as
> >>>> well. The objective is two-fold:
> >>>>
> >>>> 1.) Split off controller from disk handling. This is done by
> >>>>     introducing a new domain element <controller> that is used to
> >>>>     describe disk controllers. Support for hotplugging such
> >>>>     controllers is added.  Support to reference controllers by
> >>>>     name is also included.
> >>>> 2.) <disk>s can now be associated with a specific controller;
> >>>>     this is done by means of a <controller> subelemnt for disks.
> >>>>
> >>>> This patch addresses the questions that were raised during the
> >>>> review of the initial submission, massages the code by fixing
> >>>> some whitespace issues, gets static controller configurations to
> >>>> work, and adds documentation. Notice that in contrast to the
> >>>> first submission I did not include the patch that adds support
> >>>> for disk- and controller hot_remove_. Since the qemu codebase is
> >>>> still in bit of a flux wrt.  the necessary patches required for
> >>>> this functionality, it will reappear some time later as a
> >>>> separate submission.
> >>> What libvirt version / GIT changeset  did you create these patches
> >>> against ?  The current libvirt QEMU driver code in GIT is quite
> >>> different, so the patches here don't apply for me as is.
> >> sorry for the late reply, I could not access my eMail during
> >> the last couple of days. Patches are on top of
> >> 790f0b3057787bb64, I did not realise that this one was only
> >> in the middle of qemu refactoring, not at the end :-(
> >>
> >> Do you plan any more refactorings to the qemu base in the near
> >> future, and if yes, are these already available somewhere? I'd
> >> like to avoid another useless rebase...
> > 
> > No, the monitor code for the QEMU driver is stable now. I'll only be adding
> > extra functionality, not changing existing stuff, so it should be good to
> > rebase against now.
> 
> 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.

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


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

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