Re: [Qemu-devel] [PATCH 1/4] hw/s390x/virtio-ccw: Add virtio_ccw_device_plugged for virtio-ccw
Date:
Sat, 18 Apr 2015 09:36:41 +0800
On Friday, 17 April 2015, Peter Maydell <address@hidden> wrote: > On 17 April 2015 at 13:13, Shannon Zhao <address@hidden> wrote: >> Add virtio_ccw_device_plugged, it can be used to get backend's features. >> >> Signed-off-by: Shannon Zhao <address@hidden> >> Signed-off-by: Shannon Zhao <address@hidden> >> --- >> hw/s390x/virtio-ccw.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c >> index 130535c..30ca377 100644 >> --- a/hw/s390x/virtio-ccw.c >> +++ b/hw/s390x/virtio-ccw.c >> @@ -1395,6 +1395,16 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) >> return 0; >> } >> >> +/* This is called by virtio-bus just after the device is plugged. */ >> +static void virtio_ccw_device_plugged(DeviceState *d) >> +{ >> + VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); >> + >> + /* Only the first 32 feature bits are used. */ >> + dev->host_features[0] = virtio_bus_get_vdev_features(&dev->bus, >> + dev->host_features[0]); >> +} > > This means that this transport now calls virtio_bus_get_vdev_features > twice, which doesn't look right. In particular, we call it from > realize to set dev->host_features[0], and then add some features to > dev->host_features[0]. Then I think we will call the 'plugged' > method which will throw away those extra features. > > So I think that if we need to call this from 'plugged' > rather than 'realize' we need to move all the code for > setting host_features from 'realize' to here. >
So sorry, when I reply this mail I'm using mobile phone, no codes on hand. So I didn't confirm that.
> But I'm confused about why this change is necessary -- > don't the blk backends already use the "properties are > on the backend" approach, and they work with this transport? >
Ok, maybe I missed that the transport already get the features when realized. If so, this change is not necessary.