Re: [PATCH v8 34/50] vhost/net: virtio 1.0 byte swap

2015-01-07 Thread Michael S. Tsirkin
On Wed, Jan 07, 2015 at 09:31:05AM +0100, Greg Kurz wrote:
 On Tue, 06 Jan 2015 16:55:30 -0700
 Alex Williamson alex.william...@redhat.com wrote:
 
  On Mon, 2014-12-01 at 18:05 +0200, Michael S. Tsirkin wrote:
   I had to add an explicit tag to suppress compiler warning:
   gcc isn't smart enough to notice that
   len is always initialized since function is called with size  0.
  
  I'm getting a panic inside a guest when this change is applied on the
  host.  I identified this patch via bisect and confirmed by reverting it
  from v3.19-rc2.  Guest is centos6.  Thanks,
  
  Alex
  
  commit 8b38694a2dc8b18374310df50174f1e4376d6824
  Author: Michael S. Tsirkin m...@redhat.com
  Date:   Fri Oct 24 14:19:48 2014 +0300
  
  vhost/net: virtio 1.0 byte swap
  
  I had to add an explicit tag to suppress compiler warning:
  gcc isn't smart enough to notice that
  len is always initialized since function is called with size  0.
  
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
  
 
 This chunk looks suspicious:
 
 - heads[headcount - 1].len += datalen;
 + heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
 
 s/len - datalen/len + datalen/ ?

Indeed!
I just sent a patch fixing this, thanks a lot.


  XML chunk:
  
  interface type='direct'
mac address='52:54:00:64:f3:34'/
source dev='iscsinet0' mode='bridge'/
model type='virtio'/
address type='pci' domain='0x' bus='0x00' slot='0x03' 
  function='0x0'/
  /interface
  
  Panic log:
  
  1BUG: unable to handle kernel NULL pointer dereference at 0010
  1IP: [a0079469] virtnet_poll+0x4f9/0x910 [virtio_net]
  4PGD 1aa2f4067 PUD 1aa2f5067 PMD 0 
  4Oops:  [#1] SMP 
  4last sysfs file: 
  /sys/devices/pci:00/:00:03.0/virtio0/net/eth9/ifindex
  4CPU 0 
  4Modules linked in: 8021q garp stp llc ipt_REJECT nf_conntrack_ipv4 
  nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 
  nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 uinput 
  microcode snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep 
  snd_seq snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc igbvf 
  nvidia(P)(U) i2c_core tg3 ptp pps_core virtio_balloon virtio_net 
  virtio_console ext4 jbd2 mbcache virtio_blk virtio_pci virtio_ring virtio 
  pata_acpi ata_generic ata_piix dm_mirror dm_region_hash dm_log dm_mod [last 
  unloaded: speedstep_lib]
  4
  4Pid: 1374, comm: NetworkManager Tainted: P   ---
  2.6.32-431.23.3.el6.centos.plus.x86_64 #1 QEMU Standard PC (i440FX + PIIX, 
  1996)
  4RIP: 0010:[a0079469]  [a0079469] 
  virtnet_poll+0x4f9/0x910 [virtio_net]
  4RSP: 0018:880028203e48  EFLAGS: 00010246
  4RAX: 8801a3383d00 RBX: 8801a6aaf480 RCX: 8801aa20b6e0
  4RDX: 00c0 RSI: 8801a3383c00 RDI: 8801a3383cc0
  4RBP: 880028203ed8 R08: 009e R09: 8801aa1d800c
  4R10: 0218 R11:  R12: 8801aa20b6e0
  4R13:  R14:  R15: 
  4FS:  7febf114d800() GS:88002820() 
  knlGS:
  4CS:  0010 DS:  ES:  CR0: 80050033
  4CR2: 0010 CR3: 0001aa793000 CR4: 06f0
  4DR0:  DR1:  DR2: 
  4DR3:  DR6: 0ff0 DR7: 0400
  4Process NetworkManager (pid: 1374, threadinfo 8801a74ba000, task 
  8801a8d56040)
  4Stack:
  4 8801aa1d8000 009e 8801aa20b6e0 8801aa20b718
  4d 8801aa20b780 8801aa1d800c 8801a6aaf4b8 8801aa20b020
  4d 0080 8801aa20b708 0001 1f5981a830c8
  4Call Trace:
  4 IRQ 
  4 [8146ae33] net_rx_action+0x103/0x2f0
  4 [8107a5f1] __do_softirq+0xc1/0x1e0
  4 [8100c30c] ? call_softirq+0x1c/0x30
  4 [8100c30c] call_softirq+0x1c/0x30
  4 EOI 
  4 [8100fa75] ? do_softirq+0x65/0xa0
  4 [8107b2ea] local_bh_enable+0x9a/0xb0
  4 [a007813a] virtnet_napi_enable+0x4a/0x60 [virtio_net]
  4 [a0078ebf] virtnet_open+0x4f/0x60 [virtio_net]
  4 [81467691] dev_open+0xa1/0x100
  4 [81466751] dev_change_flags+0xa1/0x1d0
  4 [81474a59] do_setlink+0x169/0x8b0
  4 [814770b6] ? rtnl_fill_ifinfo+0x946/0xcb0
  4 [812a3d24] ? nla_parse+0x34/0x110
  4 [8147659e] rtnl_setlink+0xee/0x130
  4 [81475b67] rtnetlink_rcv_msg+0x2d7/0x340
  4 [81231e14] ? socket_has_perm+0x74/0x90
  4 [81475890] ? rtnetlink_rcv_msg+0x0/0x340
  4 [814910a9] netlink_rcv_skb+0xa9/0xd0
  4 [81475875] rtnetlink_rcv+0x25/0x40
  4 [81490cdb] netlink_unicast+0x2db/0x320
  4 [81491750] netlink_sendmsg+0x2c0/0x3d0
  4 [814520c3] sock_sendmsg+0x123/0x150
  4 [81453d73] 

Re: [PATCH v8 34/50] vhost/net: virtio 1.0 byte swap

2015-01-07 Thread Greg Kurz
On Tue, 06 Jan 2015 16:55:30 -0700
Alex Williamson alex.william...@redhat.com wrote:

 On Mon, 2014-12-01 at 18:05 +0200, Michael S. Tsirkin wrote:
  I had to add an explicit tag to suppress compiler warning:
  gcc isn't smart enough to notice that
  len is always initialized since function is called with size  0.
 
 I'm getting a panic inside a guest when this change is applied on the
 host.  I identified this patch via bisect and confirmed by reverting it
 from v3.19-rc2.  Guest is centos6.  Thanks,
 
 Alex
 
 commit 8b38694a2dc8b18374310df50174f1e4376d6824
 Author: Michael S. Tsirkin m...@redhat.com
 Date:   Fri Oct 24 14:19:48 2014 +0300
 
 vhost/net: virtio 1.0 byte swap
 
 I had to add an explicit tag to suppress compiler warning:
 gcc isn't smart enough to notice that
 len is always initialized since function is called with size  0.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com
 

This chunk looks suspicious:

-   heads[headcount - 1].len += datalen;
+   heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);

s/len - datalen/len + datalen/ ?

 XML chunk:
 
 interface type='direct'
   mac address='52:54:00:64:f3:34'/
   source dev='iscsinet0' mode='bridge'/
   model type='virtio'/
   address type='pci' domain='0x' bus='0x00' slot='0x03' 
 function='0x0'/
 /interface
 
 Panic log:
 
 1BUG: unable to handle kernel NULL pointer dereference at 0010
 1IP: [a0079469] virtnet_poll+0x4f9/0x910 [virtio_net]
 4PGD 1aa2f4067 PUD 1aa2f5067 PMD 0 
 4Oops:  [#1] SMP 
 4last sysfs file: 
 /sys/devices/pci:00/:00:03.0/virtio0/net/eth9/ifindex
 4CPU 0 
 4Modules linked in: 8021q garp stp llc ipt_REJECT nf_conntrack_ipv4 
 nf_defrag_ipv4 iptable_filter ip_tables ip6t_REJECT nf_conntrack_ipv6 
 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables ipv6 uinput 
 microcode snd_hda_codec_realtek snd_hda_intel snd_hda_codec snd_hwdep snd_seq 
 snd_seq_device snd_pcm snd_timer snd soundcore snd_page_alloc igbvf 
 nvidia(P)(U) i2c_core tg3 ptp pps_core virtio_balloon virtio_net 
 virtio_console ext4 jbd2 mbcache virtio_blk virtio_pci virtio_ring virtio 
 pata_acpi ata_generic ata_piix dm_mirror dm_region_hash dm_log dm_mod [last 
 unloaded: speedstep_lib]
 4
 4Pid: 1374, comm: NetworkManager Tainted: P   ---
 2.6.32-431.23.3.el6.centos.plus.x86_64 #1 QEMU Standard PC (i440FX + PIIX, 
 1996)
 4RIP: 0010:[a0079469]  [a0079469] 
 virtnet_poll+0x4f9/0x910 [virtio_net]
 4RSP: 0018:880028203e48  EFLAGS: 00010246
 4RAX: 8801a3383d00 RBX: 8801a6aaf480 RCX: 8801aa20b6e0
 4RDX: 00c0 RSI: 8801a3383c00 RDI: 8801a3383cc0
 4RBP: 880028203ed8 R08: 009e R09: 8801aa1d800c
 4R10: 0218 R11:  R12: 8801aa20b6e0
 4R13:  R14:  R15: 
 4FS:  7febf114d800() GS:88002820() 
 knlGS:
 4CS:  0010 DS:  ES:  CR0: 80050033
 4CR2: 0010 CR3: 0001aa793000 CR4: 06f0
 4DR0:  DR1:  DR2: 
 4DR3:  DR6: 0ff0 DR7: 0400
 4Process NetworkManager (pid: 1374, threadinfo 8801a74ba000, task 
 8801a8d56040)
 4Stack:
 4 8801aa1d8000 009e 8801aa20b6e0 8801aa20b718
 4d 8801aa20b780 8801aa1d800c 8801a6aaf4b8 8801aa20b020
 4d 0080 8801aa20b708 0001 1f5981a830c8
 4Call Trace:
 4 IRQ 
 4 [8146ae33] net_rx_action+0x103/0x2f0
 4 [8107a5f1] __do_softirq+0xc1/0x1e0
 4 [8100c30c] ? call_softirq+0x1c/0x30
 4 [8100c30c] call_softirq+0x1c/0x30
 4 EOI 
 4 [8100fa75] ? do_softirq+0x65/0xa0
 4 [8107b2ea] local_bh_enable+0x9a/0xb0
 4 [a007813a] virtnet_napi_enable+0x4a/0x60 [virtio_net]
 4 [a0078ebf] virtnet_open+0x4f/0x60 [virtio_net]
 4 [81467691] dev_open+0xa1/0x100
 4 [81466751] dev_change_flags+0xa1/0x1d0
 4 [81474a59] do_setlink+0x169/0x8b0
 4 [814770b6] ? rtnl_fill_ifinfo+0x946/0xcb0
 4 [812a3d24] ? nla_parse+0x34/0x110
 4 [8147659e] rtnl_setlink+0xee/0x130
 4 [81475b67] rtnetlink_rcv_msg+0x2d7/0x340
 4 [81231e14] ? socket_has_perm+0x74/0x90
 4 [81475890] ? rtnetlink_rcv_msg+0x0/0x340
 4 [814910a9] netlink_rcv_skb+0xa9/0xd0
 4 [81475875] rtnetlink_rcv+0x25/0x40
 4 [81490cdb] netlink_unicast+0x2db/0x320
 4 [81491750] netlink_sendmsg+0x2c0/0x3d0
 4 [814520c3] sock_sendmsg+0x123/0x150
 4 [81453d73] ? sock_recvmsg+0x133/0x160
 4 [8109afa0] ? autoremove_wake_function+0x0/0x40
 4 [81136941] ? lru_cache_add_lru+0x21/0x40
 4 [8115522d] ? page_add_new_anon_rmap+0x9d/0xf0
 4 [8114aeef] ? 

[PATCH] vhost/net: length miscalculation

2015-01-07 Thread Michael S. Tsirkin
commit 8b38694a2dc8b18374310df50174f1e4376d6824
vhost/net: virtio 1.0 byte swap
had this chunk:
-   heads[headcount - 1].len += datalen;
+   heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);

This adds datalen with the wrong sign, causing guest panics.

Fixes: 8b38694a2dc8b18374310df50174f1e4376d6824
Reported-by: Alex Williamson alex.william...@redhat.com
Suggested-by: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---

Alex, could you please confirm this fixes the crash for you?

 drivers/vhost/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 14419a8..d415d69 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -538,7 +538,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
++headcount;
seg += in;
}
-   heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
+   heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
*iovcount = seg;
if (unlikely(log))
*log_num = nlogs;
-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PULL] vhost: cleanups and fixes

2015-01-07 Thread Michael S. Tsirkin
The following changes since commit b1940cd21c0f4abdce101253e860feff547291b0:

  Linux 3.19-rc3 (2015-01-05 17:05:20 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to 99975cc6ada0d5f2675e83abecae05aba5f437d2:

  vhost/net: length miscalculation (2015-01-07 12:22:00 +0200)


virtio, vhost fixes for 3.19

This fixes a couple of bugs triggered by hot-unplug
of virtio devices, as well as a regression in vhost-net.

Cc: Rusty Russell ru...@rustcorp.com.au
Signed-off-by: Michael S. Tsirkin m...@redhat.com


Michael S. Tsirkin (4):
  virtio: make del_vqs idempotent
  virtio_pci: device-specific release callback
  virtio_pci: document why we defer kfree
  vhost/net: length miscalculation

Sasha Levin (1):
  virtio_pci: defer kfree until release callback

 drivers/virtio/virtio_pci_common.h |  1 -
 drivers/vhost/net.c|  2 +-
 drivers/virtio/virtio_pci_common.c | 10 +-
 drivers/virtio/virtio_pci_legacy.c | 12 +++-
 4 files changed, 13 insertions(+), 12 deletions(-)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC v6 13/20] virtio: allow to fail setting status

2015-01-07 Thread Cornelia Huck
On Wed, 7 Jan 2015 21:08:21 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Wed, Jan 07, 2015 at 05:13:32PM +0100, Cornelia Huck wrote:
  On Tue, 30 Dec 2014 14:25:37 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Thu, Dec 11, 2014 at 02:25:15PM +0100, Cornelia Huck wrote:
virtio-1 allow setting of the FEATURES_OK status bit to fail if
the negotiated feature bits are inconsistent: let's fail
virtio_set_status() in that case and update virtio-ccw to post an
error to the guest.

Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
   
   Right but a separate validate_features call is awkward.
   How about we defer virtio_set_features until FEATURES_OK,
   and teach virtio_set_features that it can fail?
  
  Hm. But we would need to keep virtio_set_features() where it is called
  now for legacy devices, as they will never see FEATURES_OK, right?
  So
  we need to make this depending on revisions (or whatever the equivalent
  is for pci/mmio), as we cannot check for VERSION_1. Not sure whether
  this makes the code easier to follow.
 
 So let's make this a separate callback then.
 virtio_legacy_set_features?

I'm not sure I like that. We'd need to touch every transport, right?

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] vhost/net: length miscalculation

2015-01-07 Thread Sergei Shtylyov

Hello.

On 01/07/2015 11:55 AM, Michael S. Tsirkin wrote:


commit 8b38694a2dc8b18374310df50174f1e4376d6824
 vhost/net: virtio 1.0 byte swap
had this chunk:
-   heads[headcount - 1].len += datalen;
+   heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);



This adds datalen with the wrong sign, causing guest panics.



Fixes: 8b38694a2dc8b18374310df50174f1e4376d6824


   The format of this tag assumes 12-digit SHA1 hash and the commit 
description enclosed in parens and double quotes. See 
Documentation/SubmittingPatches.



Reported-by: Alex Williamson alex.william...@redhat.com
Suggested-by: Greg Kurz gk...@linux.vnet.ibm.com
Signed-off-by: Michael S. Tsirkin m...@redhat.com


WBR, Sergei

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] vhost/net: length miscalculation

2015-01-07 Thread Alex Williamson
On Wed, 2015-01-07 at 10:55 +0200, Michael S. Tsirkin wrote:
 commit 8b38694a2dc8b18374310df50174f1e4376d6824
 vhost/net: virtio 1.0 byte swap
 had this chunk:
 -   heads[headcount - 1].len += datalen;
 +   heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
 
 This adds datalen with the wrong sign, causing guest panics.
 
 Fixes: 8b38694a2dc8b18374310df50174f1e4376d6824
 Reported-by: Alex Williamson alex.william...@redhat.com
 Suggested-by: Greg Kurz gk...@linux.vnet.ibm.com
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
 
 Alex, could you please confirm this fixes the crash for you?

Confirmed, this works.  Thanks,

Alex

  drivers/vhost/net.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
 index 14419a8..d415d69 100644
 --- a/drivers/vhost/net.c
 +++ b/drivers/vhost/net.c
 @@ -538,7 +538,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
   ++headcount;
   seg += in;
   }
 - heads[headcount - 1].len = cpu_to_vhost32(vq, len - datalen);
 + heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
   *iovcount = seg;
   if (unlikely(log))
   *log_num = nlogs;



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC v6 13/20] virtio: allow to fail setting status

2015-01-07 Thread Cornelia Huck
On Tue, 30 Dec 2014 14:25:37 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Thu, Dec 11, 2014 at 02:25:15PM +0100, Cornelia Huck wrote:
  virtio-1 allow setting of the FEATURES_OK status bit to fail if
  the negotiated feature bits are inconsistent: let's fail
  virtio_set_status() in that case and update virtio-ccw to post an
  error to the guest.
  
  Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
 
 Right but a separate validate_features call is awkward.
 How about we defer virtio_set_features until FEATURES_OK,
 and teach virtio_set_features that it can fail?

Hm. But we would need to keep virtio_set_features() where it is called
now for legacy devices, as they will never see FEATURES_OK, right? So
we need to make this depending on revisions (or whatever the equivalent
is for pci/mmio), as we cannot check for VERSION_1. Not sure whether
this makes the code easier to follow.

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC v6 18/20] virtio: support revision-specific features

2015-01-07 Thread Cornelia Huck
On Sun, 28 Dec 2014 10:32:06 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Thu, Dec 11, 2014 at 02:25:20PM +0100, Cornelia Huck wrote:
  Devices may support different sets of feature bits depending on which
  revision they're operating at. Let's give the transport a way to
  re-query the device about its features when the revision has been
  changed.
  
  Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
 
 So now we have both get_features and get_features_rev, and
 it's never clear which revision does host_features refer to.
 IMHO that's just too messy.
 Let's add get_legacy_features and host_legacy_features instead?

I wanted to avoid touching anything that does not support version 1.
And this interface might still work for later revisions, no?

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC v6 19/20] virtio-blk: revision specific feature bits

2015-01-07 Thread Cornelia Huck
On Sun, 28 Dec 2014 12:24:46 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Thu, Dec 11, 2014 at 02:25:21PM +0100, Cornelia Huck wrote:
  Wire up virtio-blk to provide different feature bit sets depending
  on whether legacy or v1.0 has been requested.
  
  Note that VERSION_1 is still disabled due to missing ANY_LAYOUT support.
  
  Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
 
 So we need some way for devices to tell transports
 not to negotiate rev 1.
 Does clearing VERSION_1 have this effect?
 
I just noticed that my patch is running in circles here.

What we need is probably the transport-dependent maximum revision
checker (which at least for ccw is acting on a device) pass in the
requested revision and check if the feature bits for the revision
include VERSION_1. Does that make sense?

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC v6 13/20] virtio: allow to fail setting status

2015-01-07 Thread Michael S. Tsirkin
On Wed, Jan 07, 2015 at 05:13:32PM +0100, Cornelia Huck wrote:
 On Tue, 30 Dec 2014 14:25:37 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Thu, Dec 11, 2014 at 02:25:15PM +0100, Cornelia Huck wrote:
   virtio-1 allow setting of the FEATURES_OK status bit to fail if
   the negotiated feature bits are inconsistent: let's fail
   virtio_set_status() in that case and update virtio-ccw to post an
   error to the guest.
   
   Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
  
  Right but a separate validate_features call is awkward.
  How about we defer virtio_set_features until FEATURES_OK,
  and teach virtio_set_features that it can fail?
 
 Hm. But we would need to keep virtio_set_features() where it is called
 now for legacy devices, as they will never see FEATURES_OK, right?
 So
 we need to make this depending on revisions (or whatever the equivalent
 is for pci/mmio), as we cannot check for VERSION_1. Not sure whether
 this makes the code easier to follow.

So let's make this a separate callback then.
virtio_legacy_set_features?

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC v6 18/20] virtio: support revision-specific features

2015-01-07 Thread Michael S. Tsirkin
On Wed, Jan 07, 2015 at 05:22:32PM +0100, Cornelia Huck wrote:
 On Sun, 28 Dec 2014 10:32:06 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Thu, Dec 11, 2014 at 02:25:20PM +0100, Cornelia Huck wrote:
   Devices may support different sets of feature bits depending on which
   revision they're operating at. Let's give the transport a way to
   re-query the device about its features when the revision has been
   changed.
   
   Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
  
  So now we have both get_features and get_features_rev, and
  it's never clear which revision does host_features refer to.
  IMHO that's just too messy.
  Let's add get_legacy_features and host_legacy_features instead?
 
 I wanted to avoid touching anything that does not support version 1.
 And this interface might still work for later revisions, no?

We can add _modern_ then, or rename host_features to host_legacy_features
everywhere as preparation.

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH RFC v6 19/20] virtio-blk: revision specific feature bits

2015-01-07 Thread Michael S. Tsirkin
On Wed, Jan 07, 2015 at 05:29:49PM +0100, Cornelia Huck wrote:
 On Sun, 28 Dec 2014 12:24:46 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Thu, Dec 11, 2014 at 02:25:21PM +0100, Cornelia Huck wrote:
   Wire up virtio-blk to provide different feature bit sets depending
   on whether legacy or v1.0 has been requested.
   
   Note that VERSION_1 is still disabled due to missing ANY_LAYOUT support.
   
   Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com
  
  So we need some way for devices to tell transports
  not to negotiate rev 1.
  Does clearing VERSION_1 have this effect?
  
 I just noticed that my patch is running in circles here.
 
 What we need is probably the transport-dependent maximum revision
 checker (which at least for ccw is acting on a device) pass in the
 requested revision and check if the feature bits for the revision
 include VERSION_1. Does that make sense?

Just make devices set 'rev 1 supported' flag?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization