* [Qemu-devel] [PULL] vhost,e1000 fixes
@ 2010-09-20 18:08 Michael S. Tsirkin
2010-09-20 18:23 ` [Qemu-devel] " Anthony Liguori
2010-09-20 18:23 ` Blue Swirl
0 siblings, 2 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2010-09-20 18:08 UTC (permalink / raw)
To: blauwirbel, Anthony Liguori, qemu-devel
This fixes a bug in vhost error handling
(also triggers build warning with vhost enabled)
and fixes e1000 handling of short frames.
Discussion on best ways to fix the e1000 issue
is still ongoing but the bug is severe enough
for some guests and the fix is safe enough
that I feel we should have it fixed ASAP
and look for that perfect approach later.
Both fixes are 0.13 material IMO.
The following changes since commit 952afb719f3c965bae12b5bd5f0f0f7ed0251cb8:
mingw: use ASLR, no-SEH and DEP if available (2010-09-19 08:36:34 +0000)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony
Michael S. Tsirkin (1):
vhost: fix infinite loop on error path
Stefan Hajnoczi (1):
e1000: Pad short frames to minimum size (60 bytes)
hw/e1000.c | 10 ++++++++++
hw/vhost_net.c | 2 +-
2 files changed, 11 insertions(+), 1 deletions(-)
--
MST
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] Re: [PULL] vhost,e1000 fixes
2010-09-20 18:08 [Qemu-devel] [PULL] vhost,e1000 fixes Michael S. Tsirkin
@ 2010-09-20 18:23 ` Anthony Liguori
2010-09-20 18:23 ` Blue Swirl
1 sibling, 0 replies; 6+ messages in thread
From: Anthony Liguori @ 2010-09-20 18:23 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: blauwirbel, qemu-devel
On 09/20/2010 01:08 PM, Michael S. Tsirkin wrote:
> This fixes a bug in vhost error handling
> (also triggers build warning with vhost enabled)
> and fixes e1000 handling of short frames.
>
> Discussion on best ways to fix the e1000 issue
> is still ongoing but the bug is severe enough
> for some guests and the fix is safe enough
> that I feel we should have it fixed ASAP
> and look for that perfect approach later.
>
> Both fixes are 0.13 material IMO.
>
Pulled. Thanks.
Regards,
Anthony LIguori
> The following changes since commit 952afb719f3c965bae12b5bd5f0f0f7ed0251cb8:
>
> mingw: use ASLR, no-SEH and DEP if available (2010-09-19 08:36:34 +0000)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony
>
> Michael S. Tsirkin (1):
> vhost: fix infinite loop on error path
>
> Stefan Hajnoczi (1):
> e1000: Pad short frames to minimum size (60 bytes)
>
> hw/e1000.c | 10 ++++++++++
> hw/vhost_net.c | 2 +-
> 2 files changed, 11 insertions(+), 1 deletions(-)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] Re: [PULL] vhost,e1000 fixes
2010-09-20 18:08 [Qemu-devel] [PULL] vhost,e1000 fixes Michael S. Tsirkin
2010-09-20 18:23 ` [Qemu-devel] " Anthony Liguori
@ 2010-09-20 18:23 ` Blue Swirl
2010-09-20 19:21 ` Michael S. Tsirkin
1 sibling, 1 reply; 6+ messages in thread
From: Blue Swirl @ 2010-09-20 18:23 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Mon, Sep 20, 2010 at 6:08 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> This fixes a bug in vhost error handling
> (also triggers build warning with vhost enabled)
> and fixes e1000 handling of short frames.
>
> Discussion on best ways to fix the e1000 issue
> is still ongoing but the bug is severe enough
> for some guests and the fix is safe enough
> that I feel we should have it fixed ASAP
> and look for that perfect approach later.
>
> Both fixes are 0.13 material IMO.
>
> The following changes since commit 952afb719f3c965bae12b5bd5f0f0f7ed0251cb8:
>
> mingw: use ASLR, no-SEH and DEP if available (2010-09-19 08:36:34 +0000)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony
>
> Michael S. Tsirkin (1):
> vhost: fix infinite loop on error path
I don't think your fix is correct either, it will call the ioctl()
with file.index == -1. How about int i; for (i = file.index; i >= 0;
i--) { file.index = i;... ?
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] Re: [PULL] vhost,e1000 fixes
2010-09-20 18:23 ` Blue Swirl
@ 2010-09-20 19:21 ` Michael S. Tsirkin
2010-09-20 19:35 ` Blue Swirl
0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2010-09-20 19:21 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel
On Mon, Sep 20, 2010 at 06:23:55PM +0000, Blue Swirl wrote:
> On Mon, Sep 20, 2010 at 6:08 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > This fixes a bug in vhost error handling
> > (also triggers build warning with vhost enabled)
> > and fixes e1000 handling of short frames.
> >
> > Discussion on best ways to fix the e1000 issue
> > is still ongoing but the bug is severe enough
> > for some guests and the fix is safe enough
> > that I feel we should have it fixed ASAP
> > and look for that perfect approach later.
> >
> > Both fixes are 0.13 material IMO.
> >
> > The following changes since commit 952afb719f3c965bae12b5bd5f0f0f7ed0251cb8:
> >
> > mingw: use ASLR, no-SEH and DEP if available (2010-09-19 08:36:34 +0000)
> >
> > are available in the git repository at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony
> >
> > Michael S. Tsirkin (1):
> > vhost: fix infinite loop on error path
>
> I don't think your fix is correct either, it will call the ioctl()
> with file.index == -1.
This is my patch:
- while (--file.index >= 0) {
+ while (file.index-- > 0) {
int r = ioctl(net->dev.control, VHOST_NET_SET_BACKEND, &file);
assert(r >= 0);
}
For ioctl to get called with -1, index needs to be 0
before the decrement, and while won't be entered ...
what am I missing?
> How about int i; for (i = file.index; i >= 0;
> i--) { file.index = i;... ?
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] Re: [PULL] vhost,e1000 fixes
2010-09-20 19:21 ` Michael S. Tsirkin
@ 2010-09-20 19:35 ` Blue Swirl
2010-09-20 19:45 ` Michael S. Tsirkin
0 siblings, 1 reply; 6+ messages in thread
From: Blue Swirl @ 2010-09-20 19:35 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
On Mon, Sep 20, 2010 at 7:21 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Sep 20, 2010 at 06:23:55PM +0000, Blue Swirl wrote:
>> On Mon, Sep 20, 2010 at 6:08 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > This fixes a bug in vhost error handling
>> > (also triggers build warning with vhost enabled)
>> > and fixes e1000 handling of short frames.
>> >
>> > Discussion on best ways to fix the e1000 issue
>> > is still ongoing but the bug is severe enough
>> > for some guests and the fix is safe enough
>> > that I feel we should have it fixed ASAP
>> > and look for that perfect approach later.
>> >
>> > Both fixes are 0.13 material IMO.
>> >
>> > The following changes since commit 952afb719f3c965bae12b5bd5f0f0f7ed0251cb8:
>> >
>> > mingw: use ASLR, no-SEH and DEP if available (2010-09-19 08:36:34 +0000)
>> >
>> > are available in the git repository at:
>> > git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony
>> >
>> > Michael S. Tsirkin (1):
>> > vhost: fix infinite loop on error path
>>
>> I don't think your fix is correct either, it will call the ioctl()
>> with file.index == -1.
>
> This is my patch:
>
> - while (--file.index >= 0) {
> + while (file.index-- > 0) {
> int r = ioctl(net->dev.control, VHOST_NET_SET_BACKEND, &file);
> assert(r >= 0);
> }
>
> For ioctl to get called with -1, index needs to be 0
> before the decrement, and while won't be entered ...
> what am I missing?
Then ioctl() won't be called at all. Is that correct?
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] Re: [PULL] vhost,e1000 fixes
2010-09-20 19:35 ` Blue Swirl
@ 2010-09-20 19:45 ` Michael S. Tsirkin
0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2010-09-20 19:45 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel
On Mon, Sep 20, 2010 at 07:35:36PM +0000, Blue Swirl wrote:
> On Mon, Sep 20, 2010 at 7:21 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Sep 20, 2010 at 06:23:55PM +0000, Blue Swirl wrote:
> >> On Mon, Sep 20, 2010 at 6:08 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > This fixes a bug in vhost error handling
> >> > (also triggers build warning with vhost enabled)
> >> > and fixes e1000 handling of short frames.
> >> >
> >> > Discussion on best ways to fix the e1000 issue
> >> > is still ongoing but the bug is severe enough
> >> > for some guests and the fix is safe enough
> >> > that I feel we should have it fixed ASAP
> >> > and look for that perfect approach later.
> >> >
> >> > Both fixes are 0.13 material IMO.
> >> >
> >> > The following changes since commit 952afb719f3c965bae12b5bd5f0f0f7ed0251cb8:
> >> >
> >> > mingw: use ASLR, no-SEH and DEP if available (2010-09-19 08:36:34 +0000)
> >> >
> >> > are available in the git repository at:
> >> > git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu.git for_anthony
> >> >
> >> > Michael S. Tsirkin (1):
> >> > vhost: fix infinite loop on error path
> >>
> >> I don't think your fix is correct either, it will call the ioctl()
> >> with file.index == -1.
> >
> > This is my patch:
> >
> > - while (--file.index >= 0) {
> > + while (file.index-- > 0) {
> > int r = ioctl(net->dev.control, VHOST_NET_SET_BACKEND, &file);
> > assert(r >= 0);
> > }
> >
> > For ioctl to get called with -1, index needs to be 0
> > before the decrement, and while won't be entered ...
> > what am I missing?
>
> Then ioctl() won't be called at all. Is that correct?
Yes, this means the first vq SET_BACKEND call failed so
nothing needs to be reverted.
--
MST
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-09-20 20:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-20 18:08 [Qemu-devel] [PULL] vhost,e1000 fixes Michael S. Tsirkin
2010-09-20 18:23 ` [Qemu-devel] " Anthony Liguori
2010-09-20 18:23 ` Blue Swirl
2010-09-20 19:21 ` Michael S. Tsirkin
2010-09-20 19:35 ` Blue Swirl
2010-09-20 19:45 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).