| Message ID | 1335180384-4804-1-git-send-email-alevy@redhat.com |
|---|---|
| State | New |
| Headers | show |
On (Mon) 23 Apr 2012 [14:26:23], Alon Levy wrote: > We could drop the features parameter but that's a little more work and > it's not really needed. > > Signed-off-by: Alon Levy <alevy@redhat.com> Michael, can you ack this? I'm running these through the test suite and will send them both once everything's fine. > --- > hw/virtio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio.c b/hw/virtio.c > index 064aecf..aeddc81 100644 > --- a/hw/virtio.c > +++ b/hw/virtio.c > @@ -770,10 +770,10 @@ int virtio_set_features(VirtIODevice *vdev, uint32_t val) > bool bad = (val & ~supported_features) != 0; > > val &= supported_features; > + vdev->guest_features = val; > if (vdev->set_features) { > vdev->set_features(vdev, val); > } > - vdev->guest_features = val; > return bad ? -1 : 0; > } > > -- > 1.7.10 > Amit
On Mon, Apr 23, 2012 at 02:26:23PM +0300, Alon Levy wrote: > We could drop the features parameter but that's a little more work and > it's not really needed. > > Signed-off-by: Alon Levy <alevy@redhat.com> I think it's fine the way it is but if you want callbacks to use vdev->guest_features remove the features parameter. Having both is just confusing. > --- > hw/virtio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/virtio.c b/hw/virtio.c > index 064aecf..aeddc81 100644 > --- a/hw/virtio.c > +++ b/hw/virtio.c > @@ -770,10 +770,10 @@ int virtio_set_features(VirtIODevice *vdev, uint32_t val) > bool bad = (val & ~supported_features) != 0; > > val &= supported_features; > + vdev->guest_features = val; > if (vdev->set_features) { > vdev->set_features(vdev, val); > } > - vdev->guest_features = val; > return bad ? -1 : 0; > } > > -- > 1.7.10 >
On Mon, Apr 23, 2012 at 04:06:02PM +0300, Michael S. Tsirkin wrote: > On Mon, Apr 23, 2012 at 02:26:23PM +0300, Alon Levy wrote: > > We could drop the features parameter but that's a little more work and > > it's not really needed. > > > > Signed-off-by: Alon Levy <alevy@redhat.com> > > I think it's fine the way it is but if you want callbacks to use > vdev->guest_features remove the features parameter. > > Having both is just confusing. > I'll just drop this, it's the same with set_status, it's arbitrary. > > > --- > > hw/virtio.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/virtio.c b/hw/virtio.c > > index 064aecf..aeddc81 100644 > > --- a/hw/virtio.c > > +++ b/hw/virtio.c > > @@ -770,10 +770,10 @@ int virtio_set_features(VirtIODevice *vdev, uint32_t val) > > bool bad = (val & ~supported_features) != 0; > > > > val &= supported_features; > > + vdev->guest_features = val; > > if (vdev->set_features) { > > vdev->set_features(vdev, val); > > } > > - vdev->guest_features = val; > > return bad ? -1 : 0; > > } > > > > -- > > 1.7.10 > >
diff --git a/hw/virtio.c b/hw/virtio.c index 064aecf..aeddc81 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -770,10 +770,10 @@ int virtio_set_features(VirtIODevice *vdev, uint32_t val) bool bad = (val & ~supported_features) != 0; val &= supported_features; + vdev->guest_features = val; if (vdev->set_features) { vdev->set_features(vdev, val); } - vdev->guest_features = val; return bad ? -1 : 0; }
We could drop the features parameter but that's a little more work and it's not really needed. Signed-off-by: Alon Levy <alevy@redhat.com> --- hw/virtio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)