Am 28.04.2015 um 04:59 hat tu bo geschrieben: > Hi Kevin: > > On 04/27/2015 07:34 PM, Kevin Wolf wrote: > > Am 27.04.2015 um 13:24 hat Max Reitz geschrieben: > > On 27.04.2015 09:15, tu bo wrote: > > Hi Max: > > On 04/24/2015 01:07 AM, Max Reitz wrote: > > Well, that's a peculiar commit title. :-) > > I guess it's supposed to be "qemu-iotests: s390x: fix test > 130"? > > You're right. I will change it in the next version :-) > > > On 23.04.2015 04:42, Xiao Guang Chen wrote: > > From: Bo Tu <t...@linux.vnet.ibm.com> > > The tests for device type "ide_cd" should only be tested > for the pc > platform. > The default device id of hard disk on the s390 platform > differs to > that > of the x86 platform. A new variable device_id is defined > and > "virtio0" > set for the s390 platform. A x86 platform specific output > file is > also > needed. > > Signed-off-by: Bo Tu <t...@linux.vnet.ibm.com> > --- > tests/qemu-iotests/130 | 13 +++++++++++-- > tests/qemu-iotests/130.out | 4 ++-- > tests/qemu-iotests/130.pc.out | 43 > +++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 56 insertions(+), 4 deletions(-) > create mode 100644 tests/qemu-iotests/130.pc.out > > diff --git a/tests/qemu-iotests/130 > b/tests/qemu-iotests/130 > index bc26247..de40c7b 100755 > --- a/tests/qemu-iotests/130 > +++ b/tests/qemu-iotests/130 > @@ -58,9 +58,18 @@ echo "=== HMP commit ===" > echo > # bdrv_make_empty() involves a header update for qcow2 > +case "$QEMU_DEFAULT_MACHINE" in > + pc) > + device_id="ide0-hd0" > + ;; > + s390) > + device_id="virtio0" > + ;; > > > I think I mentioned before that I don't really like not > taking the fact > into account that there are other machine types, too. I'm > still > accepting it based on the fact that I think those machine > types won't > pass the tests right now anyway, so not caring for them in > these case > blocks won't break any tests, but it still feels like > something we can > avoid (like defaulting to virtio0 for any non-pc platform). > > Anyway, because I seem to remember I accepted it before: > > With the commit title fixed: > > Reviewed-by: Max Reitz <mre...@redhat.com> > > I guess you discussed with Xiao Guang Chen and accepted it in > "[PATCH RFC > v5 6/7] qemu-iotests s390x fix test-051", because test 130 and > 051 are > using the same fix solution, and test 051 was fixed in v5. Seeing > section > of v5 in cover letter as below: > > > Indeed we discussed it. Just for clarification, I disliked having > only cases > for "pc" and "s390" -- there are other platforms, too, which will > simply break > by not including them in these case statements. We could try to avoid > this by > defaulting to virtio0 for every non-pc platform, and it will probably > work for > most without having to do further work here. > > However, I did accept it because all those non-PC (and non-s390) > platforms > won't pass the tests before this patch set either (because these test > cases try > to use IDE devices which will not be available there). So the series > will not > break them because they didn't work before either. > > Bottom line: I'm fine with this solution as it is. > > Maybe I'm missing the obvious, but why don't you just specify an > explicit ID instead of guessing the default ID that qemu will use > depending on the platform? > > Please forgive me that I'm very sure about the meaning of "default ID" you > mentioned. Maybe you mean "default device ID"? If I'm wrong, please correct me > :-) > > The default device id of hard disk on the s390 platform differs to the device > id on the x86 platform, so we need to use different device id for different > platform. For instance, using "virtio0" for s390x, and using "ide0-hd0" for > x86 as below: > +_send_qemu_cmd $QEMU_HANDLE "commit " "virtio0" "(qemu)" > +_send_qemu_cmd $QEMU_HANDLE "commit " "ide0-hd0" "(qemu)"
Any why not use this? qemu -drive id=testdisk,... (qemu) commit testdisk That would be the same on all platforms. Kevin
