Re: [Qemu-block] [PATCH V2 2/5] virtio-blk: advertise scsi only when scsi is set
On 07/15/2015 03:57 PM, Paolo Bonzini wrote:
On 15/07/2015 07:29, Jason Wang wrote:
Cc: Stefan Hajnoczi stefa...@redhat.com
Cc: Kevin Wolf kw...@redhat.com
Cc: qemu-block@nongnu.org
Signed-off-by: Jason Wang jasow...@redhat.com
---
hw/block/virtio-blk.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 4c27974..761d763 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -731,7 +731,9 @@ static uint64_t virtio_blk_get_features(VirtIODevice
*vdev, uint64_t features,
virtio_add_feature(features, VIRTIO_BLK_F_GEOMETRY);
virtio_add_feature(features, VIRTIO_BLK_F_TOPOLOGY);
virtio_add_feature(features, VIRTIO_BLK_F_BLK_SIZE);
-virtio_add_feature(features, VIRTIO_BLK_F_SCSI);
+if (s-conf.scsi) {
+virtio_add_feature(features, VIRTIO_BLK_F_SCSI);
+}
This must only be done for newer machine types only, or you change guest
ABI for scsi=off. Effectively you have to split it in two properties,
scsi and always_set_f_scsi.
Paolo
And always_set_f_scsi is true only for legacy machine types?
Re: [Qemu-block] [Qemu-devel][PATCH]block/truncate: qcow2’s resize status is not showed to
Am 15.07.2015 um 03:57 hat 김태하 geschrieben:
To put it briefly, when resize qcow2 image, the file tool detected increased
size. However, the ls, “stat”, and “du” utility still don't know increased
size. The following patch enables to let userland tools - ls, du, stat - know
and apply changed size after resizing qcow2 image created by the qemu-img
tool.
Currently, can know only using “file” utility without this patch.
Even your goal is already wrong. For any image format other than raw,
the file size doesn't represent the virtual disk size. It's just the
size that the image file actually takes on the host, and that's
something entirely different.
The correct way to check the virtual disk size is 'qemu-img info'.
==
Signed-off-by: Taeha Kim thlab@samsung.com
diff --git a/block.c b/block.c
index 5e80336..277adf3 100644
--- a/block.c
+++ b/block.c
@@ -2473,6 +2473,9 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
if (bs-blk) {
blk_dev_resize_cb(bs-blk);
}
+ if (ret == 0) {
+ ret = truncate(bs-filename, offset);
+ }
}
return ret;
}
Even if your goal were right, this wouldn't work because images don't
have to be local files.
And to be clear: Truncating the image file is not only misguided and
unnecessary, but it will actively corrupt image files, because qcow2
images can be larger than the virtual disk size (e.g. if you have taken
some snapshots). In this case you would destroy the image by throwing
away part of it.
Kevin
Re: [Qemu-block] [PATCH V2 2/5] virtio-blk: advertise scsi only when scsi is set
On 15/07/2015 07:29, Jason Wang wrote:
Cc: Stefan Hajnoczi stefa...@redhat.com
Cc: Kevin Wolf kw...@redhat.com
Cc: qemu-block@nongnu.org
Signed-off-by: Jason Wang jasow...@redhat.com
---
hw/block/virtio-blk.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 4c27974..761d763 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -731,7 +731,9 @@ static uint64_t virtio_blk_get_features(VirtIODevice
*vdev, uint64_t features,
virtio_add_feature(features, VIRTIO_BLK_F_GEOMETRY);
virtio_add_feature(features, VIRTIO_BLK_F_TOPOLOGY);
virtio_add_feature(features, VIRTIO_BLK_F_BLK_SIZE);
-virtio_add_feature(features, VIRTIO_BLK_F_SCSI);
+if (s-conf.scsi) {
+virtio_add_feature(features, VIRTIO_BLK_F_SCSI);
+}
This must only be done for newer machine types only, or you change guest
ABI for scsi=off. Effectively you have to split it in two properties,
scsi and always_set_f_scsi.
Paolo
Re: [Qemu-block] [PATCH V2 2/5] virtio-blk: advertise scsi only when scsi is set
On 15/07/2015 10:31, Jason Wang wrote: This must only be done for newer machine types only, or you change guest ABI for scsi=off. Effectively you have to split it in two properties, scsi and always_set_f_scsi. And always_set_f_scsi is true only for legacy machine types? s/legacy/older/ :) It's also ignored for modern devices. So perhaps you could name it to transitional_force_f_scsi or something like that. Paolo
Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device
On Wed, Jul 15, 2015 at 01:46:38PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 13:59:00 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Jul 14, 2015 at 07:43:44PM +0200, Cornelia Huck wrote: Yes, and that's because as written, transitional devices must set ANY_LAYOUT, but that's incompatible with scsi. Hm, I had a patch before that dynamically allowed different feature sets for legacy or modern, not only a subset. Probably won't apply anymore, but I'd like to able to do the following: - driver reads features without negotiating a revision: driver is legacy, offer legacy bits - driver negotiates revision 0: dito - driver negotiates revision = 1: driver is modern, offer modern bits That way we could offer SCSI and !ANY_LAYOUT (if scsi is enabled) in the first two cases, and a new qemu could still offer scsi to old guests. Would it be worth pursuing that idea? Frankly, I don't think so: I don't see why it makes sense to expose more features on the legacy interface than on the modern one. Imagine updating drivers to fix a bug and losing some features. How does this make sense? I don't think one should be a strict subset of the other. But I think we don't want to withdraw features from legacy guests on qemu updates either? Absolutely. For now one has to enable the modern interface explicitly. Around 2.5 we might switch that around, we'll need to think hard about compatibility at that point. In any case, we must definitely keep the old capability for old machine types. I think the virtio TC's assumption was that the scsi passthrough was a bad idea, so in QEMU we only keep it around for legacy devices to avoid regressions. I'm not opposing this :) If you disagree and think transitional devices need the SCSI feature, either try to convince pbonzini or rewrite the spec youself to support it in the virtio 1 mode. This seems to boil down to the different meaning of transitional for ccw and pci, see the other thread. Before the revision is negotiated, ccw won't know whether it's a legacy driver - is that the difference? Fine, but revision is negotiated way before features are probed so why does it make a practical difference? -- MST
Re: [Qemu-block] [PATCH V2 3/5] virtio-blk: disable scsi passthrough by default
On 15/07/2015 14:21, Michael S. Tsirkin wrote: Disable scsi passthrough by default since it was incompatible with virtio 1.0. For legacy machine types, keep this on by default. Cc: Stefan Hajnoczi stefa...@redhat.com Cc: Kevin Wolf kw...@redhat.com Cc: qemu-block@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com Seems risky for 2.4. modern is off by default for now. Can't we limit the change to when modern is enabled? That would have the effect of disabling a feature when you turn on modern. I suggested changing this from bool to on/off/auto, and make auto mean !modern. No, please do it like Jason did. The SCSI feature effectively had to be enabled explicitly already, the requests were marked as unsupported. Paolo
Re: [Qemu-block] [PATCH V2 3/5] virtio-blk: disable scsi passthrough by default
On Wed, Jul 15, 2015 at 01:29:59PM +0800, Jason Wang wrote:
Disable scsi passthrough by default since it was incompatible with
virtio 1.0. For legacy machine types, keep this on by default.
Cc: Stefan Hajnoczi stefa...@redhat.com
Cc: Kevin Wolf kw...@redhat.com
Cc: qemu-block@nongnu.org
Signed-off-by: Jason Wang jasow...@redhat.com
Seems risky for 2.4. modern is off by default for now. Can't we limit
the change to when modern is enabled?
I suggested changing this from bool to on/off/auto, and
make auto mean !modern.
---
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 761d763..362fe53 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -964,7 +964,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 4a43466..56039d8 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,11 @@
#define HW_COMPAT_H
#define HW_COMPAT_2_3 \
-/* empty */
+{\
+.driver = virtio-blk-pci,\
+.property = scsi,\
+.value= on,\
+},
#define HW_COMPAT_2_2 \
/* empty */
--
2.1.4
Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device
On Wed, 15 Jul 2015 15:01:01 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jul 15, 2015 at 01:46:38PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 13:59:00 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Jul 14, 2015 at 07:43:44PM +0200, Cornelia Huck wrote: Yes, and that's because as written, transitional devices must set ANY_LAYOUT, but that's incompatible with scsi. Hm, I had a patch before that dynamically allowed different feature sets for legacy or modern, not only a subset. Probably won't apply anymore, but I'd like to able to do the following: - driver reads features without negotiating a revision: driver is legacy, offer legacy bits - driver negotiates revision 0: dito - driver negotiates revision = 1: driver is modern, offer modern bits That way we could offer SCSI and !ANY_LAYOUT (if scsi is enabled) in the first two cases, and a new qemu could still offer scsi to old guests. Would it be worth pursuing that idea? Frankly, I don't think so: I don't see why it makes sense to expose more features on the legacy interface than on the modern one. Imagine updating drivers to fix a bug and losing some features. How does this make sense? I don't think one should be a strict subset of the other. But I think we don't want to withdraw features from legacy guests on qemu updates either? Absolutely. For now one has to enable the modern interface explicitly. Around 2.5 we might switch that around, we'll need to think hard about compatibility at that point. In any case, we must definitely keep the old capability for old machine types. ccw only offers revision 0 (legacy) in 2.4. I plan to introduce revision 1 in 2.5 and force revision to 0 for 2.4 compatibility (as 2.4 is the first versioned ccw machine). I think the virtio TC's assumption was that the scsi passthrough was a bad idea, so in QEMU we only keep it around for legacy devices to avoid regressions. I'm not opposing this :) If you disagree and think transitional devices need the SCSI feature, either try to convince pbonzini or rewrite the spec youself to support it in the virtio 1 mode. This seems to boil down to the different meaning of transitional for ccw and pci, see the other thread. Before the revision is negotiated, ccw won't know whether it's a legacy driver - is that the difference? I'd say it doesn't know whether the driver intends to use the modern interface. Fine, but revision is negotiated way before features are probed so why does it make a practical difference? Legacy drivers (that don't know about the set-revision command) will read features without revision negotiation - we need to offer them the legacy feature set.
Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device
On Wed, Jul 15, 2015 at 05:38:53PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 17:39:18 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jul 15, 2015 at 04:30:51PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 17:11:57 +0300 Michael S. Tsirkin m...@redhat.com wrote: Fine, but revision is negotiated way before features are probed so why does it make a practical difference? Legacy drivers (that don't know about the set-revision command) will read features without revision negotiation - we need to offer them the legacy feature set. Right. So simply do if (revision 1) return features 0x and that will do this, will it not? Not for bits that we want to offer for legacy but not for modern. I don't think this selective offering works at least for scsi. scsi is a backend feature, if you connect a modern device in front the device simply does not work. It therefore makes no sense to attach a transitional device to such a backend. My point is that we're losing legacy features with that approach, and it would not be possible to offer them to legacy guests with newer qemus (at least with ccw). What's wrong with adding a disable-modern flag, like pci has? User can set that to get a legacy device. The whole idea behind the revision-stuff was that we don't need something like disable-modern. If the device is able to figure out on its own if it is to act as a modern or a legacy device, why require user intervention? It's about compatibility, e.g. being able to test legacy mode in transitional drivers in guests. Consider also bugs, e.g. the fact that linux guests lack WCE in modern mode ATM. What about the other way around (i.e. scsi is configured, therefore the device is legacy-only)? We'd only retain the scsi bit if it is actually wanted by the user's configuration. I would need to enforce a max revision of 0 for such a device in ccw, and pci could disable modern for it. Will have to think about it. But I think a flag to disable/enable modern is useful in any case, and it seems sufficient. I don't like the idea of disabling modern or legacy for ccw, where the differences between both are very minor. I also don't think requiring the user to specify a new flag on upgrade just to present the same features as before is a good idea: it is something that is easily missed and may lead to much headscratching. And doing this on a driver upgrade won't? As I said, if you believe this feature has value, argue that we shouldn't drop scsi off in virtio 1.0 then. -- MST
Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device
On Wed, Jul 15, 2015 at 03:40:22PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 16:16:07 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jul 15, 2015 at 02:43:51PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 15:01:01 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jul 15, 2015 at 01:46:38PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 13:59:00 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Jul 14, 2015 at 07:43:44PM +0200, Cornelia Huck wrote: Yes, and that's because as written, transitional devices must set ANY_LAYOUT, but that's incompatible with scsi. Hm, I had a patch before that dynamically allowed different feature sets for legacy or modern, not only a subset. Probably won't apply anymore, but I'd like to able to do the following: - driver reads features without negotiating a revision: driver is legacy, offer legacy bits - driver negotiates revision 0: dito - driver negotiates revision = 1: driver is modern, offer modern bits That way we could offer SCSI and !ANY_LAYOUT (if scsi is enabled) in the first two cases, and a new qemu could still offer scsi to old guests. Would it be worth pursuing that idea? Frankly, I don't think so: I don't see why it makes sense to expose more features on the legacy interface than on the modern one. Imagine updating drivers to fix a bug and losing some features. How does this make sense? I don't think one should be a strict subset of the other. But I think we don't want to withdraw features from legacy guests on qemu updates either? Absolutely. For now one has to enable the modern interface explicitly. Around 2.5 we might switch that around, we'll need to think hard about compatibility at that point. In any case, we must definitely keep the old capability for old machine types. ccw only offers revision 0 (legacy) in 2.4. I plan to introduce revision 1 in 2.5 and force revision to 0 for 2.4 compatibility (as 2.4 is the first versioned ccw machine). I was talking about pci here actually. Sure, and these are my plans for ccw ;) I think the virtio TC's assumption was that the scsi passthrough was a bad idea, so in QEMU we only keep it around for legacy devices to avoid regressions. I'm not opposing this :) If you disagree and think transitional devices need the SCSI feature, either try to convince pbonzini or rewrite the spec youself to support it in the virtio 1 mode. This seems to boil down to the different meaning of transitional for ccw and pci, see the other thread. Before the revision is negotiated, ccw won't know whether it's a legacy driver - is that the difference? I'd say it doesn't know whether the driver intends to use the modern interface. That's also the case for pci. But does pci know the moment it first tries to get the device's features? And does pci assume modern as default for transitional devices? I don't think it does. Fine, but revision is negotiated way before features are probed so why does it make a practical difference? Legacy drivers (that don't know about the set-revision command) will read features without revision negotiation - we need to offer them the legacy feature set. Right. So simply do if (revision 1) return features 0x and that will do this, will it not? Not for bits that we want to offer for legacy but not for modern. I don't think this selective offering works at least for scsi. scsi is a backend feature, if you connect a modern device in front the device simply does not work. It therefore makes no sense to attach a transitional device to such a backend. -- MST
Re: [Qemu-block] [PATCH V2 3/5] virtio-blk: disable scsi passthrough by default
On Wed, Jul 15, 2015 at 02:47:24PM +0200, Paolo Bonzini wrote: On 15/07/2015 14:21, Michael S. Tsirkin wrote: Disable scsi passthrough by default since it was incompatible with virtio 1.0. For legacy machine types, keep this on by default. Cc: Stefan Hajnoczi stefa...@redhat.com Cc: Kevin Wolf kw...@redhat.com Cc: qemu-block@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com Seems risky for 2.4. modern is off by default for now. Can't we limit the change to when modern is enabled? That would have the effect of disabling a feature when you turn on modern. What's wrong with that? I suggested changing this from bool to on/off/auto, and make auto mean !modern. No, please do it like Jason did. The SCSI feature effectively had to be enabled explicitly already, the requests were marked as unsupported. Paolo I didn't know. How is it enabled? -- MST
Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device
On Wed, 15 Jul 2015 17:11:57 +0300 Michael S. Tsirkin m...@redhat.com wrote: Fine, but revision is negotiated way before features are probed so why does it make a practical difference? Legacy drivers (that don't know about the set-revision command) will read features without revision negotiation - we need to offer them the legacy feature set. Right. So simply do if (revision 1) return features 0x and that will do this, will it not? Not for bits that we want to offer for legacy but not for modern. I don't think this selective offering works at least for scsi. scsi is a backend feature, if you connect a modern device in front the device simply does not work. It therefore makes no sense to attach a transitional device to such a backend. My point is that we're losing legacy features with that approach, and it would not be possible to offer them to legacy guests with newer qemus (at least with ccw). What about the other way around (i.e. scsi is configured, therefore the device is legacy-only)? We'd only retain the scsi bit if it is actually wanted by the user's configuration. I would need to enforce a max revision of 0 for such a device in ccw, and pci could disable modern for it.
Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device
On Wed, Jul 15, 2015 at 04:30:51PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 17:11:57 +0300 Michael S. Tsirkin m...@redhat.com wrote: Fine, but revision is negotiated way before features are probed so why does it make a practical difference? Legacy drivers (that don't know about the set-revision command) will read features without revision negotiation - we need to offer them the legacy feature set. Right. So simply do if (revision 1) return features 0x and that will do this, will it not? Not for bits that we want to offer for legacy but not for modern. I don't think this selective offering works at least for scsi. scsi is a backend feature, if you connect a modern device in front the device simply does not work. It therefore makes no sense to attach a transitional device to such a backend. My point is that we're losing legacy features with that approach, and it would not be possible to offer them to legacy guests with newer qemus (at least with ccw). What's wrong with adding a disable-modern flag, like pci has? User can set that to get a legacy device. What about the other way around (i.e. scsi is configured, therefore the device is legacy-only)? We'd only retain the scsi bit if it is actually wanted by the user's configuration. I would need to enforce a max revision of 0 for such a device in ccw, and pci could disable modern for it. Will have to think about it. But I think a flag to disable/enable modern is useful in any case, and it seems sufficient. -- MST
Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device
On Wed, 15 Jul 2015 16:16:07 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jul 15, 2015 at 02:43:51PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 15:01:01 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jul 15, 2015 at 01:46:38PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 13:59:00 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Jul 14, 2015 at 07:43:44PM +0200, Cornelia Huck wrote: Yes, and that's because as written, transitional devices must set ANY_LAYOUT, but that's incompatible with scsi. Hm, I had a patch before that dynamically allowed different feature sets for legacy or modern, not only a subset. Probably won't apply anymore, but I'd like to able to do the following: - driver reads features without negotiating a revision: driver is legacy, offer legacy bits - driver negotiates revision 0: dito - driver negotiates revision = 1: driver is modern, offer modern bits That way we could offer SCSI and !ANY_LAYOUT (if scsi is enabled) in the first two cases, and a new qemu could still offer scsi to old guests. Would it be worth pursuing that idea? Frankly, I don't think so: I don't see why it makes sense to expose more features on the legacy interface than on the modern one. Imagine updating drivers to fix a bug and losing some features. How does this make sense? I don't think one should be a strict subset of the other. But I think we don't want to withdraw features from legacy guests on qemu updates either? Absolutely. For now one has to enable the modern interface explicitly. Around 2.5 we might switch that around, we'll need to think hard about compatibility at that point. In any case, we must definitely keep the old capability for old machine types. ccw only offers revision 0 (legacy) in 2.4. I plan to introduce revision 1 in 2.5 and force revision to 0 for 2.4 compatibility (as 2.4 is the first versioned ccw machine). I was talking about pci here actually. Sure, and these are my plans for ccw ;) I think the virtio TC's assumption was that the scsi passthrough was a bad idea, so in QEMU we only keep it around for legacy devices to avoid regressions. I'm not opposing this :) If you disagree and think transitional devices need the SCSI feature, either try to convince pbonzini or rewrite the spec youself to support it in the virtio 1 mode. This seems to boil down to the different meaning of transitional for ccw and pci, see the other thread. Before the revision is negotiated, ccw won't know whether it's a legacy driver - is that the difference? I'd say it doesn't know whether the driver intends to use the modern interface. That's also the case for pci. But does pci know the moment it first tries to get the device's features? And does pci assume modern as default for transitional devices? Fine, but revision is negotiated way before features are probed so why does it make a practical difference? Legacy drivers (that don't know about the set-revision command) will read features without revision negotiation - we need to offer them the legacy feature set. Right. So simply do if (revision 1) return features 0x and that will do this, will it not? Not for bits that we want to offer for legacy but not for modern.
Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device
On Wed, Jul 15, 2015 at 02:43:51PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 15:01:01 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jul 15, 2015 at 01:46:38PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 13:59:00 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Jul 14, 2015 at 07:43:44PM +0200, Cornelia Huck wrote: Yes, and that's because as written, transitional devices must set ANY_LAYOUT, but that's incompatible with scsi. Hm, I had a patch before that dynamically allowed different feature sets for legacy or modern, not only a subset. Probably won't apply anymore, but I'd like to able to do the following: - driver reads features without negotiating a revision: driver is legacy, offer legacy bits - driver negotiates revision 0: dito - driver negotiates revision = 1: driver is modern, offer modern bits That way we could offer SCSI and !ANY_LAYOUT (if scsi is enabled) in the first two cases, and a new qemu could still offer scsi to old guests. Would it be worth pursuing that idea? Frankly, I don't think so: I don't see why it makes sense to expose more features on the legacy interface than on the modern one. Imagine updating drivers to fix a bug and losing some features. How does this make sense? I don't think one should be a strict subset of the other. But I think we don't want to withdraw features from legacy guests on qemu updates either? Absolutely. For now one has to enable the modern interface explicitly. Around 2.5 we might switch that around, we'll need to think hard about compatibility at that point. In any case, we must definitely keep the old capability for old machine types. ccw only offers revision 0 (legacy) in 2.4. I plan to introduce revision 1 in 2.5 and force revision to 0 for 2.4 compatibility (as 2.4 is the first versioned ccw machine). I was talking about pci here actually. I think the virtio TC's assumption was that the scsi passthrough was a bad idea, so in QEMU we only keep it around for legacy devices to avoid regressions. I'm not opposing this :) If you disagree and think transitional devices need the SCSI feature, either try to convince pbonzini or rewrite the spec youself to support it in the virtio 1 mode. This seems to boil down to the different meaning of transitional for ccw and pci, see the other thread. Before the revision is negotiated, ccw won't know whether it's a legacy driver - is that the difference? I'd say it doesn't know whether the driver intends to use the modern interface. That's also the case for pci. Fine, but revision is negotiated way before features are probed so why does it make a practical difference? Legacy drivers (that don't know about the set-revision command) will read features without revision negotiation - we need to offer them the legacy feature set. Right. So simply do if (revision 1) return features 0x and that will do this, will it not? -- MST
Re: [Qemu-block] [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device
On Wed, 15 Jul 2015 17:39:18 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jul 15, 2015 at 04:30:51PM +0200, Cornelia Huck wrote: On Wed, 15 Jul 2015 17:11:57 +0300 Michael S. Tsirkin m...@redhat.com wrote: Fine, but revision is negotiated way before features are probed so why does it make a practical difference? Legacy drivers (that don't know about the set-revision command) will read features without revision negotiation - we need to offer them the legacy feature set. Right. So simply do if (revision 1) return features 0x and that will do this, will it not? Not for bits that we want to offer for legacy but not for modern. I don't think this selective offering works at least for scsi. scsi is a backend feature, if you connect a modern device in front the device simply does not work. It therefore makes no sense to attach a transitional device to such a backend. My point is that we're losing legacy features with that approach, and it would not be possible to offer them to legacy guests with newer qemus (at least with ccw). What's wrong with adding a disable-modern flag, like pci has? User can set that to get a legacy device. The whole idea behind the revision-stuff was that we don't need something like disable-modern. If the device is able to figure out on its own if it is to act as a modern or a legacy device, why require user intervention? What about the other way around (i.e. scsi is configured, therefore the device is legacy-only)? We'd only retain the scsi bit if it is actually wanted by the user's configuration. I would need to enforce a max revision of 0 for such a device in ccw, and pci could disable modern for it. Will have to think about it. But I think a flag to disable/enable modern is useful in any case, and it seems sufficient. I don't like the idea of disabling modern or legacy for ccw, where the differences between both are very minor. I also don't think requiring the user to specify a new flag on upgrade just to present the same features as before is a good idea: it is something that is easily missed and may lead to much headscratching.
Re: [Qemu-block] [PATCH V2 3/5] virtio-blk: disable scsi passthrough by default
On 15/07/2015 16:28, Michael S. Tsirkin wrote: On Wed, Jul 15, 2015 at 04:18:49PM +0200, Paolo Bonzini wrote: On 15/07/2015 16:14, Michael S. Tsirkin wrote: On Wed, Jul 15, 2015 at 02:47:24PM +0200, Paolo Bonzini wrote: On 15/07/2015 14:21, Michael S. Tsirkin wrote: Disable scsi passthrough by default since it was incompatible with virtio 1.0. For legacy machine types, keep this on by default. Cc: Stefan Hajnoczi stefa...@redhat.com Cc: Kevin Wolf kw...@redhat.com Cc: qemu-block@nongnu.org Signed-off-by: Jason Wang jasow...@redhat.com Seems risky for 2.4. modern is off by default for now. Can't we limit the change to when modern is enabled? That would have the effect of disabling a feature when you turn on modern. What's wrong with that? Weren't you complaining about it a few hours ago? :) No, I complained about guest driver update disabling it. Ah, sorry for the confusion. I suggested changing this from bool to on/off/auto, and make auto mean !modern. No, please do it like Jason did. The SCSI feature effectively had to be enabled explicitly already, the requests were marked as unsupported. I didn't know. How is it enabled? It's enabled by default in QEMU, but disabled by default in libvirt. And it only works if you pass a whole _disk_ (not a partition or logical volume) to QEMU, which is definitely not the common case. It can just be documented in the release notes; the feature is still available, and libvirt won't be broken because it adds explicitly both scsi=on and scsi=off. So for libvirt, we don't really care about the default, right? For command line, would it not be friendlier to make it follow the modern flag automatically? I wouldn't mind if we grabbed the occasion to disable it altogether. However, that would indeed work as well. Paolo
