wrote:

wrote:

On 11/13/2013 04:31 PM, Shivaprasad bhat wrote: > Hi Jan, Cole, > > Could you please reviewing my patch ? > > Thanks, > Shiva > I'd recommend adding a test case that demonstrates what exactly this is changing. - Cole > > On Wed, Oct 30, 2013 at 1:37 PM, Shivaprasad bhat < shivaprasadbhat(a)gmail.com > <mailto:shivaprasadbhat@gmail.com>> wrote: > > Hi, > > Could someone please help reviewing the patch ? > > Thanks and Regards, > Shiva > > > On Mon, Oct 28, 2013 at 2:50 PM, Shivaprasad G Bhat > <shivaprasadbhat(a)gmail.com <mailto:shivaprasadbhat@gmail.com>> wrote: > > The bus type IDE being enum Zero, the bus type on pseries system > appears as IDE for all the disk types. Pseries platform needs this to > appear as SCSI instead of IDE. > > Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com > <mailto:sbhat@linux.vnet.ibm.com>> > --- > src/qemu/qemu_domain.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index b8aec2d..df06c13 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -827,6 +827,12 @@ > qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, > if (dev->type == VIR_DOMAIN_DEVICE_DISK) { > virDomainDiskDefPtr disk = dev->data.disk; > > + if ((def->os.arch == VIR_ARCH_PPC64) && > + def->os.machine && STREQ(def->os.machine, "pseries") && > + (disk->bus == VIR_DOMAIN_DISK_BUS_IDE)) { > + disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; > + } > + > /* both of these require data from the driver config */ > if (driver && (cfg = virQEMUDriverGetConfig(driver))) { > /* assign default storage format and driver according to > config */ > @@ -868,6 +874,11 @@ > qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, > (def->os.arch == VIR_ARCH_S390 || def->os.arch == > VIR_ARCH_S390X)) > dev->data.chr->targetType = > VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; > > + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER && > + dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && > + def->os.machine && STREQ(def->os.machine, "pseries")) > + dev->data.controller->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; > + > /* set the default USB model to none for s390 unless an address > is found */ > if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER && > dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && > > -- > libvir-list mailing list > libvir-list(a)redhat.com <mailto:libvir-list@redhat.com> > https://www.redhat.com/mailman/listinfo/libvir-list > > >

On 11/14/2013 04:11 AM, Shivaprasad bhat wrote: > Thanks Cole. > > The change is to correct the IDE disk type to SCSI on pseries systems for > domxml-from-native. > > Here is the test case and results. > > sh# cat cmd.txt > *qemu-system-ppc64 -M pseries -m 4096 -nographic -enable-kvm -hda > /data/images/rhel70.qcow2 -name rhel70 -cdrom > /data/iso/RHEL-7.0-20130306.0-Server-ppc64-dvd1.iso -boot d -vnc :30* > > sh# ./run tools/*virsh domxml-from-native qemu-argv cmd.txt* > <domain type='kvm'> > <name>rhel70</name> > <uuid>eae019ae-a155-4dd8-be21-f9738b6aedea</uuid> > <memory unit='KiB'>4194304</memory> > <currentMemory unit='KiB'>4194304</currentMemory> > <vcpu placement='static'>1</vcpu> > <os> > <type arch='ppc64' machine='pseries'>hvm</type> > <boot dev='cdrom'/> > </os> > <clock offset='utc'/> > <on_poweroff>destroy</on_poweroff> > <on_reboot>restart</on_reboot> > <on_crash>destroy</on_crash> > <devices> > <emulator>qemu-system-ppc64</emulator> > <disk type='file' device='disk'> > <driver name='qemu' type='raw'/> > <source file='/data/images/rhel70.qcow2'/> > * <target dev='hda' bus='scsi'/> //----> ide is > changed to SCSI* > <address type='drive' controller='0' bus='0' target='0' unit='0'/> > </disk> > <disk type='file' device='cdrom'> > <driver name='qemu' type='raw'/> > <source file='/data/iso/RHEL-7.0-20130306.0-Server-ppc64-dvd1.iso'/> > * <target dev='hdc' bus='scsi'/> //----> ide is changed > to SCSI* > <readonly/> > <address type='drive' controller='0' bus='1' target='0' unit='0'/> > </disk> > * <controller type='scsi' index='0'/> //----> Controller ide > also changed to SCSI* > <controller type='usb' index='0'/> > <controller type='pci' index='0' model='pci-root'/> > <input type='mouse' bus='ps2'/> > <graphics type='vnc' port='5930' autoport='no' listen=''> > <listen type='address' address=''/> > </graphics> > <video> > <model type='cirrus' vram='9216' heads='1'/> > </video> > <memballoon model='virtio'/> > </devices> > </domain> > sh# > Thanks, but what I meant was to add an automated test case under the tests/ directory that demonstrates what is actually fixed. See tests/qemuargv2xmltest.c, will require appropriate *.args and *.xml files in tests/qemuxml2argvdata/ - Cole > > > On Thu, Nov 14, 2013 at 3:02 AM, Cole Robinson <crobinso(a)redhat.com > <mailto:crobinso@redhat.com>> wrote: > > On 11/13/2013 04:31 PM, Shivaprasad bhat wrote: > > Hi Jan, Cole, > > > > Could you please reviewing my patch ? > > > > Thanks, > > Shiva > > > > I'd recommend adding a test case that demonstrates what exactly this is > changing. > > - Cole > > > > > On Wed, Oct 30, 2013 at 1:37 PM, Shivaprasad bhat > <shivaprasadbhat(a)gmail.com <mailto:shivaprasadbhat@gmail.com> > > <mailto:shivaprasadbhat@gmail.com <mailto: shivaprasadbhat(a)gmail.com&gt;&gt;&gt; > wrote: > > > > Hi, > > > > Could someone please help reviewing the patch ? > > > > Thanks and Regards, > > Shiva > > > > > > On Mon, Oct 28, 2013 at 2:50 PM, Shivaprasad G Bhat > > <shivaprasadbhat(a)gmail.com <mailto:shivaprasadbhat@gmail.com> > <mailto:shivaprasadbhat@gmail.com <mailto:shivaprasadbhat@gmail.com>>> wrote: > > > > The bus type IDE being enum Zero, the bus type on pseries system > > appears as IDE for all the disk types. Pseries platform needs > this to > > appear as SCSI instead of IDE. > > > > Signed-off-by: Shivaprasad G Bhat < sbhat(a)linux.vnet.ibm.com > <mailto:sbhat@linux.vnet.ibm.com> > > <mailto:sbhat@linux.vnet.ibm.com <mailto: sbhat(a)linux.vnet.ibm.com&gt;&gt;&gt; > > --- > > src/qemu/qemu_domain.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > > index b8aec2d..df06c13 100644 > > --- a/src/qemu/qemu_domain.c > > +++ b/src/qemu/qemu_domain.c > > @@ -827,6 +827,12 @@ > > qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, > > if (dev->type == VIR_DOMAIN_DEVICE_DISK) { > > virDomainDiskDefPtr disk = dev->data.disk; > > > > + if ((def->os.arch == VIR_ARCH_PPC64) && > > + def->os.machine && STREQ(def->os.machine, "pseries") && > > + (disk->bus == VIR_DOMAIN_DISK_BUS_IDE)) { > > + disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; > > + } > > + > > /* both of these require data from the driver config */ > > if (driver && (cfg = virQEMUDriverGetConfig(driver))) { > > /* assign default storage format and driver > according to > > config */ > > @@ -868,6 +874,11 @@ > > qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, > > (def->os.arch == VIR_ARCH_S390 || def->os.arch == > > VIR_ARCH_S390X)) > > dev->data.chr->targetType = > > VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; > > > > + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER && > > + dev->data.controller->type == > VIR_DOMAIN_CONTROLLER_TYPE_IDE && > > + def->os.machine && STREQ(def->os.machine, "pseries")) > > + dev->data.controller->type = > VIR_DOMAIN_CONTROLLER_TYPE_SCSI; > > + > > /* set the default USB model to none for s390 unless an address > > is found */ > > if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER && > > dev->data.controller->type == > VIR_DOMAIN_CONTROLLER_TYPE_USB && > > > > -- > > libvir-list mailing list > > libvir-list(a)redhat.com <mailto:libvir-list@redhat.com> > <mailto:libvir-list@redhat.com <mailto:libvir-list@redhat.com>> > > https://www.redhat.com/mailman/listinfo/libvir-list > > > > > > > >

On 10/28/2013 10:20 AM, Shivaprasad G Bhat wrote: > The bus type IDE being enum Zero, the bus type on pseries system appears as IDE for all the disk types. Pseries platform needs this to appear as SCSI instead of IDE. > > Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com&gt; > --- > src/qemu/qemu_domain.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index b8aec2d..df06c13 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -827,6 +827,12 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, > if (dev->type == VIR_DOMAIN_DEVICE_DISK) { > virDomainDiskDefPtr disk = dev->data.disk; > > + if ((def->os.arch == VIR_ARCH_PPC64) && > + def->os.machine && STREQ(def->os.machine, "pseries") && > + (disk->bus == VIR_DOMAIN_DISK_BUS_IDE)) { > + disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; > + } > + > /* both of these require data from the driver config */ > if (driver && (cfg = virQEMUDriverGetConfig(driver))) { > /* assign default storage format and driver according to config */ > @@ -868,6 +874,11 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, > (def->os.arch == VIR_ARCH_S390 || def->os.arch == VIR_ARCH_S390X)) > dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO; > > + if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER && > + dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && > + def->os.machine && STREQ(def->os.machine, "pseries")) > + dev->data.controller->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI; > + > /* set the default USB model to none for s390 unless an address is found */ > if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER && > dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && > This would also affect XML parsing, as these PostParse functions are called when parsing the XML too, not just when doing a XML->native translation. This also affect all the disks specified on the command line. You shouldn't assume the disk is SCSI when IDE was explicitly specified.

Jan