Re: [Qemu-block] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default
On Mon, 19 Oct 2015 14:57:31 +0300 "Michael S. Tsirkin" wrote: > On Mon, Oct 19, 2015 at 01:53:50PM +0200, Cornelia Huck wrote: > > On Sun, 18 Oct 2015 10:59:59 +0300 > > "Michael S. Tsirkin" wrote: > > > > > On Fri, Oct 16, 2015 at 01:07:28PM +0200, Christian Borntraeger wrote: > > > > > > Lets keep this patch as is to have scsi=off as default for virtio 1.0 > > > > > > > > (some iotests do fail because of this) > > > > > > > > Christian > > > > > > What fails, exactly? > > > > For example, testcase 068 (on s390x, since we default virtio-1 to on): > > I see, thanks. > So it's just the assertion that we have in code that fires. > Sure, scsi must be off by default before virtio 1 is on. And -hda always assumes defaults...
Re: [Qemu-block] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default
On Mon, Oct 19, 2015 at 01:53:50PM +0200, Cornelia Huck wrote: > On Sun, 18 Oct 2015 10:59:59 +0300 > "Michael S. Tsirkin" wrote: > > > On Fri, Oct 16, 2015 at 01:07:28PM +0200, Christian Borntraeger wrote: > > > > Lets keep this patch as is to have scsi=off as default for virtio 1.0 > > > > > > (some iotests do fail because of this) > > > > > > Christian > > > > What fails, exactly? > > For example, testcase 068 (on s390x, since we default virtio-1 to on): I see, thanks. So it's just the assertion that we have in code that fires. Sure, scsi must be off by default before virtio 1 is on. > --- /data/git/yyy/qemu/tests/qemu-iotests/068.out 2015-03-09 > 12:32:35.245444052 +0100 > +++ 068.out.bad 2015-10-19 13:48:00.023772655 +0200 > @@ -3,9 +3,8 @@ > === Saving and reloading a VM state to/from a qcow2 image === > > Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 > +qemu-system-s390x: -hda > /data/git/yyy/qemu/build/tests/qemu-iotests/scratch/t.qcow2: Please set > scsi=off for virtio-blk devices in order to use virtio 1.0 > QEMU X.Y.Z monitor - type 'help' for more information > -(qemu) savevm 0 > -(qemu) quit > +(qemu) qemu-system-s390x: -hda > /data/git/yyy/qemu/build/tests/qemu-iotests/scratch/t.qcow2: Please set > scsi=off for virtio-blk devices in order to use virtio 1.0 > QEMU X.Y.Z monitor - type 'help' for more information > -(qemu) quit > -*** done > +(qemu) *** done
Re: [Qemu-block] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default
On Sun, 18 Oct 2015 10:59:59 +0300 "Michael S. Tsirkin" wrote: > On Fri, Oct 16, 2015 at 01:07:28PM +0200, Christian Borntraeger wrote: > > Lets keep this patch as is to have scsi=off as default for virtio 1.0 > > > > (some iotests do fail because of this) > > > > Christian > > What fails, exactly? For example, testcase 068 (on s390x, since we default virtio-1 to on): --- /data/git/yyy/qemu/tests/qemu-iotests/068.out 2015-03-09 12:32:35.245444052 +0100 +++ 068.out.bad 2015-10-19 13:48:00.023772655 +0200 @@ -3,9 +3,8 @@ === Saving and reloading a VM state to/from a qcow2 image === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 +qemu-system-s390x: -hda /data/git/yyy/qemu/build/tests/qemu-iotests/scratch/t.qcow2: Please set scsi=off for virtio-blk devices in order to use virtio 1.0 QEMU X.Y.Z monitor - type 'help' for more information -(qemu) savevm 0 -(qemu) quit +(qemu) qemu-system-s390x: -hda /data/git/yyy/qemu/build/tests/qemu-iotests/scratch/t.qcow2: Please set scsi=off for virtio-blk devices in order to use virtio 1.0 QEMU X.Y.Z monitor - type 'help' for more information -(qemu) quit -*** done +(qemu) *** done
Re: [Qemu-block] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default
On Fri, Oct 16, 2015 at 01:07:28PM +0200, Christian Borntraeger wrote: > Am 16.10.2015 um 12:44 schrieb Cornelia Huck: > > On Fri, 16 Oct 2015 12:32:52 +0200 > > Christian Borntraeger wrote: > > > >> Am 16.10.2015 um 12:25 schrieb Cornelia Huck: > >>> Devices that are compliant with virtio-1 do not support scsi > >>> passthrough any more (and it has not been a recommended setup > >>> anyway for quite some time). To avoid having to switch it off > >>> explicitly in newer qemus that turn on virtio-1 by default, let's > >>> switch the default to scsi=false for 2.5. > >>> > >>> Signed-off-by: Cornelia Huck > >>> --- > >>> hw/block/virtio-blk.c | 2 +- > >>> include/hw/compat.h | 6 +- > >>> 2 files changed, 6 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > >>> index 8beb26b..999dbd7 100644 > >>> --- a/hw/block/virtio-blk.c > >>> +++ b/hw/block/virtio-blk.c > >>> @@ -975,7 +975,7 @@ static Property virtio_blk_properties[] = { > >>> DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial), > >>> DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true), > >>> #ifdef __linux__ > >>> -DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, true), > >>> +DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false), > >>> #endif > >>> DEFINE_PROP_BIT("request-merging", VirtIOBlock, > >>> conf.request_merging, 0, > >>> true), > >>> diff --git a/include/hw/compat.h b/include/hw/compat.h > >>> index 095de5d..93e71af 100644 > >>> --- a/include/hw/compat.h > >>> +++ b/include/hw/compat.h > >>> @@ -2,7 +2,11 @@ > >>> #define HW_COMPAT_H > >>> > >>> #define HW_COMPAT_2_4 \ > >>> -/* empty */ > >>> +{\ > >>> +.driver = "virtio-blk-device",\ > >>> +.property = "scsi",\ > >>> +.value= "true",\ > >> > >> does that work? > > > > It did for me :) > > > >> > >> If yes, would it make sense to convert the things in HW_COMPAT_2_3 from > >> pci to device, e.g. > >> > >> > >> {\ > >> - .driver = "virtio-blk-pci",\ > >> + .driver = "virtio-blk-device",\ > >> .property = "any_layout",\ > >> .value= "off",\ > >> ... > > > > Not sure: We don't have 2.3 compat for ccw... but would give a better > > template for later changes. > > Yes. But this can be an addon patch. > > Lets keep this patch as is to have scsi=off as default for virtio 1.0 > > (some iotests do fail because of this) > > Christian What fails, exactly?
Re: [Qemu-block] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default
Am 16.10.2015 um 12:44 schrieb Cornelia Huck: > On Fri, 16 Oct 2015 12:32:52 +0200 > Christian Borntraeger wrote: > >> Am 16.10.2015 um 12:25 schrieb Cornelia Huck: >>> Devices that are compliant with virtio-1 do not support scsi >>> passthrough any more (and it has not been a recommended setup >>> anyway for quite some time). To avoid having to switch it off >>> explicitly in newer qemus that turn on virtio-1 by default, let's >>> switch the default to scsi=false for 2.5. >>> >>> Signed-off-by: Cornelia Huck >>> --- >>> hw/block/virtio-blk.c | 2 +- >>> include/hw/compat.h | 6 +- >>> 2 files changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c >>> index 8beb26b..999dbd7 100644 >>> --- a/hw/block/virtio-blk.c >>> +++ b/hw/block/virtio-blk.c >>> @@ -975,7 +975,7 @@ static Property virtio_blk_properties[] = { >>> DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial), >>> DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true), >>> #ifdef __linux__ >>> -DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, true), >>> +DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false), >>> #endif >>> DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, >>> 0, >>> true), >>> diff --git a/include/hw/compat.h b/include/hw/compat.h >>> index 095de5d..93e71af 100644 >>> --- a/include/hw/compat.h >>> +++ b/include/hw/compat.h >>> @@ -2,7 +2,11 @@ >>> #define HW_COMPAT_H >>> >>> #define HW_COMPAT_2_4 \ >>> -/* empty */ >>> +{\ >>> +.driver = "virtio-blk-device",\ >>> +.property = "scsi",\ >>> +.value= "true",\ >> >> does that work? > > It did for me :) > >> >> If yes, would it make sense to convert the things in HW_COMPAT_2_3 from >> pci to device, e.g. >> >> >> {\ >> - .driver = "virtio-blk-pci",\ >> + .driver = "virtio-blk-device",\ >> .property = "any_layout",\ >> .value= "off",\ >> ... > > Not sure: We don't have 2.3 compat for ccw... but would give a better > template for later changes. Yes. But this can be an addon patch. Lets keep this patch as is to have scsi=off as default for virtio 1.0 (some iotests do fail because of this) Christian
Re: [Qemu-block] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default
On Fri, 16 Oct 2015 12:32:52 +0200 Christian Borntraeger wrote: > Am 16.10.2015 um 12:25 schrieb Cornelia Huck: > > Devices that are compliant with virtio-1 do not support scsi > > passthrough any more (and it has not been a recommended setup > > anyway for quite some time). To avoid having to switch it off > > explicitly in newer qemus that turn on virtio-1 by default, let's > > switch the default to scsi=false for 2.5. > > > > Signed-off-by: Cornelia Huck > > --- > > hw/block/virtio-blk.c | 2 +- > > include/hw/compat.h | 6 +- > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > > index 8beb26b..999dbd7 100644 > > --- a/hw/block/virtio-blk.c > > +++ b/hw/block/virtio-blk.c > > @@ -975,7 +975,7 @@ static Property virtio_blk_properties[] = { > > DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial), > > DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true), > > #ifdef __linux__ > > -DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, true), > > +DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false), > > #endif > > DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, > > 0, > > true), > > diff --git a/include/hw/compat.h b/include/hw/compat.h > > index 095de5d..93e71af 100644 > > --- a/include/hw/compat.h > > +++ b/include/hw/compat.h > > @@ -2,7 +2,11 @@ > > #define HW_COMPAT_H > > > > #define HW_COMPAT_2_4 \ > > -/* empty */ > > +{\ > > +.driver = "virtio-blk-device",\ > > +.property = "scsi",\ > > +.value= "true",\ > > does that work? It did for me :) > > If yes, would it make sense to convert the things in HW_COMPAT_2_3 from > pci to device, e.g. > > > {\ > - .driver = "virtio-blk-pci",\ > + .driver = "virtio-blk-device",\ > .property = "any_layout",\ > .value= "off",\ > ... Not sure: We don't have 2.3 compat for ccw... but would give a better template for later changes.
Re: [Qemu-block] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default
Am 16.10.2015 um 12:25 schrieb Cornelia Huck: > Devices that are compliant with virtio-1 do not support scsi > passthrough any more (and it has not been a recommended setup > anyway for quite some time). To avoid having to switch it off > explicitly in newer qemus that turn on virtio-1 by default, let's > switch the default to scsi=false for 2.5. > > Signed-off-by: Cornelia Huck > --- > hw/block/virtio-blk.c | 2 +- > include/hw/compat.h | 6 +- > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 8beb26b..999dbd7 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -975,7 +975,7 @@ static Property virtio_blk_properties[] = { > DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial), > DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true), > #ifdef __linux__ > -DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, true), > +DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false), > #endif > DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0, > true), > diff --git a/include/hw/compat.h b/include/hw/compat.h > index 095de5d..93e71af 100644 > --- a/include/hw/compat.h > +++ b/include/hw/compat.h > @@ -2,7 +2,11 @@ > #define HW_COMPAT_H > > #define HW_COMPAT_2_4 \ > -/* empty */ > +{\ > +.driver = "virtio-blk-device",\ > +.property = "scsi",\ > +.value= "true",\ does that work? If yes, would it make sense to convert the things in HW_COMPAT_2_3 from pci to device, e.g. {\ - .driver = "virtio-blk-pci",\ + .driver = "virtio-blk-device",\ .property = "any_layout",\ .value= "off",\ ...
[Qemu-block] [PATCH v2 3/3] virtio-blk: switch off scsi-passthrough by default
Devices that are compliant with virtio-1 do not support scsi passthrough any more (and it has not been a recommended setup anyway for quite some time). To avoid having to switch it off explicitly in newer qemus that turn on virtio-1 by default, let's switch the default to scsi=false for 2.5. Signed-off-by: Cornelia Huck --- hw/block/virtio-blk.c | 2 +- include/hw/compat.h | 6 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 8beb26b..999dbd7 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -975,7 +975,7 @@ static Property virtio_blk_properties[] = { DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial), DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true), #ifdef __linux__ -DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, true), +DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false), #endif DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging, 0, true), diff --git a/include/hw/compat.h b/include/hw/compat.h index 095de5d..93e71af 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -2,7 +2,11 @@ #define HW_COMPAT_H #define HW_COMPAT_2_4 \ -/* empty */ +{\ +.driver = "virtio-blk-device",\ +.property = "scsi",\ +.value= "true",\ +}, #define HW_COMPAT_2_3 \ {\ -- 2.3.9