| Message ID | f91470cb09ef21f7caeeb81eda407d498b3c1ad1.1296809653.git.amit.shah@redhat.com |
|---|---|
| State | New |
| Headers | show |
Amit Shah <amit.shah@redhat.com> wrote: > This can happen if a port gets unplugged before guest has chance to > initialise vqs. > > Reported-by: Juan Quintela <quintela@redhat.com> > Signed-off-by: Amit Shah <amit.shah@redhat.com> > --- > hw/virtio-serial-bus.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > index 09e22aa..e05ab5e 100644 > --- a/hw/virtio-serial-bus.c > +++ b/hw/virtio-serial-bus.c > @@ -117,6 +117,9 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev) > { > VirtQueueElement elem; > > + if (!virtio_queue_ready(vq)) { > + return; > + } > while (virtqueue_pop(vq, &elem)) { > virtqueue_push(vq, &elem, 0); > } Reviewed-by: Juan Quintela <quintela@redhat.com>
On 02/04/2011 02:54 AM, Amit Shah wrote: > This can happen if a port gets unplugged before guest has chance to > initialise vqs. > > Reported-by: Juan Quintela<quintela@redhat.com> > Signed-off-by: Amit Shah<amit.shah@redhat.com> > Applied to master, Thanks. Regards, Anthony Liguori > --- > hw/virtio-serial-bus.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > index 09e22aa..e05ab5e 100644 > --- a/hw/virtio-serial-bus.c > +++ b/hw/virtio-serial-bus.c > @@ -117,6 +117,9 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev) > { > VirtQueueElement elem; > > + if (!virtio_queue_ready(vq)) { > + return; > + } > while (virtqueue_pop(vq,&elem)) { > virtqueue_push(vq,&elem, 0); > } >
On (Fri) 04 Feb 2011 [06:52:04], Anthony Liguori wrote: > On 02/04/2011 02:54 AM, Amit Shah wrote: > >This can happen if a port gets unplugged before guest has chance to > >initialise vqs. > > > >Reported-by: Juan Quintela<quintela@redhat.com> > >Signed-off-by: Amit Shah<amit.shah@redhat.com> > > Applied to master, Thanks. Hi, What's the strategy to commit to 0.14? Amit
On (Sun) 06 Feb 2011 [11:50:46], Amit Shah wrote: > On (Fri) 04 Feb 2011 [06:52:04], Anthony Liguori wrote: > > On 02/04/2011 02:54 AM, Amit Shah wrote: > > >This can happen if a port gets unplugged before guest has chance to > > >initialise vqs. > > > > > >Reported-by: Juan Quintela<quintela@redhat.com> > > >Signed-off-by: Amit Shah<amit.shah@redhat.com> > > > > Applied to master, Thanks. > > What's the strategy to commit to 0.14? Nevermind; saw it in the last pull. Amit
On 02/07/2011 08:14 PM, Amit Shah wrote: > On (Sun) 06 Feb 2011 [11:50:46], Amit Shah wrote: > >> On (Fri) 04 Feb 2011 [06:52:04], Anthony Liguori wrote: >> >>> On 02/04/2011 02:54 AM, Amit Shah wrote: >>> >>>> This can happen if a port gets unplugged before guest has chance to >>>> initialise vqs. >>>> >>>> Reported-by: Juan Quintela<quintela@redhat.com> >>>> Signed-off-by: Amit Shah<amit.shah@redhat.com> >>>> >>> Applied to master, Thanks. >>> >> What's the strategy to commit to 0.14? >> > Nevermind; saw it in the last pull. > Yes, but if we decide to make this a subsection, we may need to revert this before the 0.14.0 release because once 0.14.0 goes out, we can't go back and change it. If that's how you want to approach it, then please send the revert patches. The flow_control=0 patches you sent out before definitely aren't the right approach for migration compatibility. Regards, Anthony Liguori > Amit > >
On (Mon) 07 Feb 2011 [20:25:59], Anthony Liguori wrote: > On 02/07/2011 08:14 PM, Amit Shah wrote: > >On (Sun) 06 Feb 2011 [11:50:46], Amit Shah wrote: > >>On (Fri) 04 Feb 2011 [06:52:04], Anthony Liguori wrote: > >>>On 02/04/2011 02:54 AM, Amit Shah wrote: > >>>>This can happen if a port gets unplugged before guest has chance to > >>>>initialise vqs. > >>>> > >>>>Reported-by: Juan Quintela<quintela@redhat.com> > >>>>Signed-off-by: Amit Shah<amit.shah@redhat.com> > >>>Applied to master, Thanks. > >>What's the strategy to commit to 0.14? > >Nevermind; saw it in the last pull. > > Yes, but if we decide to make this a subsection, we may need to > revert this before the 0.14.0 release because once 0.14.0 goes out, > we can't go back and change it. Right. However, virtio doesn't have vmstate yet. Are we willing to put vmstate in 0.14 to introduce subsections? I doubt that. > If that's how you want to approach it, then please send the revert > patches. The flow_control=0 patches you sent out before definitely > aren't the right approach for migration compatibility. I understand, I've not looked at that thread yet, but if we want to do that for 0.14 w/o vmstate, that might be the only way. Amit
On 02/07/2011 08:55 PM, Amit Shah wrote: > On (Mon) 07 Feb 2011 [20:25:59], Anthony Liguori wrote: > >> On 02/07/2011 08:14 PM, Amit Shah wrote: >> >>> On (Sun) 06 Feb 2011 [11:50:46], Amit Shah wrote: >>> >>>> On (Fri) 04 Feb 2011 [06:52:04], Anthony Liguori wrote: >>>> >>>>> On 02/04/2011 02:54 AM, Amit Shah wrote: >>>>> >>>>>> This can happen if a port gets unplugged before guest has chance to >>>>>> initialise vqs. >>>>>> >>>>>> Reported-by: Juan Quintela<quintela@redhat.com> >>>>>> Signed-off-by: Amit Shah<amit.shah@redhat.com> >>>>>> >>>>> Applied to master, Thanks. >>>>> >>>> What's the strategy to commit to 0.14? >>>> >>> Nevermind; saw it in the last pull. >>> >> Yes, but if we decide to make this a subsection, we may need to >> revert this before the 0.14.0 release because once 0.14.0 goes out, >> we can't go back and change it. >> > Right. > > However, virtio doesn't have vmstate yet. Are we willing to put vmstate > in 0.14 to introduce subsections? I doubt that. > Not, that's why I suggested reverting this change and pushing it to 0.15. We're not going to fix it properly in 0.14. Regards, Anthony Liguori >> If that's how you want to approach it, then please send the revert >> patches. The flow_control=0 patches you sent out before definitely >> aren't the right approach for migration compatibility. >> > I understand, I've not looked at that thread yet, but if we want to do > that for 0.14 w/o vmstate, that might be the only way. > > Amit > >
On (Tue) 08 Feb 2011 [01:40:09], Anthony Liguori wrote: > > > >However, virtio doesn't have vmstate yet. Are we willing to put vmstate > >in 0.14 to introduce subsections? I doubt that. > > Not, that's why I suggested reverting this change and pushing it to > 0.15. We're not going to fix it properly in 0.14. You mean reverting all of the flow control work? But then it exposes another bug: a malicious guest doing huge writes will cause an OOM on the host; it's better to have this fix rather than not have it. Amit
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 09e22aa..e05ab5e 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -117,6 +117,9 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev) { VirtQueueElement elem; + if (!virtio_queue_ready(vq)) { + return; + } while (virtqueue_pop(vq, &elem)) { virtqueue_push(vq, &elem, 0); }
This can happen if a port gets unplugged before guest has chance to initialise vqs. Reported-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Amit Shah <amit.shah@redhat.com> --- hw/virtio-serial-bus.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)