[Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules

Daniel P. Berrangé posted 3 patches 7 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180514171913.17664-1-berrange@redhat.com
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
util/qemu-option.c  |  8 +++-----
2 files changed, 19 insertions(+), 24 deletions(-)
[Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules
Posted by Daniel P. Berrangé 7 years, 5 months ago
This short series fixes a flaw identified by coverity which broke the
handling of multiboot modules.

Daniel P. Berrangé (3):
  i386: fix regression parsing multiboot initrd modules
  i386: only parse the initrd_filename once for multiboot modules
  opts: remove redundant check for NULL parameter

 hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
 util/qemu-option.c  |  8 +++-----
 2 files changed, 19 insertions(+), 24 deletions(-)

-- 
2.17.0


Re: [Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules
Posted by Daniel P. Berrangé 7 years, 4 months ago
ping...

On Mon, May 14, 2018 at 06:19:10PM +0100, Daniel P. Berrangé wrote:
> This short series fixes a flaw identified by coverity which broke the
> handling of multiboot modules.
> 
> Daniel P. Berrangé (3):
>   i386: fix regression parsing multiboot initrd modules
>   i386: only parse the initrd_filename once for multiboot modules
>   opts: remove redundant check for NULL parameter
> 
>  hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
>  util/qemu-option.c  |  8 +++-----
>  2 files changed, 19 insertions(+), 24 deletions(-)
> 
> -- 
> 2.17.0
> 

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 :|

Re: [Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules
Posted by Roman Kagan 7 years, 3 months ago
On Thu, Jun 07, 2018 at 10:47:47AM +0100, Daniel P. Berrangé wrote:
> ping...
> 
> On Mon, May 14, 2018 at 06:19:10PM +0100, Daniel P. Berrangé wrote:
> > This short series fixes a flaw identified by coverity which broke the
> > handling of multiboot modules.
> > 
> > Daniel P. Berrangé (3):
> >   i386: fix regression parsing multiboot initrd modules
> >   i386: only parse the initrd_filename once for multiboot modules
> >   opts: remove redundant check for NULL parameter
> > 
> >  hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
> >  util/qemu-option.c  |  8 +++-----
> >  2 files changed, 19 insertions(+), 24 deletions(-)
> > 
> > -- 
> > 2.17.0

Any reason this hasn't been merged yet?

This fixes a regression that, in particular, makes QEMU segfault when
running kvm-unit-tests.

Thanks,
Roman.

Re: [Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules
Posted by Eduardo Habkost 7 years, 3 months ago
On Tue, Jul 10, 2018 at 08:11:58PM +0300, Roman Kagan wrote:
> On Thu, Jun 07, 2018 at 10:47:47AM +0100, Daniel P. Berrangé wrote:
> > ping...
> > 
> > On Mon, May 14, 2018 at 06:19:10PM +0100, Daniel P. Berrangé wrote:
> > > This short series fixes a flaw identified by coverity which broke the
> > > handling of multiboot modules.
> > > 
> > > Daniel P. Berrangé (3):
> > >   i386: fix regression parsing multiboot initrd modules
> > >   i386: only parse the initrd_filename once for multiboot modules
> > >   opts: remove redundant check for NULL parameter
> > > 
> > >  hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
> > >  util/qemu-option.c  |  8 +++-----
> > >  2 files changed, 19 insertions(+), 24 deletions(-)
> > > 
> > > -- 
> > > 2.17.0
> 
> Any reason this hasn't been merged yet?

Lack of reviews, unfortunately.  I guess we don't have many
people familiar with the multiboot code.

> 
> This fixes a regression that, in particular, makes QEMU segfault when
> running kvm-unit-tests.

As it is a bug fix, I will try to review merge it for the next
rc.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules
Posted by Roman Kagan 7 years, 2 months ago
On Tue, Jul 10, 2018 at 02:23:09PM -0300, Eduardo Habkost wrote:
> On Tue, Jul 10, 2018 at 08:11:58PM +0300, Roman Kagan wrote:
> > On Thu, Jun 07, 2018 at 10:47:47AM +0100, Daniel P. Berrangé wrote:
> > > ping...
> > > 
> > > On Mon, May 14, 2018 at 06:19:10PM +0100, Daniel P. Berrangé wrote:
> > > > This short series fixes a flaw identified by coverity which broke the
> > > > handling of multiboot modules.
> > > > 
> > > > Daniel P. Berrangé (3):
> > > >   i386: fix regression parsing multiboot initrd modules
> > > >   i386: only parse the initrd_filename once for multiboot modules
> > > >   opts: remove redundant check for NULL parameter
> > > > 
> > > >  hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
> > > >  util/qemu-option.c  |  8 +++-----
> > > >  2 files changed, 19 insertions(+), 24 deletions(-)
> > > > 
> > > > -- 
> > > > 2.17.0
> > 
> > Any reason this hasn't been merged yet?
> 
> Lack of reviews, unfortunately.  I guess we don't have many
> people familiar with the multiboot code.
> 
> > 
> > This fixes a regression that, in particular, makes QEMU segfault when
> > running kvm-unit-tests.
> 
> As it is a bug fix, I will try to review merge it for the next
> rc.

Looks like it ended up missing 3.0, so kvm-unit-tests still segfault
with the released version of QEMU :(

Roman.

Re: [Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules
Posted by Daniel P. Berrangé 7 years, 2 months ago
On Thu, Aug 16, 2018 at 05:34:43PM +0300, Roman Kagan wrote:
> On Tue, Jul 10, 2018 at 02:23:09PM -0300, Eduardo Habkost wrote:
> > On Tue, Jul 10, 2018 at 08:11:58PM +0300, Roman Kagan wrote:
> > > On Thu, Jun 07, 2018 at 10:47:47AM +0100, Daniel P. Berrangé wrote:
> > > > ping...
> > > > 
> > > > On Mon, May 14, 2018 at 06:19:10PM +0100, Daniel P. Berrangé wrote:
> > > > > This short series fixes a flaw identified by coverity which broke the
> > > > > handling of multiboot modules.
> > > > > 
> > > > > Daniel P. Berrangé (3):
> > > > >   i386: fix regression parsing multiboot initrd modules
> > > > >   i386: only parse the initrd_filename once for multiboot modules
> > > > >   opts: remove redundant check for NULL parameter
> > > > > 
> > > > >  hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
> > > > >  util/qemu-option.c  |  8 +++-----
> > > > >  2 files changed, 19 insertions(+), 24 deletions(-)
> > > > > 
> > > > > -- 
> > > > > 2.17.0
> > > 
> > > Any reason this hasn't been merged yet?
> > 
> > Lack of reviews, unfortunately.  I guess we don't have many
> > people familiar with the multiboot code.
> > 
> > > 
> > > This fixes a regression that, in particular, makes QEMU segfault when
> > > running kvm-unit-tests.
> > 
> > As it is a bug fix, I will try to review merge it for the next
> > rc.
> 
> Looks like it ended up missing 3.0, so kvm-unit-tests still segfault
> with the released version of QEMU :(

Please check again, as I see it merged before 3.0:


  commit 59b5552f020b739e273e969a0933c23d8f4e2284
  Merge: ccf02d73d1 dfaa7d50b0
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   Tue Jul 17 17:06:32 2018 +0100

    Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
    
    Bug fixes.
    
    # gpg: Signature made Tue 17 Jul 2018 16:06:07 BST
    # gpg:                using RSA key BFFBD25F78C7AE83
    # gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>"
    # gpg:                 aka "Paolo Bonzini <pbonzini@redhat.com>"
    # Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4  E2F7 7E15 100C CD36 69B1
    #      Subkey fingerprint: F133 3857 4B66 2389 866C  7682 BFFB D25F 78C7 AE83
    
    * remotes/bonzini/tags/for-upstream:
      Document command line options with single dash
      opts: remove redundant check for NULL parameter
      i386: only parse the initrd_filename once for multiboot modules
      i386: fix regression parsing multiboot initrd modules
      virtio-scsi: fix hotplug ->reset() vs event race
      qdev: add HotplugHandler->post_plug() callback
      hw/char/serial: retry write if EAGAIN
      PC Chipset: Improve serial divisor calculation
      vhost-user-test: added proper TestServer *dest initialization in test_migrate()
      hyperv: ensure VP index equal to QEMU cpu_index
      hyperv: rename vcpu_id to vp_index
      accel: Fix typo and grammar in comment
      dump: add kernel_gs_base to QEMU CPU state
    
    Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


$ git describe  59b5552f020b739e273e969a0933c23d8f4e2284
v3.0.0-rc0-80-g59b5552f02

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 :|

Re: [Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules
Posted by Roman Kagan 7 years, 2 months ago
On Thu, Aug 16, 2018 at 03:38:14PM +0100, Daniel P. Berrangé wrote:
> On Thu, Aug 16, 2018 at 05:34:43PM +0300, Roman Kagan wrote:
> > On Tue, Jul 10, 2018 at 02:23:09PM -0300, Eduardo Habkost wrote:
> > > On Tue, Jul 10, 2018 at 08:11:58PM +0300, Roman Kagan wrote:
> > > > On Thu, Jun 07, 2018 at 10:47:47AM +0100, Daniel P. Berrangé wrote:
> > > > > ping...
> > > > > 
> > > > > On Mon, May 14, 2018 at 06:19:10PM +0100, Daniel P. Berrangé wrote:
> > > > > > This short series fixes a flaw identified by coverity which broke the
> > > > > > handling of multiboot modules.
> > > > > > 
> > > > > > Daniel P. Berrangé (3):
> > > > > >   i386: fix regression parsing multiboot initrd modules
> > > > > >   i386: only parse the initrd_filename once for multiboot modules
> > > > > >   opts: remove redundant check for NULL parameter
> > > > > > 
> > > > > >  hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
> > > > > >  util/qemu-option.c  |  8 +++-----
> > > > > >  2 files changed, 19 insertions(+), 24 deletions(-)
> > > > > > 
> > > > > > -- 
> > > > > > 2.17.0
> > > > 
> > > > Any reason this hasn't been merged yet?
> > > 
> > > Lack of reviews, unfortunately.  I guess we don't have many
> > > people familiar with the multiboot code.
> > > 
> > > > 
> > > > This fixes a regression that, in particular, makes QEMU segfault when
> > > > running kvm-unit-tests.
> > > 
> > > As it is a bug fix, I will try to review merge it for the next
> > > rc.
> > 
> > Looks like it ended up missing 3.0, so kvm-unit-tests still segfault
> > with the released version of QEMU :(
> 
> Please check again, as I see it merged before 3.0:

Oh, you're right, I just ran the tests against a wrong QEMU.

It's OK now indeed.

Sorry for the noise,
Roman.

Re: [Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules
Posted by Roman Kagan 7 years, 4 months ago
On Mon, May 14, 2018 at 06:19:10PM +0100, Daniel P. Berrangé wrote:
> This short series fixes a flaw identified by coverity which broke the
> handling of multiboot modules.

This flaw also makes QEMU segfault when running kvm-unit-tests, and the
series fixes it.

So

Tested-by: Roman Kagan <rkagan@virtuozzo.com>

Wondering if anyone else runs kvm-unit-tests with QEMU master,
Roman.

> 
> Daniel P. Berrangé (3):
>   i386: fix regression parsing multiboot initrd modules
>   i386: only parse the initrd_filename once for multiboot modules
>   opts: remove redundant check for NULL parameter
> 
>  hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
>  util/qemu-option.c  |  8 +++-----
>  2 files changed, 19 insertions(+), 24 deletions(-)
> 
> -- 
> 2.17.0
> 
> 

Re: [Qemu-devel] [PATCH 0/3] i386: fix handling of multiboot modules
Posted by Michael S. Tsirkin 7 years, 4 months ago
On Wed, Jun 20, 2018 at 05:57:56PM +0300, Roman Kagan wrote:
> On Mon, May 14, 2018 at 06:19:10PM +0100, Daniel P. Berrangé wrote:
> > This short series fixes a flaw identified by coverity which broke the
> > handling of multiboot modules.
> 
> This flaw also makes QEMU segfault when running kvm-unit-tests, and the
> series fixes it.
> 
> So
> 
> Tested-by: Roman Kagan <rkagan@virtuozzo.com>
> 
> Wondering if anyone else runs kvm-unit-tests with QEMU master,
> Roman.

It might be a good idea to make it a submodule, and
use it for unit tests. I'll take a look.

> > 
> > Daniel P. Berrangé (3):
> >   i386: fix regression parsing multiboot initrd modules
> >   i386: only parse the initrd_filename once for multiboot modules
> >   opts: remove redundant check for NULL parameter
> > 
> >  hw/i386/multiboot.c | 35 ++++++++++++++++-------------------
> >  util/qemu-option.c  |  8 +++-----
> >  2 files changed, 19 insertions(+), 24 deletions(-)
> > 
> > -- 
> > 2.17.0
> > 
> >