[Qemu-commits] [qemu/qemu] f5946d: vl.c: Fix memory leak in qemu_register_machine()

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: f5946dbab388050da6d9343978a38c81cce0508d
      
https://github.com/qemu/qemu/commit/f5946dbab388050da6d9343978a38c81cce0508d
  Author: Christian Borntraeger <address@hidden>
  Date:   2014-03-19 (Wed, 19 Mar 2014)

  Changed paths:
    M vl.c

  Log Message:
  -----------
  vl.c: Fix memory leak in qemu_register_machine()

Since commit 261747f176f6 (vl: Use MachineClass instead of global
QEMUMachine list) valgrind complains about the following:

==54082== 57 bytes in 3 blocks are definitely lost in loss record 365 of
729
==54082==    at 0x4031AFE: malloc (vg_replace_malloc.c:292)
==54082==    by 0x4145569: g_malloc (in
/usr/lib64/libglib-2.0.so.0.3400.2)
==54082==    by 0x415F9E9: g_strconcat (in
/usr/lib64/libglib-2.0.so.0.3400.2)
==54082==    by 0x80157FE7: qemu_register_machine (vl.c:1597)
==54082==    by 0x80208E6B: module_call_init (module.c:105)
==54082==    by 0x80013B91: main (vl.c:3000)

Turns out that valgrind is right. We simply forget the memory that
g_strconcat() has allocated. Lets free it after the type_register().
We need a 2nd variable due to constness of the name part of the
type structure.

Signed-off-by: Christian Borntraeger <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Andreas Färber <address@hidden>


  Commit: c8897e8eb965c0d091683ffaf127c50f8231d048
      
https://github.com/qemu/qemu/commit/c8897e8eb965c0d091683ffaf127c50f8231d048
  Author: Marcel Apfelbaum <address@hidden>
  Date:   2014-03-19 (Wed, 19 Mar 2014)

  Changed paths:
    M include/hw/boards.h
    M vl.c

  Log Message:
  -----------
  vl.c: Fix OpenBSD compilation issue due to namespace collisions

Machine rewriting added MACHINE() macro which is
already in use by other OpenBSD library.
Since qemu/sockets.h exposes the OpenBSD namespace,
the minimalistic approach is to add it as the first QEMU include.

Reported-by: Brad Smith <address@hidden>
Signed-off-by: Marcel Apfelbaum <address@hidden>
Signed-off-by: Andreas Färber <address@hidden>


  Commit: f5ec6704c73932291c303d0cf72352ac26411d0d
      
https://github.com/qemu/qemu/commit/f5ec6704c73932291c303d0cf72352ac26411d0d
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2014-03-19 (Wed, 19 Mar 2014)

  Changed paths:
    M qom/object.c

  Log Message:
  -----------
  qom: Split object_property_set_link()

The path resolution logic in object_property_set_link() should be a
separate function.  This makes the code easier to read and maintain.

Signed-off-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Signed-off-by: Andreas Färber <address@hidden>


  Commit: c6aed9833419eed9de19919ff31aa021a6171521
      
https://github.com/qemu/qemu/commit/c6aed9833419eed9de19919ff31aa021a6171521
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2014-03-19 (Wed, 19 Mar 2014)

  Changed paths:
    M qom/object.c

  Log Message:
  -----------
  qom: Don't make link NULL on object_property_set_link() failure

The error behavior of object_property_set_link() is dangerous.  It sets
the link property object to NULL if an error occurs.  A setter function
should either succeed or fail, it shouldn't leave the value NULL on
failure.

Signed-off-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Signed-off-by: Andreas Färber <address@hidden>


  Commit: 9561fda8d90e176bef598ba87c42a1bd6ad03ef7
      
https://github.com/qemu/qemu/commit/9561fda8d90e176bef598ba87c42a1bd6ad03ef7
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2014-03-19 (Wed, 19 Mar 2014)

  Changed paths:
    M hw/core/qdev.c
    M hw/dma/xilinx_axidma.c
    M hw/net/xilinx_axienet.c
    M hw/pcmcia/pxa2xx.c
    M hw/s390x/s390-virtio-bus.c
    M hw/s390x/virtio-ccw.c
    M hw/virtio/virtio-pci.c
    M hw/virtio/virtio-rng.c
    M include/qom/object.h
    M qom/object.c
    M ui/console.c

  Log Message:
  -----------
  qom: Make QOM link property unref optional

Some object_property_add_link() callers expect property deletion to
unref the link property object.  Other callers expect to manage the
refcount themselves.  The former are currently broken and therefore leak
the link property object.

This patch adds a flags argument to object_property_add_link() so the
caller can specify which refcount behavior they require.  The new
OBJ_PROP_LINK_UNREF_ON_RELEASE flag causes the link pointer to be
unreferenced when the property is deleted.

This fixes refcount leaks in qdev.c, xilinx_axidma.c, xilinx_axienet.c,
s390-virtio-bus.c, virtio-pci.c, virtio-rng.c, and ui/console.c.

Rationale for refcount behavior:

 * hw/core/qdev.c
   - bus children are explicitly unreferenced, don't interfere
   - parent_bus is essentially a read-only property that doesn't hold a
     refcount, don't unref
   - hotplug_handler is leaked, do unref

 * hw/dma/xilinx_axidma.c
   - rx stream "dma" links are set using set_link, therefore they
     need unref
   - tx streams are set using set_link, therefore they need unref

 * hw/net/xilinx_axienet.c
   - same reasoning as hw/dma/xilinx_axidma.c

 * hw/pcmcia/pxa2xx.c
   - pxa2xx bypasses set_link and therefore does not use refcounts

 * hw/s390x/s390-virtio-bus.c
 * hw/virtio/virtio-pci.c
 * hw/virtio/virtio-rng.c
 * ui/console.c
   - set_link is used and there is no explicit unref, do unref

Cc: Peter Crosthwaite <address@hidden>
Cc: Alexander Graf <address@hidden>
Cc: Anthony Liguori <address@hidden>
Cc: "Michael S. Tsirkin" <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Signed-off-by: Andreas Färber <address@hidden>


  Commit: 39f72ef94ba74701d18daf82b44c18a60f94eb60
      
https://github.com/qemu/qemu/commit/39f72ef94ba74701d18daf82b44c18a60f94eb60
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2014-03-19 (Wed, 19 Mar 2014)

  Changed paths:
    M hw/core/qdev-properties.c
    M hw/core/qdev.c
    M hw/dma/xilinx_axidma.c
    M hw/net/xilinx_axienet.c
    M hw/pcmcia/pxa2xx.c
    M hw/s390x/s390-virtio-bus.c
    M hw/s390x/virtio-ccw.c
    M hw/virtio/virtio-pci.c
    M hw/virtio/virtio-rng.c
    M include/hw/qdev-properties.h
    M include/qom/object.h
    M qom/object.c
    M ui/console.c

  Log Message:
  -----------
  qom: Add check() argument to object_property_add_link()

There are currently three types of object_property_add_link() callers:

1. The link property may be set at any time.
2. The link property of a DeviceState instance may only be set before
   realize.
3. The link property may never be set, it is read-only.

Something similar can already be achieved with
object_property_add_str()'s set() argument.  Follow its example and add
a check() argument to object_property_add_link().

Also provide default check() functions for case #1 and #2.  Case #3 is
covered by passing a NULL function pointer.

Cc: Peter Crosthwaite <address@hidden>
Cc: Alexander Graf <address@hidden>
Cc: Anthony Liguori <address@hidden>
Cc: "Michael S. Tsirkin" <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
[AF: Tweaked documentation comment]
Signed-off-by: Andreas Färber <address@hidden>


  Commit: abdffd1fb78c1b98bda925d3d59123beca6761a3
      
https://github.com/qemu/qemu/commit/abdffd1fb78c1b98bda925d3d59123beca6761a3
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2014-03-19 (Wed, 19 Mar 2014)

  Changed paths:
    M hw/virtio/virtio-rng.c

  Log Message:
  -----------
  virtio-rng: Avoid default_backend refcount leak

QOM child properties take a reference to the object and release it when
the property is deleted.  Therefore we should unref the default_backend
after we have added it as a child property.

Cc: KONRAD Frederic <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Paolo Bonzini <address@hidden>
Signed-off-by: Andreas Färber <address@hidden>


  Commit: 037b7addb7f9ad5dc52c3d05da8b2f60386252ff
      
https://github.com/qemu/qemu/commit/037b7addb7f9ad5dc52c3d05da8b2f60386252ff
  Author: Peter Maydell <address@hidden>
  Date:   2014-03-19 (Wed, 19 Mar 2014)

  Changed paths:
    M hw/core/qdev-properties.c
    M hw/core/qdev.c
    M hw/dma/xilinx_axidma.c
    M hw/net/xilinx_axienet.c
    M hw/pcmcia/pxa2xx.c
    M hw/s390x/s390-virtio-bus.c
    M hw/s390x/virtio-ccw.c
    M hw/virtio/virtio-pci.c
    M hw/virtio/virtio-rng.c
    M include/hw/boards.h
    M include/hw/qdev-properties.h
    M include/qom/object.h
    M qom/object.c
    M ui/console.c
    M vl.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/afaerber/tags/qom-devices-for-2.0' into 
staging

QOM/QTest infrastructure fixes

* QOM machine memory and build fixes
* QOM link<> and child<> property reference counting fixes

# gpg: Signature made Wed 19 Mar 2014 21:44:04 GMT using RSA key ID 3E7E013F
# gpg: Good signature from "Andreas Färber <address@hidden>"
# gpg:                 aka "Andreas Färber <address@hidden>"

* remotes/afaerber/tags/qom-devices-for-2.0:
  virtio-rng: Avoid default_backend refcount leak
  qom: Add check() argument to object_property_add_link()
  qom: Make QOM link property unref optional
  qom: Don't make link NULL on object_property_set_link() failure
  qom: Split object_property_set_link()
  vl.c: Fix OpenBSD compilation issue due to namespace collisions
  vl.c: Fix memory leak in qemu_register_machine()

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/f71e769d0754...037b7addb7f9