[libvirt] [PATCH v2] virsh domxml-from-native to treat SCSI as the bus type for pseries by default

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@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 &&

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@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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Hi Jan, Cole, Could you please reviewing my patch ? Thanks, Shiva On Wed, Oct 30, 2013 at 1:37 PM, Shivaprasad bhat <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@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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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@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@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@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@redhat.com <mailto:libvir-list@redhat.com> https://www.redhat.com/mailman/listinfo/libvir-list

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# Regards, Shiva On Thu, Nov 14, 2013 at 3:02 AM, Cole Robinson <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 <
<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@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
shivaprasadbhat@gmail.com this to
appear as SCSI instead of IDE.
Signed-off-by: Shivaprasad G Bhat <sbhat@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@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@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@gmail.com <mailto:shivaprasadbhat@gmail.com> > <mailto:shivaprasadbhat@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@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@linux.vnet.ibm.com <mailto:sbhat@linux.vnet.ibm.com> > <mailto:sbhat@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@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 > > >

Thanks Cole. There are some changes to the patch. Next patch I'll try to add. Thanks, Shiva On Thu, Nov 14, 2013 at 7:31 PM, Cole Robinson <crobinso@redhat.com> wrote:
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@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@gmail.com <mailto:shivaprasadbhat@gmail.com> > <mailto:shivaprasadbhat@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@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@linux.vnet.ibm.com
<mailto:sbhat@linux.vnet.ibm.com> > <mailto:sbhat@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@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@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 &&
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

On 2013年11月14日 18:02, Ján Tomko wrote:
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@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 &&
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.
This patch is to parse "-hda" as SCSI device type because Power doesn't support IDE device. I am not sure whether it's necessary to keep IDE device type for Power in libvirt. Currently, QEMU set "-hda" as SCSI for Power platform.
Jan
-- Li Zhang IBM China Linux Technology Centre

On 11/15/2013 04:20 AM, Li Zhang wrote:
On 2013年11月14日 18:02, Ján Tomko wrote:
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@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
... 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.
This patch is to parse "-hda" as SCSI device type because Power doesn't support IDE device. I am not sure whether it's necessary to keep IDE device type for Power in libvirt. Currently, QEMU set "-hda" as SCSI for Power platform.
Yes, since QEMU treats it as SCSI, we could do the same for -hdX. I meant that we shouldn't correct command line like -device ide-cd,drive=ddd,bus=ide.0 to SCSI. But it seems domxml-to-native can't even parse the command line libvirt outputs for disks at the moment: error: internal error: missing index/unit/bus parameter in drive 'file=/var/iso/f19.iso,if=none,id=drive-ide0-1-0,readonly=on,format=raw' Jan

On 2013年11月15日 15:59, Ján Tomko wrote:
On 11/15/2013 04:20 AM, Li Zhang wrote:
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@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
... 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. This patch is to parse "-hda" as SCSI device type because Power doesn't support IDE device. I am not sure whether it's necessary to keep IDE device type for Power in
On 2013年11月14日 18:02, Ján Tomko wrote: libvirt. Currently, QEMU set "-hda" as SCSI for Power platform.
Yes, since QEMU treats it as SCSI, we could do the same for -hdX. I meant that we shouldn't correct command line like -device ide-cd,drive=ddd,bus=ide.0 to SCSI.
Yes, you are right. We should also tell users not to use IDE device on Power. :)
But it seems domxml-to-native can't even parse the command line libvirt outputs for disks at the moment: error: internal error: missing index/unit/bus parameter in drive 'file=/var/iso/f19.iso,if=none,id=drive-ide0-1-0,readonly=on,format=raw'
Hmm, it seems that few people use this command since the error still exists.
Jan
-- Li Zhang IBM China Linux Technology Centre

Hi Jan, Thanks for the replies. On Fri, Nov 15, 2013 at 1:29 PM, Ján Tomko <jtomko@redhat.com> wrote:
On 11/15/2013 04:20 AM, Li Zhang wrote:
On 2013年11月14日 18:02, Ján Tomko wrote:
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@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
... 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.
This patch is to parse "-hda" as SCSI device type because Power doesn't
support IDE device. I am not sure whether it's necessary to keep IDE device type for Power in libvirt. Currently, QEMU set "-hda" as SCSI for Power platform.
Yes, since QEMU treats it as SCSI, we could do the same for -hdX. I meant that we shouldn't correct command line like -device ide-cd,drive=ddd,bus=ide.0 to SCSI.
I checked that the domxml-to-native doesnt process -device option in qemuParseCommandLine. It simply add an arg entry into the xml. <qemu:arg value='-device'/> <qemu:arg value='scsi-cd,bus=scsi0.0,channel=0,scsi-id=0,lun=2,drive=drive-scsi0-0-0-2,id=scsi0-0-0-2,bootindex=1'/> May be we should check these ide strings for device option in qemuParseCommandLine() and error out. Let me know if this is the right approach than doing in PostParsing.
But it seems domxml-to-native can't even parse the command line libvirt outputs for disks at the moment: error: internal error: missing index/unit/bus parameter in drive 'file=/var/iso/f19.iso,if=none,id=drive-ide0-1-0,readonly=on,format=raw'
The -drive option parsing doesnt process the id="XXXXXX" attribute today. The bus, index, unitid need to be derived from the string "drive-ide0-1-0". Let me know if you want me to add the parsing code. Otherwise, I see there is no mention of id="" in any of the common usage documentation. Link that i referred is http://wiki.qemu.org/download/qemu-doc.html. Is that a hidden attribute?
So, I assume only the "un-documented" options("id=drive-ide0-1-0") may be "many" of them are not working. Otherwise options "drive file=file,if=scsi,bus=0,unit=6" or "qemu -drive file=file,index=0,if=floppy" are working. The documented usage "-drive file=a" and "-drive file=b" Fails though. Even the id attribute parsing doesnt help here :) Thanks and Regards Shiva Jan

On 11/15/2013 03:10 PM, Shivaprasad bhat wrote:
I checked that the domxml-to-native doesnt process -device option in qemuParseCommandLine. It simply add an arg entry into the xml. <qemu:arg value='-device'/> <qemu:arg value='scsi-cd,bus=scsi0.0,channel=0,scsi-id=0,lun=2,drive=drive-scsi0-0-0-2,id=scsi0-0-0-2,bootindex=1'/>
May be we should check these ide strings for device option in qemuParseCommandLine() and error out. Let me know if this is the right approach than doing in PostParsing.
Knowing how to parse them (at least the command line libvirt generates) would be better :) Personally, I think adding the arg entry is better than an error. IMO the way to do it would be in the functions that parse the "-hdX" option (and "-drive", if they work).
But it seems domxml-to-native can't even parse the command line libvirt outputs for disks at the moment: error: internal error: missing index/unit/bus parameter in drive 'file=/var/iso/f19.iso,if=none,id=drive-ide0-1-0,readonly=on,format=raw'
The -drive option parsing doesnt process the id="XXXXXX" attribute today. The bus, index, unitid need to be derived from the string "drive-ide0-1-0". Let me know if you want me to add the parsing code. Otherwise, I see there is no mention of id="" in any of the common usage documentation. Link that i referred is http://wiki.qemu.org/download/qemu-doc.html. Is that a hidden attribute?
id is just a name, the important part is the -device argument that contains that uses the id in the drive= parameter, and also specifies the bus. Jan
participants (5)
-
Cole Robinson
-
Ján Tomko
-
Li Zhang
-
Shivaprasad bhat
-
Shivaprasad G Bhat