On 06/19/2012 04:26 AM, Dipankar Sarma wrote:
>
> From: Dipankar Sarma <dipankar@xxxxxxxxxx>
>
> Fix emumeration of scsi disks to use scsi id instead of lun. Without
s/emumeration/enumeration/
> this, pseries partition firmware (OF) breaks. Also, as per PAPR,
> one controler per device for ibmvscsi.
s/controler/controller/
>
> Signed-off-by: Dipankar Sarma <dipankar@xxxxxxxxxx>
> ---
>
> 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?
>
>
> 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.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list