[libvirt] [PATCH 2/2] one drive per controller for ibmvscsi

[Dipankar Sarma]

On Tue, Jun 19, 2012 at 03:55:47PM -0600, Eric Blake wrote:
> On 06/19/2012 04:26 AM, Dipankar Sarma wrote:
> > 
> >  src/conf/domain_conf.c  |   40 +++++++++++++++++++++++++++++++++-------
> >  src/conf/domain_conf.h  |    2 +-
> >  src/qemu/qemu_command.c |    9 +++++----
> >  src/vmx/vmx.c           |   28 ++++++++++++++++------------
> >  src/vmx/vmx.h           |    6 +++---
> >  5 files changed, 58 insertions(+), 27 deletions(-)
> 
> This change is a bit bigger than the last; is there any way you can add
> to tests/qemuxml2argvtest.c (and by implication, to
> tests/qemuxml2argvdata) to cover the new code paths?

Will do.

> 
> > 
> > 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> 
> > @@ -3004,8 +3022,15 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def)
> >      switch (def->bus) {
> >      case VIR_DOMAIN_DISK_BUS_SCSI:
> >          def->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
> > -
> > -        if (caps->hasWideScsiBus) {
> > +        if (STREQ(ddef->os.arch, "ppc64") &&
> 
> This doesn't feel right.  We shouldn't be hard-coding strings into the
> generic domain_conf code, so much as letting specific hypervisor drivers
> be making decisions.  Remember, running LXC on ppc64 does not
> necessarily have the same default bus handling as running a ppc64 guest
> via qemu on an x86 machine.  We either need a new field in caps (similar
> to caps->hasWideScsiBus) or even a callback function, so that the
> hypervisor driver code can answer questions about what defaults to use
> as part of the existing caps mechanism.  The fact that you even had to
> touch vmx and qemu code in the same patch for a feature that you are
> really only trying to add to qemu support is further evidence that this
> is not quite right.

Thanks for the hint. That seems cleaner. It should be useful for some
other changes for ppc64 that I am working on. I will rework this patch.

Thanks
Dipankar