| Message ID | 1437990561-22134-4-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 27/07/2015 11:49, Jason Wang wrote: > So this patch only clear VIRTIO_F_LAYOUT for legacy device. > > Cc: Stefan Hajnoczi <stefanha@redhat.com> > Cc: Kevin Wolf <kwolf@redhat.com> > Cc: qemu-block@nongnu.org > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/block/virtio-blk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 9acbc3a..1d3f26c 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -731,7 +731,6 @@ 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_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); > if (__virtio_has_feature(features, VIRTIO_F_VERSION_1)) { > if (s->conf.scsi) { > error_setg(errp, "Virtio 1.0 does not support scsi passthrough!"); > @@ -739,6 +738,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, > } > virtio_add_feature(&features, VIRTIO_F_ANY_LAYOUT); > } else { > + virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); > virtio_add_feature(&features, VIRTIO_BLK_F_SCSI); > } This patch is unnecessary, since the feature is added back below under "if (__virtio_has_feature(features, VIRTIO_F_VERSION_1))". Paolo
On Mon, Jul 27, 2015 at 12:30:19PM +0200, Paolo Bonzini wrote: > > > On 27/07/2015 11:49, Jason Wang wrote: > > So this patch only clear VIRTIO_F_LAYOUT for legacy device. > > > > Cc: Stefan Hajnoczi <stefanha@redhat.com> > > Cc: Kevin Wolf <kwolf@redhat.com> > > Cc: qemu-block@nongnu.org > > Signed-off-by: Jason Wang <jasowang@redhat.com> > > --- > > hw/block/virtio-blk.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > > index 9acbc3a..1d3f26c 100644 > > --- a/hw/block/virtio-blk.c > > +++ b/hw/block/virtio-blk.c > > @@ -731,7 +731,6 @@ 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_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); > > if (__virtio_has_feature(features, VIRTIO_F_VERSION_1)) { > > if (s->conf.scsi) { > > error_setg(errp, "Virtio 1.0 does not support scsi passthrough!"); > > @@ -739,6 +738,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, > > } > > virtio_add_feature(&features, VIRTIO_F_ANY_LAYOUT); > > } else { > > + virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); > > virtio_add_feature(&features, VIRTIO_BLK_F_SCSI); > > } > > This patch is unnecessary, since the feature is added back below under > "if (__virtio_has_feature(features, VIRTIO_F_VERSION_1))". > > Paolo It's needed so we can apply virtio: set any_layout in virtio core
On 27/07/2015 13:22, Michael S. Tsirkin wrote: > > This patch is unnecessary, since the feature is added back below under > > "if (__virtio_has_feature(features, VIRTIO_F_VERSION_1))". > > It's needed so we can apply > virtio: set any_layout in virtio core Ah, okay. Paolo
On Mon, 27 Jul 2015 14:22:37 +0300 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Mon, Jul 27, 2015 at 12:30:19PM +0200, Paolo Bonzini wrote: > > > > > > On 27/07/2015 11:49, Jason Wang wrote: > > > So this patch only clear VIRTIO_F_LAYOUT for legacy device. > > > > > > Cc: Stefan Hajnoczi <stefanha@redhat.com> > > > Cc: Kevin Wolf <kwolf@redhat.com> > > > Cc: qemu-block@nongnu.org > > > Signed-off-by: Jason Wang <jasowang@redhat.com> > > > --- > > > hw/block/virtio-blk.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > > > index 9acbc3a..1d3f26c 100644 > > > --- a/hw/block/virtio-blk.c > > > +++ b/hw/block/virtio-blk.c > > > @@ -731,7 +731,6 @@ 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_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); > > > if (__virtio_has_feature(features, VIRTIO_F_VERSION_1)) { > > > if (s->conf.scsi) { > > > error_setg(errp, "Virtio 1.0 does not support scsi passthrough!"); > > > @@ -739,6 +738,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, > > > } > > > virtio_add_feature(&features, VIRTIO_F_ANY_LAYOUT); > > > } else { > > > + virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); > > > virtio_add_feature(&features, VIRTIO_BLK_F_SCSI); > > > } > > > > This patch is unnecessary, since the feature is added back below under > > "if (__virtio_has_feature(features, VIRTIO_F_VERSION_1))". > > > > Paolo > > It's needed so we can apply > virtio: set any_layout in virtio core So what's the plan on all those virtio feature patches? It's hard to keep track about what is based upon what, and what the end result looks like. I don't have a good feeling about doing this that late in the 2.4 cycle.
On Mon, Jul 27, 2015 at 03:28:51PM +0200, Cornelia Huck wrote: > On Mon, 27 Jul 2015 14:22:37 +0300 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Mon, Jul 27, 2015 at 12:30:19PM +0200, Paolo Bonzini wrote: > > > > > > > > > On 27/07/2015 11:49, Jason Wang wrote: > > > > So this patch only clear VIRTIO_F_LAYOUT for legacy device. > > > > > > > > Cc: Stefan Hajnoczi <stefanha@redhat.com> > > > > Cc: Kevin Wolf <kwolf@redhat.com> > > > > Cc: qemu-block@nongnu.org > > > > Signed-off-by: Jason Wang <jasowang@redhat.com> > > > > --- > > > > hw/block/virtio-blk.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > > > > index 9acbc3a..1d3f26c 100644 > > > > --- a/hw/block/virtio-blk.c > > > > +++ b/hw/block/virtio-blk.c > > > > @@ -731,7 +731,6 @@ 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_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); > > > > if (__virtio_has_feature(features, VIRTIO_F_VERSION_1)) { > > > > if (s->conf.scsi) { > > > > error_setg(errp, "Virtio 1.0 does not support scsi passthrough!"); > > > > @@ -739,6 +738,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, > > > > } > > > > virtio_add_feature(&features, VIRTIO_F_ANY_LAYOUT); > > > > } else { > > > > + virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); > > > > virtio_add_feature(&features, VIRTIO_BLK_F_SCSI); > > > > } > > > > > > This patch is unnecessary, since the feature is added back below under > > > "if (__virtio_has_feature(features, VIRTIO_F_VERSION_1))". > > > > > > Paolo > > > > It's needed so we can apply > > virtio: set any_layout in virtio core > > So what's the plan on all those virtio feature patches? It's hard to > keep track about what is based upon what, and what the end result looks > like. I pushed everything out to my pci branch, pls take a look. This is what I have: b787b35 acpi: fix pvpanic device is not shown in ui 49009db hw/acpi/ich9: clean up stale comment about KVM not supporting SMM e513e9c hw/acpi/ich9: clear smi_en on reset c9b11f9 virtio-blk: only clear VIRTIO_F_ANY_LAYOUT for legacy device efb8206 virtio-blk: fail get_features when both scsi and 1.0 were set 9d5b731 virtio: get_features() can fail 2746269 virtio-pci: fix memory MR cleanup for modern 09999a5 virtio: set any_layout in virtio core cd4bfbb virtio-9p: fix any_layout 7882080 virtio-serial: fix ANY_LAYOUT 5f45607 virtio: hide legacy features from modern guests > I don't have a good feeling about doing this that late in the 2.4 > cycle. Well there will always be bugs. Given modern is disabled by default, even if more bugs surface after release it's not the end of the world.
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 9acbc3a..1d3f26c 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -731,7 +731,6 @@ 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_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); if (__virtio_has_feature(features, VIRTIO_F_VERSION_1)) { if (s->conf.scsi) { error_setg(errp, "Virtio 1.0 does not support scsi passthrough!"); @@ -739,6 +738,7 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features, } virtio_add_feature(&features, VIRTIO_F_ANY_LAYOUT); } else { + virtio_clear_feature(&features, VIRTIO_F_ANY_LAYOUT); virtio_add_feature(&features, VIRTIO_BLK_F_SCSI); }
Chapter 6.3 of spec said " Transitional devices MUST offer, and if offered by the device transitional drivers MUST accept the following: VIRTIO_F_ANY_LAYOUT (27) " So this patch only clear VIRTIO_F_LAYOUT for legacy device. Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Kevin Wolf <kwolf@redhat.com> Cc: qemu-block@nongnu.org Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/block/virtio-blk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)