| Message ID | 20180627112520.26085-1-pagupta@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 27, 2018 at 04:55:20PM +0530, Pankaj Gupta wrote: > virtio-rng device causing old guest kernels(2.6.32) to hang on latest qemu. > The driver attempts to read from the virtio-rng device too early in it's > initialization. Qemu detects guest is not ready and returns, resulting in > hang. Presumably this all worked previously at some point, so do you know what change in QEMU has caused a regression in this regard. > > Fix is to handle pending request when guest is running and driver status is > set to 'VIRTIO_CONFIG_S_DRIVER_OK'. > > Reported-by: Sergio lopez <slopezpa@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com> > --- > v1->v2: Update new status value early only for virtio-rng (Stefan) > > hw/virtio/virtio-rng.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c > index 289bbcac03..855f1b41d1 100644 > --- a/hw/virtio/virtio-rng.c > +++ b/hw/virtio/virtio-rng.c > @@ -156,6 +156,19 @@ static void check_rate_limit(void *opaque) > vrng->activate_timer = true; > } > > +static void virtio_rng_set_status(VirtIODevice *vdev, uint8_t status) > +{ > + VirtIORNG *vrng = VIRTIO_RNG(vdev); > + > + if (!vdev->vm_running) { > + return; > + } > + vdev->status = status; > + > + /* Something changed, try to process buffers */ > + virtio_rng_process(vrng); > +} > + > static void virtio_rng_device_realize(DeviceState *dev, Error **errp) > { > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > @@ -261,6 +274,7 @@ static void virtio_rng_class_init(ObjectClass *klass, void *data) > vdc->realize = virtio_rng_device_realize; > vdc->unrealize = virtio_rng_device_unrealize; > vdc->get_features = get_features; > + vdc->set_status = virtio_rng_set_status; > } > > static const TypeInfo virtio_rng_info = { > -- > 2.14.3 > > Regards, Daniel
> > On Wed, Jun 27, 2018 at 04:55:20PM +0530, Pankaj Gupta wrote: > > virtio-rng device causing old guest kernels(2.6.32) to hang on latest > > qemu. > > The driver attempts to read from the virtio-rng device too early in it's > > initialization. Qemu detects guest is not ready and returns, resulting in > > hang. > > Presumably this all worked previously at some point, so do you know what > change in QEMU has caused a regression in this regard. Latest qemu(2.x) has this issue with older guest kernel(2.6.32) versions. But it does work properly with upstream guest kernel because the way virtio-rng driver is reworked, it sends request to virtio-rng driver when(before) there is read call. If guest driver is ready when it notifies qemu this works fine. There are number of changes in guest virtio/driver code infrastructure as well as qemu side, I don't know exact commit after which this stopped working for older guest kernels. Thanks, Pankaj > > > > > Fix is to handle pending request when guest is running and driver status > > is > > set to 'VIRTIO_CONFIG_S_DRIVER_OK'. > > > > Reported-by: Sergio lopez <slopezpa@redhat.com> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > Signed-off-by: Pankaj Gupta <pagupta@redhat.com> > > --- > > v1->v2: Update new status value early only for virtio-rng (Stefan) > > > > hw/virtio/virtio-rng.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c > > index 289bbcac03..855f1b41d1 100644 > > --- a/hw/virtio/virtio-rng.c > > +++ b/hw/virtio/virtio-rng.c > > @@ -156,6 +156,19 @@ static void check_rate_limit(void *opaque) > > vrng->activate_timer = true; > > } > > > > +static void virtio_rng_set_status(VirtIODevice *vdev, uint8_t status) > > +{ > > + VirtIORNG *vrng = VIRTIO_RNG(vdev); > > + > > + if (!vdev->vm_running) { > > + return; > > + } > > + vdev->status = status; > > + > > + /* Something changed, try to process buffers */ > > + virtio_rng_process(vrng); > > +} > > + > > static void virtio_rng_device_realize(DeviceState *dev, Error **errp) > > { > > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > > @@ -261,6 +274,7 @@ static void virtio_rng_class_init(ObjectClass *klass, > > void *data) > > vdc->realize = virtio_rng_device_realize; > > vdc->unrealize = virtio_rng_device_unrealize; > > vdc->get_features = get_features; > > + vdc->set_status = virtio_rng_set_status; > > } > > > > static const TypeInfo virtio_rng_info = { > > -- > > 2.14.3 > > > > > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange > |:| > |: https://libvirt.org -o- https://fstop138.berrange.com > |:| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange > |:| > >
On Wed, Jun 27, 2018 at 04:55:20PM +0530, Pankaj Gupta wrote: > +static void virtio_rng_set_status(VirtIODevice *vdev, uint8_t status) > +{ > + VirtIORNG *vrng = VIRTIO_RNG(vdev); > + > + if (!vdev->vm_running) { > + return; > + } > + vdev->status = status; Please add a comment to explain this, otherwise someone might think this is unnecessary since virtio.c also does it.
> > On Wed, Jun 27, 2018 at 04:55:20PM +0530, Pankaj Gupta wrote: > > +static void virtio_rng_set_status(VirtIODevice *vdev, uint8_t status) > > +{ > > + VirtIORNG *vrng = VIRTIO_RNG(vdev); > > + > > + if (!vdev->vm_running) { > > + return; > > + } > > + vdev->status = status; > > Please add a comment to explain this, otherwise someone might think this > is unnecessary since virtio.c also does it. Sure. Will add a comment and send v3. Thanks, Pankaj
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c index 289bbcac03..855f1b41d1 100644 --- a/hw/virtio/virtio-rng.c +++ b/hw/virtio/virtio-rng.c @@ -156,6 +156,19 @@ static void check_rate_limit(void *opaque) vrng->activate_timer = true; } +static void virtio_rng_set_status(VirtIODevice *vdev, uint8_t status) +{ + VirtIORNG *vrng = VIRTIO_RNG(vdev); + + if (!vdev->vm_running) { + return; + } + vdev->status = status; + + /* Something changed, try to process buffers */ + virtio_rng_process(vrng); +} + static void virtio_rng_device_realize(DeviceState *dev, Error **errp) { VirtIODevice *vdev = VIRTIO_DEVICE(dev); @@ -261,6 +274,7 @@ static void virtio_rng_class_init(ObjectClass *klass, void *data) vdc->realize = virtio_rng_device_realize; vdc->unrealize = virtio_rng_device_unrealize; vdc->get_features = get_features; + vdc->set_status = virtio_rng_set_status; } static const TypeInfo virtio_rng_info = {