[Qemu-devel] [PATCH 00/24] CPU DeviceState v7

[Qemu-devel] [PATCH 00/24] CPU DeviceState v7

[Eduardo Habkost]

Changes on v7:
 - Use the new QEMU_WEAK_ALIAS mechanism instead of the (now extinct)
   GCC_WEAK attribute (patches 20 and 21)

v6:
 - Simple rebase against latest qemu.git master
 - Patch 13: some new typedefs were added and others were removed
 - Patch 19: trivial rebase
v5:
 - Tons of header cleanups just to eliminate qlist.h <-> cpu-common.h circular
   dependency (patches 1-17)
 - Add copyright/license information to qdev-properties.c (patch 17)
 - Add copyright/license information to qdev-properties-system.c (patch 22)
 - use error_report()+abort() instead of hw_error() on qdev.c (patch 18)
 - Move qemu_[un]register_reset() and qemu_devices_reset() to qdev-core.c
   (patch 19)
 - Make vmstate_[un]register() weak stubs, instead of a new function (patch 20)
 - Make sysbus_get_default() weak stub, instead of new qbus reset (un)register
   functions (patch 21)
 - Eliminate qdev-system.c (all code is kept on qdev.c, now) (patch 22)
v4:
  - Add GCC_WEAK_DECL to functions that have GCC_WEAK versions
  - Updated the qdev_init_gpio_in() code on qdev-system.c to current version
  - Patch description updates (moved changelog below "---" and/or move info
    about changes made by different authors between SoB lines)
v3 (submitted by Igor):
  - rebased on top of 8b4a3df (today's master)
  - slight code reshuffling in (see commit's changelog)
     "qdev: separate core from the code used only by qemu-system-*"
     "move qemu_irq typedef out of cpu-common.h"
  - commit messages cleanup
v2:
  Removes the CONFIG_USER_ONLY ifdefs, and use weak symbols to move
  the vmstate and qemu_register_reset() handling to qdev-system.c

git tree for testing:
  https://github.com/ehabkost/qemu-hacks/tree/cpu_qdev.v7

References to previous versions:
  v6: http://article.gmane.org/gmane.comp.emulators.qemu/179918
  v5: http://article.gmane.org/gmane.comp.emulators.qemu/177426
  v4: http://article.gmane.org/gmane.comp.emulators.qemu/176127
  v3: http://article.gmane.org/gmane.comp.emulators.qemu/175980
  v2: http://article.gmane.org/gmane.comp.emulators.qemu/173909
  v1: http://article.gmane.org/gmane.comp.emulators.qemu/166630


Anthony Liguori (1):
  qdev: split up header so it can be used in cpu.h

Eduardo Habkost (20):
  user: move *-user/qemu-types.h to main directory
  user: rename qemu-types.h to qemu-user-types.h
  qemu-common.h: comment about usage rules
  move I/O-related definitions from qemu-common.h to a new header
    (qemu-stdio.h)
  qemu-fsdev-dummy.c: include module.h
  vnc-palette.h: include <stdbool.h>
  ui/vnc-pallete.c: include headers it needs
  qemu-config.h: include headers it needs
  qapi/qmp-registry.c: include headers it needs
  qga/channel-posix.c: include headers it needs
  create qemu-types.h for struct typedefs
  sysemu.h: include qemu-types.h instead of qemu-common.h
  qlist.h: do not include qemu-common.h
  qdev-properties.c: add copyright/license information
  qdev: qdev_create(): use error_report() instead of hw_error()
  qdev: move reset handler list from vl.c to qdev.c
  qdev: add weak aliases for vmstate handling on qdev.c
  qdev: add weak alias to sysbus_get_default() on qdev.c
  qdev-properties.c: separate core from the code used only by
    qemu-system-*
  include qdev code into *-user, too

Igor Mammedov (3):
  move qemu_irq typedef out of cpu-common.h
  qapi-types.h: don't include qemu-common.h
  qom: make CPU a child of DeviceState

 Makefile.objs                                |   8 +
 bsd-user/qemu-types.h                        |  24 --
 bsd-user/qemu.h                              |   2 +-
 cpu-all.h                                    |   2 +-
 fsdev/qemu-fsdev-dummy.c                     |   1 +
 hw/Makefile.objs                             |   9 +-
 hw/arm-misc.h                                |   1 +
 hw/bt.h                                      |   2 +
 hw/devices.h                                 |   2 +
 hw/hw.h                                      |   6 +-
 hw/irq.h                                     |   2 +
 hw/mc146818rtc.c                             |   1 +
 hw/omap.h                                    |   1 +
 hw/qdev-addr.c                               |   1 +
 hw/qdev-core.h                               | 232 +++++++++++++++++
 hw/qdev-monitor.h                            |  16 ++
 hw/qdev-properties-system.c                  | 371 +++++++++++++++++++++++++++
 hw/qdev-properties.c                         | 352 +++----------------------
 hw/qdev-properties.h                         | 131 ++++++++++
 hw/qdev-reset.h                              |  11 +
 hw/qdev.c                                    | 100 ++++++--
 hw/qdev.h                                    | 371 +--------------------------
 hw/soc_dma.h                                 |   1 +
 hw/xen.h                                     |   1 +
 include/qemu/cpu.h                           |   6 +-
 linux-user/qemu.h                            |   2 +-
 qapi/qmp-registry.c                          |   2 +
 qemu-common.h                                | 123 +--------
 qemu-config.h                                |   2 +
 qemu-stdio.h                                 |  76 ++++++
 qemu-types.h                                 |  61 +++++
 linux-user/qemu-types.h => qemu-user-types.h |   0
 qga/channel-posix.c                          |   4 +
 qlist.h                                      |   1 -
 qom/cpu.c                                    |   3 +-
 scripts/qapi-types.py                        |   3 +-
 sysemu.h                                     |   3 +-
 ui/vnc-palette.c                             |   2 +
 ui/vnc-palette.h                             |   1 +
 vl.c                                         |  40 ---
 vmstate.h                                    |   2 +
 41 files changed, 1085 insertions(+), 894 deletions(-)
 delete mode 100644 bsd-user/qemu-types.h
 create mode 100644 hw/qdev-core.h
 create mode 100644 hw/qdev-monitor.h
 create mode 100644 hw/qdev-properties-system.c
 create mode 100644 hw/qdev-properties.h
 create mode 100644 hw/qdev-reset.h
 create mode 100644 qemu-stdio.h
 create mode 100644 qemu-types.h
 rename linux-user/qemu-types.h => qemu-user-types.h (100%)

-- 
1.7.11.7

[Qemu-devel] [PATCH 01/24] user: move *-user/qemu-types.h to main directory

[Eduardo Habkost]

The bsd-user/qemu-types.h and linux-user/qemu-types.h files are almost
the same, but linux-user have the additional definitions of tswapal().

This moves the linux-user file to the main directory, so the same file
can be used by linux-user and bsd-user.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Riku Voipio <riku.voipio@iki.fi>
---
 bsd-user/qemu-types.h                   | 24 ------------------------
 linux-user/qemu-types.h => qemu-types.h |  0
 2 files changed, 24 deletions(-)
 delete mode 100644 bsd-user/qemu-types.h
 rename linux-user/qemu-types.h => qemu-types.h (100%)

diff --git a/bsd-user/qemu-types.h b/bsd-user/qemu-types.h
deleted file mode 100644
index 1adda9f..0000000
--- a/bsd-user/qemu-types.h
+++ /dev/null
@@ -1,24 +0,0 @@
-#ifndef QEMU_TYPES_H
-#define QEMU_TYPES_H
-#include "cpu.h"
-
-#ifdef TARGET_ABI32
-typedef uint32_t abi_ulong;
-typedef int32_t abi_long;
-#define TARGET_ABI_FMT_lx "%08x"
-#define TARGET_ABI_FMT_ld "%d"
-#define TARGET_ABI_FMT_lu "%u"
-#define TARGET_ABI_BITS 32
-#else
-typedef target_ulong abi_ulong;
-typedef target_long abi_long;
-#define TARGET_ABI_FMT_lx TARGET_FMT_lx
-#define TARGET_ABI_FMT_ld TARGET_FMT_ld
-#define TARGET_ABI_FMT_lu TARGET_FMT_lu
-#define TARGET_ABI_BITS TARGET_LONG_BITS
-/* for consistency, define ABI32 too */
-#if TARGET_ABI_BITS == 32
-#define TARGET_ABI32 1
-#endif
-#endif
-#endif
diff --git a/linux-user/qemu-types.h b/qemu-types.h
similarity index 100%
rename from linux-user/qemu-types.h
rename to qemu-types.h
-- 
1.7.11.7

[Qemu-devel] [PATCH 02/24] user: rename qemu-types.h to qemu-user-types.h

[Eduardo Habkost]

The header file is specific for *-user, but I plan to introduce a more
generic qemu-types.h file, so I'm renaming it.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Riku Voipio <riku.voipio@iki.fi>
---
 bsd-user/qemu.h                   | 2 +-
 cpu-all.h                         | 2 +-
 linux-user/qemu.h                 | 2 +-
 qemu-types.h => qemu-user-types.h | 0
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename qemu-types.h => qemu-user-types.h (100%)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 8a5ee3d..d268899 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -11,7 +11,7 @@
 #include <stdlib.h>
 #endif /* DEBUG_REMAP */
 
-#include "qemu-types.h"
+#include "qemu-user-types.h"
 
 enum BSDType {
     target_freebsd,
diff --git a/cpu-all.h b/cpu-all.h
index c9c51b8..d6b2b19 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -180,7 +180,7 @@ static inline void tswap64s(uint64_t *s)
 
 #if defined(CONFIG_USER_ONLY)
 #include <assert.h>
-#include "qemu-types.h"
+#include "qemu-user-types.h"
 
 /* On some host systems the guest address space is reserved on the host.
  * This allows the guest address space to be offset to a convenient location.
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 5e53dca..ceddb3c 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -11,7 +11,7 @@
 #include <stdlib.h>
 #endif /* DEBUG_REMAP */
 
-#include "qemu-types.h"
+#include "qemu-user-types.h"
 
 #include "thunk.h"
 #include "syscall_defs.h"
diff --git a/qemu-types.h b/qemu-user-types.h
similarity index 100%
rename from qemu-types.h
rename to qemu-user-types.h
-- 
1.7.11.7

[Qemu-devel] [PATCH 03/24] qemu-common.h: comment about usage rules

[Eduardo Habkost]

Every time we make a tiny change on a header file, we often find
circular header dependency problems. To avoid this nightmare, we need to
stop including qemu-common.h on other headers, and we should gradually
move the declarations from the catchall qemu-common.h header to their
specific headers.

This simply adds a comment documenting the rules about qemu-common.h,
hoping that people will see it before including qemu-common.h from other
header files, and before adding more declarations to qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qemu-common.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/qemu-common.h b/qemu-common.h
index ac9985c..ea43bfa 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -1,5 +1,14 @@
 
-/* Common header file that is included by all of qemu.  */
+/* Common header file that is included by all of qemu.
+ *
+ * This file is supposed to be included only by .c files. No header file should
+ * depend on qemu-common.h, as this would easily lead to circular header
+ * dependencies.
+ *
+ * If a header files uses a definition from qemu-common.h, that definition
+ * must be moved to a separate header file, and the header that uses it
+ * must include that header.
+ */
 #ifndef QEMU_COMMON_H
 #define QEMU_COMMON_H
 
-- 
1.7.11.7

[Qemu-devel] [PATCH 04/24] move qemu_irq typedef out of cpu-common.h

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

it's necessary for making CPU child of DEVICE without
causing circular header deps.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
[ehabkost: re-added the typedef to hw/irq.h after rebasing]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 (from Igor) -> v2 (sent by Eduardo):
 - Chunk adding typedef on hw/irq.h was not present, because of patch
   rebase against qdev header split patch

Changes v2 -> v3 (from Igor):
 - sysemu.h doesn't need irq.h since 013c2f150

Changes v3 -> v4 (from Eduardo):
 - re-added the typedef code to hw/irq.h, after rebasing the patch
---
 hw/arm-misc.h | 1 +
 hw/bt.h       | 2 ++
 hw/devices.h  | 2 ++
 hw/irq.h      | 2 ++
 hw/omap.h     | 1 +
 hw/soc_dma.h  | 1 +
 hw/xen.h      | 1 +
 qemu-common.h | 1 -
 8 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/arm-misc.h b/hw/arm-misc.h
index adb1665..d129678 100644
--- a/hw/arm-misc.h
+++ b/hw/arm-misc.h
@@ -12,6 +12,7 @@
 #define ARM_MISC_H 1
 
 #include "memory.h"
+#include "hw/irq.h"
 
 /* The CPU is also modeled as an interrupt controller.  */
 #define ARM_PIC_CPU_IRQ 0
diff --git a/hw/bt.h b/hw/bt.h
index a48b8d4..ebf6a37 100644
--- a/hw/bt.h
+++ b/hw/bt.h
@@ -23,6 +23,8 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "hw/irq.h"
+
 /* BD Address */
 typedef struct {
     uint8_t b[6];
diff --git a/hw/devices.h b/hw/devices.h
index 1a55c1e..c60bcab 100644
--- a/hw/devices.h
+++ b/hw/devices.h
@@ -1,6 +1,8 @@
 #ifndef QEMU_DEVICES_H
 #define QEMU_DEVICES_H
 
+#include "hw/irq.h"
+
 /* ??? Not all users of this file can include cpu-common.h.  */
 struct MemoryRegion;
 
diff --git a/hw/irq.h b/hw/irq.h
index e640c10..610e6b7 100644
--- a/hw/irq.h
+++ b/hw/irq.h
@@ -3,6 +3,8 @@
 
 /* Generic IRQ/GPIO pin infrastructure.  */
 
+typedef struct IRQState *qemu_irq;
+
 typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
 
 void qemu_set_irq(qemu_irq irq, int level);
diff --git a/hw/omap.h b/hw/omap.h
index 8bd7c73..2b383ff 100644
--- a/hw/omap.h
+++ b/hw/omap.h
@@ -19,6 +19,7 @@
 #ifndef hw_omap_h
 #include "memory.h"
 # define hw_omap_h		"omap.h"
+#include "hw/irq.h"
 
 # define OMAP_EMIFS_BASE	0x00000000
 # define OMAP2_Q0_BASE		0x00000000
diff --git a/hw/soc_dma.h b/hw/soc_dma.h
index 9340b8f..5948489 100644
--- a/hw/soc_dma.h
+++ b/hw/soc_dma.h
@@ -19,6 +19,7 @@
  */
 
 #include "memory.h"
+#include "hw/irq.h"
 
 struct soc_dma_s;
 struct soc_dma_ch_s;
diff --git a/hw/xen.h b/hw/xen.h
index d14e92d..e3cca7f 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -8,6 +8,7 @@
  */
 #include <inttypes.h>
 
+#include "hw/irq.h"
 #include "qemu-common.h"
 
 /* xen-machine.c */
diff --git a/qemu-common.h b/qemu-common.h
index ea43bfa..6441bee 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -292,7 +292,6 @@ typedef struct PCIEPort PCIEPort;
 typedef struct PCIESlot PCIESlot;
 typedef struct MSIMessage MSIMessage;
 typedef struct SerialState SerialState;
-typedef struct IRQState *qemu_irq;
 typedef struct PCMCIACardState PCMCIACardState;
 typedef struct MouseTransformInfo MouseTransformInfo;
 typedef struct uWireSlave uWireSlave;
-- 
1.7.11.7

[Qemu-devel] [PATCH 05/24] qdev: split up header so it can be used in cpu.h

[Eduardo Habkost]

From: Anthony Liguori <aliguori@us.ibm.com>

Header file dependency is a frickin' nightmare right now.  cpu.h tends to get
included in our 'include everything' header files but qdev also needs to include
those headers mainly for qdev-properties since it knows about CharDriverState
and friends.

We can solve this for now by splitting out qdev.h along the same lines that we
previously split the C file.  Then cpu.h just needs to include qdev-core.h

hw/qdev.h is split into following new headers:
    hw/qdev-core.h
    hw/qdev-properties.h
    hw/qdev-monitor.h

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

[ehabkost: re-add DEFINE_PROP_PCI_HOST_DEVADDR, that was removed on the
 original patch (by mistake, I guess)]
[ehabkost: kill qdev_prop_set_vlan() declaration]
[ehabkost: moved get_fw_dev_path() comment to the original location
 (I don't know why it was moved)]
[ehabkost: removed qdev_exists() declaration]
[ehabkost: keep using 'QemuOpts' instead of 'struct QemuOpts', as
 qdev-core.h includes qemu-option.h]

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Yes, there is "changelog" data before the "---" mark, but I believe that
in this case they are important to indicate authorship and the scope of
the Signed-off-by lines (so they need to get into the git commit
message)

Detailed changelog:

Changes v1 (from Anthony) -> v2 (from Eduardo):
 - re-add DEFINE_PROP_PCI_HOST_DEVADDR, that was removed on the original
   patch (by mistake, I guess)
 - kill qdev_prop_set_vlan() declaration

Changes v2 -> v3 (from Eduardo):
 - moved get_fw_dev_path() comment to the original location (I don't
   know why it was moved)
 - removed qdev_exists() declaration
 - keep using 'QemuOpts' instead of 'struct QemuOpts', as qdev-core.h
   includes qemu-option.h

Changes v3 -> v4 (from Eduardo):
 - Edited commit message to include additional information about the
   specific files being created
---
 hw/mc146818rtc.c     |   1 +
 hw/qdev-addr.c       |   1 +
 hw/qdev-core.h       | 238 +++++++++++++++++++++++++++++++++
 hw/qdev-monitor.h    |  16 +++
 hw/qdev-properties.c |   1 +
 hw/qdev-properties.h | 130 ++++++++++++++++++
 hw/qdev.c            |   1 +
 hw/qdev.h            | 370 +--------------------------------------------------
 8 files changed, 392 insertions(+), 366 deletions(-)
 create mode 100644 hw/qdev-core.h
 create mode 100644 hw/qdev-monitor.h
 create mode 100644 hw/qdev-properties.h

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 98839f2..7d84ce3 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -25,6 +25,7 @@
 #include "qemu-timer.h"
 #include "sysemu.h"
 #include "mc146818rtc.h"
+#include "qapi/qapi-visit-core.h"
 
 #ifdef TARGET_I386
 #include "apic.h"
diff --git a/hw/qdev-addr.c b/hw/qdev-addr.c
index de0ba87..ea32c31 100644
--- a/hw/qdev-addr.c
+++ b/hw/qdev-addr.c
@@ -1,6 +1,7 @@
 #include "qdev.h"
 #include "qdev-addr.h"
 #include "hwaddr.h"
+#include "qapi/qapi-visit-core.h"
 
 /* --- target physical address --- */
 
diff --git a/hw/qdev-core.h b/hw/qdev-core.h
new file mode 100644
index 0000000..fce9e22
--- /dev/null
+++ b/hw/qdev-core.h
@@ -0,0 +1,238 @@
+#ifndef QDEV_CORE_H
+#define QDEV_CORE_H
+
+#include "qemu-queue.h"
+#include "qemu-option.h"
+#include "qemu/object.h"
+#include "hw/irq.h"
+#include "error.h"
+
+typedef struct Property Property;
+
+typedef struct PropertyInfo PropertyInfo;
+
+typedef struct CompatProperty CompatProperty;
+
+typedef struct BusState BusState;
+
+typedef struct BusClass BusClass;
+
+enum DevState {
+    DEV_STATE_CREATED = 1,
+    DEV_STATE_INITIALIZED,
+};
+
+enum {
+    DEV_NVECTORS_UNSPECIFIED = -1,
+};
+
+#define TYPE_DEVICE "device"
+#define DEVICE(obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)
+#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
+#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
+
+typedef int (*qdev_initfn)(DeviceState *dev);
+typedef int (*qdev_event)(DeviceState *dev);
+typedef void (*qdev_resetfn)(DeviceState *dev);
+
+struct VMStateDescription;
+
+typedef struct DeviceClass {
+    ObjectClass parent_class;
+
+    const char *fw_name;
+    const char *desc;
+    Property *props;
+    int no_user;
+
+    /* callbacks */
+    void (*reset)(DeviceState *dev);
+
+    /* device state */
+    const struct VMStateDescription *vmsd;
+
+    /* Private to qdev / bus.  */
+    qdev_initfn init;
+    qdev_event unplug;
+    qdev_event exit;
+    const char *bus_type;
+} DeviceClass;
+
+/* This structure should not be accessed directly.  We declare it here
+   so that it can be embedded in individual device state structures.  */
+struct DeviceState {
+    Object parent_obj;
+
+    const char *id;
+    enum DevState state;
+    QemuOpts *opts;
+    int hotplugged;
+    BusState *parent_bus;
+    int num_gpio_out;
+    qemu_irq *gpio_out;
+    int num_gpio_in;
+    qemu_irq *gpio_in;
+    QLIST_HEAD(, BusState) child_bus;
+    int num_child_bus;
+    int instance_id_alias;
+    int alias_required_for_version;
+};
+
+#define TYPE_BUS "bus"
+#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
+#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
+#define BUS_GET_CLASS(obj) OBJECT_GET_CLASS(BusClass, (obj), TYPE_BUS)
+
+struct BusClass {
+    ObjectClass parent_class;
+
+    /* FIXME first arg should be BusState */
+    void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
+    char *(*get_dev_path)(DeviceState *dev);
+    /*
+     * This callback is used to create Open Firmware device path in accordance
+     * with OF spec http://forthworks.com/standards/of1275.pdf. Individual bus
+     * bindings can be found at http://playground.sun.com/1275/bindings/.
+     */
+    char *(*get_fw_dev_path)(DeviceState *dev);
+    int (*reset)(BusState *bus);
+};
+
+typedef struct BusChild {
+    DeviceState *child;
+    int index;
+    QTAILQ_ENTRY(BusChild) sibling;
+} BusChild;
+
+/**
+ * BusState:
+ * @qom_allocated: Indicates whether the object was allocated by QOM.
+ * @glib_allocated: Indicates whether the object was initialized in-place
+ * yet is expected to be freed with g_free().
+ */
+struct BusState {
+    Object obj;
+    DeviceState *parent;
+    const char *name;
+    int allow_hotplug;
+    bool qom_allocated;
+    bool glib_allocated;
+    int max_index;
+    QTAILQ_HEAD(ChildrenHead, BusChild) children;
+    QLIST_ENTRY(BusState) sibling;
+};
+
+struct Property {
+    const char   *name;
+    PropertyInfo *info;
+    int          offset;
+    uint8_t      bitnr;
+    uint8_t      qtype;
+    int64_t      defval;
+};
+
+struct PropertyInfo {
+    const char *name;
+    const char *legacy_name;
+    const char **enum_table;
+    int (*parse)(DeviceState *dev, Property *prop, const char *str);
+    int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
+    ObjectPropertyAccessor *get;
+    ObjectPropertyAccessor *set;
+    ObjectPropertyRelease *release;
+};
+
+typedef struct GlobalProperty {
+    const char *driver;
+    const char *property;
+    const char *value;
+    QTAILQ_ENTRY(GlobalProperty) next;
+} GlobalProperty;
+
+/*** Board API.  This should go away once we have a machine config file.  ***/
+
+DeviceState *qdev_create(BusState *bus, const char *name);
+DeviceState *qdev_try_create(BusState *bus, const char *name);
+int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
+void qdev_init_nofail(DeviceState *dev);
+void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
+                                 int required_for_version);
+void qdev_unplug(DeviceState *dev, Error **errp);
+void qdev_free(DeviceState *dev);
+int qdev_simple_unplug_cb(DeviceState *dev);
+void qdev_machine_creation_done(void);
+bool qdev_machine_modified(void);
+
+qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
+void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
+
+BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
+
+/*** Device API.  ***/
+
+/* Register device properties.  */
+/* GPIO inputs also double as IRQ sinks.  */
+void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
+void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
+
+BusState *qdev_get_parent_bus(DeviceState *dev);
+
+/*** BUS API. ***/
+
+DeviceState *qdev_find_recursive(BusState *bus, const char *id);
+
+/* Returns 0 to walk children, > 0 to skip walk, < 0 to terminate walk. */
+typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
+typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
+
+void qbus_create_inplace(BusState *bus, const char *typename,
+                         DeviceState *parent, const char *name);
+BusState *qbus_create(const char *typename, DeviceState *parent, const char *name);
+/* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
+ *         < 0 if either devfn or busfn terminate walk somewhere in cursion,
+ *           0 otherwise. */
+int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
+                       qbus_walkerfn *busfn, void *opaque);
+int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
+                       qbus_walkerfn *busfn, void *opaque);
+void qdev_reset_all(DeviceState *dev);
+void qbus_reset_all_fn(void *opaque);
+
+void qbus_free(BusState *bus);
+
+#define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
+
+/* This should go away once we get rid of the NULL bus hack */
+BusState *sysbus_get_default(void);
+
+char *qdev_get_fw_dev_path(DeviceState *dev);
+
+/**
+ * @qdev_machine_init
+ *
+ * Initialize platform devices before machine init.  This is a hack until full
+ * support for composition is added.
+ */
+void qdev_machine_init(void);
+
+/**
+ * @device_reset
+ *
+ * Reset a single device (by calling the reset method).
+ */
+void device_reset(DeviceState *dev);
+
+const struct VMStateDescription *qdev_get_vmsd(DeviceState *dev);
+
+const char *qdev_fw_name(DeviceState *dev);
+
+Object *qdev_get_machine(void);
+
+/* FIXME: make this a link<> */
+void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
+
+extern int qdev_hotplug;
+
+char *qdev_get_dev_path(DeviceState *dev);
+
+#endif
diff --git a/hw/qdev-monitor.h b/hw/qdev-monitor.h
new file mode 100644
index 0000000..220ceba
--- /dev/null
+++ b/hw/qdev-monitor.h
@@ -0,0 +1,16 @@
+#ifndef QEMU_QDEV_MONITOR_H
+#define QEMU_QDEV_MONITOR_H
+
+#include "qdev-core.h"
+#include "monitor.h"
+
+/*** monitor commands ***/
+
+void do_info_qtree(Monitor *mon);
+void do_info_qdm(Monitor *mon);
+int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int qdev_device_help(QemuOpts *opts);
+DeviceState *qdev_device_add(QemuOpts *opts);
+
+#endif
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 8aca0d4..81d901c 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -4,6 +4,7 @@
 #include "blockdev.h"
 #include "hw/block-common.h"
 #include "net/hub.h"
+#include "qapi/qapi-visit-core.h"
 
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 {
diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
new file mode 100644
index 0000000..5b046ab
--- /dev/null
+++ b/hw/qdev-properties.h
@@ -0,0 +1,130 @@
+#ifndef QEMU_QDEV_PROPERTIES_H
+#define QEMU_QDEV_PROPERTIES_H
+
+#include "qdev-core.h"
+
+/*** qdev-properties.c ***/
+
+extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_uint8;
+extern PropertyInfo qdev_prop_uint16;
+extern PropertyInfo qdev_prop_uint32;
+extern PropertyInfo qdev_prop_int32;
+extern PropertyInfo qdev_prop_uint64;
+extern PropertyInfo qdev_prop_hex8;
+extern PropertyInfo qdev_prop_hex32;
+extern PropertyInfo qdev_prop_hex64;
+extern PropertyInfo qdev_prop_string;
+extern PropertyInfo qdev_prop_chr;
+extern PropertyInfo qdev_prop_ptr;
+extern PropertyInfo qdev_prop_macaddr;
+extern PropertyInfo qdev_prop_losttickpolicy;
+extern PropertyInfo qdev_prop_bios_chs_trans;
+extern PropertyInfo qdev_prop_drive;
+extern PropertyInfo qdev_prop_netdev;
+extern PropertyInfo qdev_prop_vlan;
+extern PropertyInfo qdev_prop_pci_devfn;
+extern PropertyInfo qdev_prop_blocksize;
+extern PropertyInfo qdev_prop_pci_host_devaddr;
+
+#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
+        .name      = (_name),                                    \
+        .info      = &(_prop),                                   \
+        .offset    = offsetof(_state, _field)                    \
+            + type_check(_type,typeof_field(_state, _field)),    \
+        }
+#define DEFINE_PROP_DEFAULT(_name, _state, _field, _defval, _prop, _type) { \
+        .name      = (_name),                                           \
+        .info      = &(_prop),                                          \
+        .offset    = offsetof(_state, _field)                           \
+            + type_check(_type,typeof_field(_state, _field)),           \
+        .qtype     = QTYPE_QINT,                                        \
+        .defval    = (_type)_defval,                                    \
+        }
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) {  \
+        .name      = (_name),                                    \
+        .info      = &(qdev_prop_bit),                           \
+        .bitnr    = (_bit),                                      \
+        .offset    = offsetof(_state, _field)                    \
+            + type_check(uint32_t,typeof_field(_state, _field)), \
+        .qtype     = QTYPE_QBOOL,                                \
+        .defval    = (bool)_defval,                              \
+        }
+
+#define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
+#define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint16, uint16_t)
+#define DEFINE_PROP_UINT32(_n, _s, _f, _d)                      \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint32, uint32_t)
+#define DEFINE_PROP_INT32(_n, _s, _f, _d)                      \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_int32, int32_t)
+#define DEFINE_PROP_UINT64(_n, _s, _f, _d)                      \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint64, uint64_t)
+#define DEFINE_PROP_HEX8(_n, _s, _f, _d)                       \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex8, uint8_t)
+#define DEFINE_PROP_HEX32(_n, _s, _f, _d)                       \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex32, uint32_t)
+#define DEFINE_PROP_HEX64(_n, _s, _f, _d)                       \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
+#define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d)                   \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
+
+#define DEFINE_PROP_PTR(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)
+#define DEFINE_PROP_CHR(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharDriverState*)
+#define DEFINE_PROP_STRING(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
+#define DEFINE_PROP_NETDEV(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, NetClientState*)
+#define DEFINE_PROP_VLAN(_n, _s, _f)             \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, NetClientState*)
+#define DEFINE_PROP_DRIVE(_n, _s, _f) \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
+#define DEFINE_PROP_MACADDR(_n, _s, _f)         \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
+#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
+                        LostTickPolicy)
+#define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
+#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
+#define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
+    DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
+
+#define DEFINE_PROP_END_OF_LIST()               \
+    {}
+
+/* Set properties between creation and init.  */
+void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
+int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
+void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
+void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
+void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
+void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
+void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value);
+void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
+void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value);
+void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value);
+void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value);
+int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value) QEMU_WARN_UNUSED_RESULT;
+void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverState *value);
+void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
+void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
+/* FIXME: Remove opaque pointer properties.  */
+void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
+
+void qdev_prop_register_global_list(GlobalProperty *props);
+void qdev_prop_set_globals(DeviceState *dev);
+void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
+                                    Property *prop, const char *value);
+
+/**
+ * @qdev_property_add_static - add a @Property to a device referencing a
+ * field in a struct.
+ */
+void qdev_property_add_static(DeviceState *dev, Property *prop, Error **errp);
+
+#endif
diff --git a/hw/qdev.c b/hw/qdev.c
index 9b9aba3..7ddcd24 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,6 +29,7 @@
 #include "qdev.h"
 #include "sysemu.h"
 #include "error.h"
+#include "qapi/qapi-visit-core.h"
 
 int qdev_hotplug = 0;
 static bool qdev_hot_added = false;
diff --git a/hw/qdev.h b/hw/qdev.h
index c6ac636..365b8d6 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -1,371 +1,9 @@
 #ifndef QDEV_H
 #define QDEV_H
 
-#include "hw.h"
-#include "qemu-queue.h"
-#include "qemu-char.h"
-#include "qemu-option.h"
-#include "qapi/qapi-visit-core.h"
-#include "qemu/object.h"
-#include "error.h"
-
-typedef struct Property Property;
-
-typedef struct PropertyInfo PropertyInfo;
-
-typedef struct CompatProperty CompatProperty;
-
-typedef struct BusState BusState;
-
-typedef struct BusClass BusClass;
-
-enum DevState {
-    DEV_STATE_CREATED = 1,
-    DEV_STATE_INITIALIZED,
-};
-
-enum {
-    DEV_NVECTORS_UNSPECIFIED = -1,
-};
-
-#define TYPE_DEVICE "device"
-#define DEVICE(obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)
-#define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
-#define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
-
-typedef int (*qdev_initfn)(DeviceState *dev);
-typedef int (*qdev_event)(DeviceState *dev);
-typedef void (*qdev_resetfn)(DeviceState *dev);
-
-typedef struct DeviceClass {
-    ObjectClass parent_class;
-
-    const char *fw_name;
-    const char *desc;
-    Property *props;
-    int no_user;
-
-    /* callbacks */
-    void (*reset)(DeviceState *dev);
-
-    /* device state */
-    const VMStateDescription *vmsd;
-
-    /* Private to qdev / bus.  */
-    qdev_initfn init;
-    qdev_event unplug;
-    qdev_event exit;
-    const char *bus_type;
-} DeviceClass;
-
-/* This structure should not be accessed directly.  We declare it here
-   so that it can be embedded in individual device state structures.  */
-struct DeviceState {
-    Object parent_obj;
-
-    const char *id;
-    enum DevState state;
-    QemuOpts *opts;
-    int hotplugged;
-    BusState *parent_bus;
-    int num_gpio_out;
-    qemu_irq *gpio_out;
-    int num_gpio_in;
-    qemu_irq *gpio_in;
-    QLIST_HEAD(, BusState) child_bus;
-    int num_child_bus;
-    int instance_id_alias;
-    int alias_required_for_version;
-};
-
-#define TYPE_BUS "bus"
-#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
-#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
-#define BUS_GET_CLASS(obj) OBJECT_GET_CLASS(BusClass, (obj), TYPE_BUS)
-
-struct BusClass {
-    ObjectClass parent_class;
-
-    /* FIXME first arg should be BusState */
-    void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
-    char *(*get_dev_path)(DeviceState *dev);
-    /*
-     * This callback is used to create Open Firmware device path in accordance
-     * with OF spec http://forthworks.com/standards/of1275.pdf. Individual bus
-     * bindings can be found at http://playground.sun.com/1275/bindings/.
-     */
-    char *(*get_fw_dev_path)(DeviceState *dev);
-    int (*reset)(BusState *bus);
-};
-
-typedef struct BusChild {
-    DeviceState *child;
-    int index;
-    QTAILQ_ENTRY(BusChild) sibling;
-} BusChild;
-
-/**
- * BusState:
- * @qom_allocated: Indicates whether the object was allocated by QOM.
- * @glib_allocated: Indicates whether the object was initialized in-place
- * yet is expected to be freed with g_free().
- */
-struct BusState {
-    Object obj;
-    DeviceState *parent;
-    const char *name;
-    int allow_hotplug;
-    bool qom_allocated;
-    bool glib_allocated;
-    int max_index;
-    QTAILQ_HEAD(ChildrenHead, BusChild) children;
-    QLIST_ENTRY(BusState) sibling;
-};
-
-struct Property {
-    const char   *name;
-    PropertyInfo *info;
-    int          offset;
-    uint8_t      bitnr;
-    uint8_t      qtype;
-    int64_t      defval;
-};
-
-struct PropertyInfo {
-    const char *name;
-    const char *legacy_name;
-    const char **enum_table;
-    int (*parse)(DeviceState *dev, Property *prop, const char *str);
-    int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
-    ObjectPropertyAccessor *get;
-    ObjectPropertyAccessor *set;
-    ObjectPropertyRelease *release;
-};
-
-typedef struct GlobalProperty {
-    const char *driver;
-    const char *property;
-    const char *value;
-    QTAILQ_ENTRY(GlobalProperty) next;
-} GlobalProperty;
-
-/*** Board API.  This should go away once we have a machine config file.  ***/
-
-DeviceState *qdev_create(BusState *bus, const char *name);
-DeviceState *qdev_try_create(BusState *bus, const char *name);
-int qdev_device_help(QemuOpts *opts);
-DeviceState *qdev_device_add(QemuOpts *opts);
-int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
-void qdev_init_nofail(DeviceState *dev);
-void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
-                                 int required_for_version);
-void qdev_unplug(DeviceState *dev, Error **errp);
-void qdev_free(DeviceState *dev);
-int qdev_simple_unplug_cb(DeviceState *dev);
-void qdev_machine_creation_done(void);
-bool qdev_machine_modified(void);
-
-qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
-void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
-
-BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
-
-/*** Device API.  ***/
-
-/* Register device properties.  */
-/* GPIO inputs also double as IRQ sinks.  */
-void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
-void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
-
-BusState *qdev_get_parent_bus(DeviceState *dev);
-
-/*** BUS API. ***/
-
-DeviceState *qdev_find_recursive(BusState *bus, const char *id);
-
-/* Returns 0 to walk children, > 0 to skip walk, < 0 to terminate walk. */
-typedef int (qbus_walkerfn)(BusState *bus, void *opaque);
-typedef int (qdev_walkerfn)(DeviceState *dev, void *opaque);
-
-void qbus_create_inplace(BusState *bus, const char *typename,
-                         DeviceState *parent, const char *name);
-BusState *qbus_create(const char *typename, DeviceState *parent, const char *name);
-/* Returns > 0 if either devfn or busfn skip walk somewhere in cursion,
- *         < 0 if either devfn or busfn terminate walk somewhere in cursion,
- *           0 otherwise. */
-int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
-                       qbus_walkerfn *busfn, void *opaque);
-int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn,
-                       qbus_walkerfn *busfn, void *opaque);
-void qdev_reset_all(DeviceState *dev);
-void qbus_reset_all_fn(void *opaque);
-
-void qbus_free(BusState *bus);
-
-#define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
-
-/* This should go away once we get rid of the NULL bus hack */
-BusState *sysbus_get_default(void);
-
-/*** monitor commands ***/
-
-void do_info_qtree(Monitor *mon);
-void do_info_qdm(Monitor *mon);
-int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
-int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
-
-/*** qdev-properties.c ***/
-
-extern PropertyInfo qdev_prop_bit;
-extern PropertyInfo qdev_prop_uint8;
-extern PropertyInfo qdev_prop_uint16;
-extern PropertyInfo qdev_prop_uint32;
-extern PropertyInfo qdev_prop_int32;
-extern PropertyInfo qdev_prop_uint64;
-extern PropertyInfo qdev_prop_hex8;
-extern PropertyInfo qdev_prop_hex32;
-extern PropertyInfo qdev_prop_hex64;
-extern PropertyInfo qdev_prop_string;
-extern PropertyInfo qdev_prop_chr;
-extern PropertyInfo qdev_prop_ptr;
-extern PropertyInfo qdev_prop_macaddr;
-extern PropertyInfo qdev_prop_losttickpolicy;
-extern PropertyInfo qdev_prop_bios_chs_trans;
-extern PropertyInfo qdev_prop_drive;
-extern PropertyInfo qdev_prop_netdev;
-extern PropertyInfo qdev_prop_vlan;
-extern PropertyInfo qdev_prop_pci_devfn;
-extern PropertyInfo qdev_prop_blocksize;
-extern PropertyInfo qdev_prop_pci_host_devaddr;
-
-#define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
-        .name      = (_name),                                    \
-        .info      = &(_prop),                                   \
-        .offset    = offsetof(_state, _field)                    \
-            + type_check(_type,typeof_field(_state, _field)),    \
-        }
-#define DEFINE_PROP_DEFAULT(_name, _state, _field, _defval, _prop, _type) { \
-        .name      = (_name),                                           \
-        .info      = &(_prop),                                          \
-        .offset    = offsetof(_state, _field)                           \
-            + type_check(_type,typeof_field(_state, _field)),           \
-        .qtype     = QTYPE_QINT,                                        \
-        .defval    = (_type)_defval,                                    \
-        }
-#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) {  \
-        .name      = (_name),                                    \
-        .info      = &(qdev_prop_bit),                           \
-        .bitnr    = (_bit),                                      \
-        .offset    = offsetof(_state, _field)                    \
-            + type_check(uint32_t,typeof_field(_state, _field)), \
-        .qtype     = QTYPE_QBOOL,                                \
-        .defval    = (bool)_defval,                              \
-        }
-
-#define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
-#define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint16, uint16_t)
-#define DEFINE_PROP_UINT32(_n, _s, _f, _d)                      \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint32, uint32_t)
-#define DEFINE_PROP_INT32(_n, _s, _f, _d)                      \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_int32, int32_t)
-#define DEFINE_PROP_UINT64(_n, _s, _f, _d)                      \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint64, uint64_t)
-#define DEFINE_PROP_HEX8(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex8, uint8_t)
-#define DEFINE_PROP_HEX32(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex32, uint32_t)
-#define DEFINE_PROP_HEX64(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
-#define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d)                   \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
-
-#define DEFINE_PROP_PTR(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)
-#define DEFINE_PROP_CHR(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharDriverState*)
-#define DEFINE_PROP_STRING(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
-#define DEFINE_PROP_NETDEV(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_netdev, NetClientState*)
-#define DEFINE_PROP_VLAN(_n, _s, _f)             \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_vlan, NetClientState*)
-#define DEFINE_PROP_DRIVE(_n, _s, _f) \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_drive, BlockDriverState *)
-#define DEFINE_PROP_MACADDR(_n, _s, _f)         \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
-#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
-                        LostTickPolicy)
-#define DEFINE_PROP_BIOS_CHS_TRANS(_n, _s, _f, _d) \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bios_chs_trans, int)
-#define DEFINE_PROP_BLOCKSIZE(_n, _s, _f, _d) \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_blocksize, uint16_t)
-#define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
-    DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
-
-#define DEFINE_PROP_END_OF_LIST()               \
-    {}
-
-/* Set properties between creation and init.  */
-void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
-int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
-void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
-void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
-void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
-void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
-void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value);
-void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value);
-void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value);
-void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value);
-void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value);
-int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value) QEMU_WARN_UNUSED_RESULT;
-void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverState *value);
-void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value);
-void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
-/* FIXME: Remove opaque pointer properties.  */
-void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
-
-void qdev_prop_register_global_list(GlobalProperty *props);
-void qdev_prop_set_globals(DeviceState *dev);
-void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
-                                    Property *prop, const char *value);
-
-char *qdev_get_fw_dev_path(DeviceState *dev);
-
-/**
- * @qdev_property_add_static - add a @Property to a device referencing a
- * field in a struct.
- */
-void qdev_property_add_static(DeviceState *dev, Property *prop, Error **errp);
-
-/**
- * @qdev_machine_init
- *
- * Initialize platform devices before machine init.  This is a hack until full
- * support for composition is added.
- */
-void qdev_machine_init(void);
-
-/**
- * @device_reset
- *
- * Reset a single device (by calling the reset method).
- */
-void device_reset(DeviceState *dev);
-
-const VMStateDescription *qdev_get_vmsd(DeviceState *dev);
-
-const char *qdev_fw_name(DeviceState *dev);
-
-Object *qdev_get_machine(void);
-
-/* FIXME: make this a link<> */
-void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
-
-extern int qdev_hotplug;
-
-char *qdev_get_dev_path(DeviceState *dev);
+#include "hw/hw.h"
+#include "qdev-core.h"
+#include "qdev-properties.h"
+#include "qdev-monitor.h"
 
 #endif
-- 
1.7.11.7

[Qemu-devel] [PATCH 06/24] move I/O-related definitions from qemu-common.h to a new header (qemu-stdio.h)

[Eduardo Habkost]

This will help reduce the qemu-common.h dependency hell.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
--
Changes v1 -> v2:
 - move qemu_open() & qemu_close() to qemu-stdio.h, too
---
 qemu-common.h | 59 ++--------------------------------------------
 qemu-stdio.h  | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 57 deletions(-)
 create mode 100644 qemu-stdio.h

diff --git a/qemu-common.h b/qemu-common.h
index 6441bee..5080382 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -15,6 +15,8 @@
 #include "compiler.h"
 #include "config-host.h"
 
+#include "qemu-stdio.h"
+
 #if defined(__arm__) || defined(__sparc__) || defined(__mips__) || defined(__hppa__) || defined(__ia64__)
 #define WORDS_ALIGNED
 #endif
@@ -58,28 +60,6 @@ typedef struct MigrationParams MigrationParams;
 #include "qemu-os-posix.h"
 #endif
 
-#ifndef O_LARGEFILE
-#define O_LARGEFILE 0
-#endif
-#ifndef O_BINARY
-#define O_BINARY 0
-#endif
-#ifndef MAP_ANONYMOUS
-#define MAP_ANONYMOUS MAP_ANON
-#endif
-#ifndef ENOMEDIUM
-#define ENOMEDIUM ENODEV
-#endif
-#if !defined(ENOTSUP)
-#define ENOTSUP 4096
-#endif
-#if !defined(ECANCELED)
-#define ECANCELED 4097
-#endif
-#ifndef TIME_MAX
-#define TIME_MAX LONG_MAX
-#endif
-
 /* HOST_LONG_BITS is the size of a native pointer in bits. */
 #if UINTPTR_MAX == UINT32_MAX
 # define HOST_LONG_BITS 32
@@ -89,39 +69,6 @@ typedef struct MigrationParams MigrationParams;
 # error Unknown pointer size
 #endif
 
-#ifndef CONFIG_IOVEC
-#define CONFIG_IOVEC
-struct iovec {
-    void *iov_base;
-    size_t iov_len;
-};
-/*
- * Use the same value as Linux for now.
- */
-#define IOV_MAX		1024
-#else
-#include <sys/uio.h>
-#endif
-
-typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
-    GCC_FMT_ATTR(2, 3);
-
-#ifdef _WIN32
-#define fsync _commit
-#if !defined(lseek)
-# define lseek _lseeki64
-#endif
-int qemu_ftruncate64(int, int64_t);
-#if !defined(ftruncate)
-# define ftruncate qemu_ftruncate64
-#endif
-
-static inline char *realpath(const char *path, char *resolved_path)
-{
-    _fullpath(resolved_path, path, _MAX_PATH);
-    return resolved_path;
-}
-#endif
 
 /* icount */
 void configure_icount(const char *option);
@@ -217,8 +164,6 @@ const char *path(const char *pathname);
 
 void *qemu_oom_check(void *ptr);
 
-int qemu_open(const char *name, int flags, ...);
-int qemu_close(int fd);
 ssize_t qemu_write_full(int fd, const void *buf, size_t count)
     QEMU_WARN_UNUSED_RESULT;
 ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
diff --git a/qemu-stdio.h b/qemu-stdio.h
new file mode 100644
index 0000000..b2e8eda
--- /dev/null
+++ b/qemu-stdio.h
@@ -0,0 +1,76 @@
+/* Some basic definitions related to stdio.h or other I/O interfaces
+ */
+#ifndef QEMU_STDIO_H
+#define QEMU_STDIO_H
+
+#include "compiler.h"
+#include "config-host.h"
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/time.h>
+#include <sys/mman.h>
+
+#ifndef O_LARGEFILE
+#define O_LARGEFILE 0
+#endif
+#ifndef O_BINARY
+#define O_BINARY 0
+#endif
+#ifndef MAP_ANONYMOUS
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+#ifndef ENOMEDIUM
+#define ENOMEDIUM ENODEV
+#endif
+#if !defined(ENOTSUP)
+#define ENOTSUP 4096
+#endif
+#if !defined(ECANCELED)
+#define ECANCELED 4097
+#endif
+#ifndef TIME_MAX
+#define TIME_MAX LONG_MAX
+#endif
+
+#ifndef CONFIG_IOVEC
+#define CONFIG_IOVEC
+struct iovec {
+    void *iov_base;
+    size_t iov_len;
+};
+/*
+ * Use the same value as Linux for now.
+ */
+#define IOV_MAX     1024
+#else
+#include <sys/uio.h>
+#endif
+
+typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
+    GCC_FMT_ATTR(2, 3);
+
+#ifdef _WIN32
+#define fsync _commit
+#if !defined(lseek)
+# define lseek _lseeki64
+#endif
+int qemu_ftruncate64(int, int64_t);
+#if !defined(ftruncate)
+# define ftruncate qemu_ftruncate64
+#endif
+
+static inline char *realpath(const char *path, char *resolved_path)
+{
+    _fullpath(resolved_path, path, _MAX_PATH);
+    return resolved_path;
+}
+#endif
+
+int qemu_open(const char *name, int flags, ...);
+int qemu_close(int fd);
+
+#endif /* QEMU_STDIO_H */
-- 
1.7.11.7

[Qemu-devel] [PATCH 07/24] qemu-fsdev-dummy.c: include module.h

[Eduardo Habkost]

module.h is where machine_init() is defined, but qemu-fsdev-dummy.c
doesn't include it.

The header is probably being included by accident because some other
headers are including qemu-common.h, but those headers should eventually
stop including qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 fsdev/qemu-fsdev-dummy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
index 4e700dd..300f275 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -14,6 +14,7 @@
 #include <string.h>
 #include "qemu-fsdev.h"
 #include "qemu-config.h"
+#include "module.h"
 
 int qemu_fsdev_add(QemuOpts *opts)
 {
-- 
1.7.11.7

[Qemu-devel] [PATCH 08/24] vnc-palette.h: include <stdbool.h>

[Eduardo Habkost]

<stdbool.h> is needed for the 'bool' type, used in the header.

The header is probably being included by accident because some other
headers are including qemu-common.h, but those headers should eventually
stop including qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 ui/vnc-palette.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vnc-palette.h b/ui/vnc-palette.h
index 3260885..b82dc5d 100644
--- a/ui/vnc-palette.h
+++ b/ui/vnc-palette.h
@@ -32,6 +32,7 @@
 #include "qlist.h"
 #include "qemu-queue.h"
 #include <stdint.h>
+#include <stdbool.h>
 
 #define VNC_PALETTE_HASH_SIZE 256
 #define VNC_PALETTE_MAX_SIZE  256
-- 
1.7.11.7

[Qemu-devel] [PATCH 09/24] ui/vnc-pallete.c: include headers it needs

[Eduardo Habkost]

Include:
 - <glib.h> for g_malloc0()
 - <string.h> for memset()

Some of those headers were probably being included by accident because
some other headers were including qemu-common.h, but those headers
should eventually stop including qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 ui/vnc-palette.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ui/vnc-palette.c b/ui/vnc-palette.c
index 63d5f64..c130dee 100644
--- a/ui/vnc-palette.c
+++ b/ui/vnc-palette.c
@@ -27,6 +27,8 @@
  */
 
 #include "vnc-palette.h"
+#include <glib.h>
+#include <string.h>
 
 static VncPaletteEntry *palette_find(const VncPalette *palette,
                                      uint32_t color, unsigned int hash)
-- 
1.7.11.7

[Qemu-devel] [PATCH 10/24] qemu-config.h: include headers it needs

[Eduardo Habkost]

Include:
- <stdio.h> for FILE
- qemu-option.h for QemuOptsList

Some of those headers were probably being included by accident because
some other headers were including qemu-common.h, but those headers
should eventually stop including qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qemu-config.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/qemu-config.h b/qemu-config.h
index 5557562..812c4c5 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -1,6 +1,8 @@
 #ifndef QEMU_CONFIG_H
 #define QEMU_CONFIG_H
 
+#include <stdio.h>
+#include "qemu-option.h"
 #include "error.h"
 
 extern QemuOptsList qemu_fsdev_opts;
-- 
1.7.11.7

[Qemu-devel] [PATCH 11/24] qapi/qmp-registry.c: include headers it needs

[Eduardo Habkost]

Include:
- <glib.h> for g_malloc0()
- <string.h> for strcmp()

Some of those headers were probably being included by accident because
some other headers were including qemu-common.h, but those headers
should eventually stop including qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qapi/qmp-registry.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
index 5414613..c2c31b4 100644
--- a/qapi/qmp-registry.c
+++ b/qapi/qmp-registry.c
@@ -12,6 +12,8 @@
  *
  */
 
+#include <glib.h>
+#include <string.h>
 #include "qapi/qmp-core.h"
 
 static QTAILQ_HEAD(QmpCommandList, QmpCommand) qmp_commands =
-- 
1.7.11.7

[Qemu-devel] [PATCH 12/24] qga/channel-posix.c: include headers it needs

[Eduardo Habkost]

Include:
 - <errno.h> for errno
 - <unistd.h> & <fcntl.h> for fcntl()
 - "qemu-stdio.h" for qemu_open()

Some of those headers were probably being included by accident because
some other headers were including qemu-common.h, but those headers
should eventually stop including qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qga/channel-posix.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index d152827..2a68451 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -1,5 +1,9 @@
 #include <glib.h>
 #include <termios.h>
+#include <errno.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include "qemu-stdio.h"
 #include "qemu_socket.h"
 #include "qga/channel.h"
 
-- 
1.7.11.7

[Qemu-devel] [PATCH 13/24] create qemu-types.h for struct typedefs

[Eduardo Habkost]

Instead of keeping all those struct typedefs on qemu-common.h, move it
to a header that can be safely included by other headers, containing
only the struct typedefs and not pulling other dependencies.

Also, move some of the qdev-core.h typedefs to the new file, too, so
other headers don't need to include qdev-core.h only because of
DeviceState and other typedefs.

This will help us remove qemu-common.h dependencies from some headers
later.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 - Rebased against latest qemu.git master
 - New typedefs: QEMUBH, QemuConsole
 - Old typedefs removed: DisplayAllocator, TextConsole, QEMUConsole
---
 hw/qdev-core.h | 11 +----------
 qemu-common.h  | 52 +------------------------------------------------
 qemu-types.h   | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 61 deletions(-)
 create mode 100644 qemu-types.h

diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index fce9e22..494b687 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -3,20 +3,11 @@
 
 #include "qemu-queue.h"
 #include "qemu-option.h"
+#include "qemu-types.h"
 #include "qemu/object.h"
 #include "hw/irq.h"
 #include "error.h"
 
-typedef struct Property Property;
-
-typedef struct PropertyInfo PropertyInfo;
-
-typedef struct CompatProperty CompatProperty;
-
-typedef struct BusState BusState;
-
-typedef struct BusClass BusClass;
-
 enum DevState {
     DEV_STATE_CREATED = 1,
     DEV_STATE_INITIALIZED,
diff --git a/qemu-common.h b/qemu-common.h
index 5080382..1b4a12c 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -16,6 +16,7 @@
 #include "config-host.h"
 
 #include "qemu-stdio.h"
+#include "qemu-types.h"
 
 #if defined(__arm__) || defined(__sparc__) || defined(__mips__) || defined(__hppa__) || defined(__ia64__)
 #define WORDS_ALIGNED
@@ -23,15 +24,6 @@
 
 #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
 
-typedef struct QEMUTimer QEMUTimer;
-typedef struct QEMUFile QEMUFile;
-typedef struct QEMUBH QEMUBH;
-typedef struct DeviceState DeviceState;
-
-struct Monitor;
-typedef struct Monitor Monitor;
-typedef struct MigrationParams MigrationParams;
-
 /* we put basic includes here to avoid repeating them in device drivers */
 #include <stdlib.h>
 #include <stdio.h>
@@ -205,48 +197,6 @@ struct ParallelIOArg {
 
 typedef int (*DMA_transfer_handler) (void *opaque, int nchan, int pos, int size);
 
-/* A load of opaque types so that device init declarations don't have to
-   pull in all the real definitions.  */
-typedef struct NICInfo NICInfo;
-typedef struct HCIInfo HCIInfo;
-typedef struct AudioState AudioState;
-typedef struct BlockDriverState BlockDriverState;
-typedef struct DriveInfo DriveInfo;
-typedef struct DisplayState DisplayState;
-typedef struct DisplayChangeListener DisplayChangeListener;
-typedef struct DisplaySurface DisplaySurface;
-typedef struct PixelFormat PixelFormat;
-typedef struct QemuConsole QemuConsole;
-typedef struct CharDriverState CharDriverState;
-typedef struct MACAddr MACAddr;
-typedef struct NetClientState NetClientState;
-typedef struct i2c_bus i2c_bus;
-typedef struct ISABus ISABus;
-typedef struct ISADevice ISADevice;
-typedef struct SMBusDevice SMBusDevice;
-typedef struct PCIHostState PCIHostState;
-typedef struct PCIExpressHost PCIExpressHost;
-typedef struct PCIBus PCIBus;
-typedef struct PCIDevice PCIDevice;
-typedef struct PCIExpressDevice PCIExpressDevice;
-typedef struct PCIBridge PCIBridge;
-typedef struct PCIEAERMsg PCIEAERMsg;
-typedef struct PCIEAERLog PCIEAERLog;
-typedef struct PCIEAERErr PCIEAERErr;
-typedef struct PCIEPort PCIEPort;
-typedef struct PCIESlot PCIESlot;
-typedef struct MSIMessage MSIMessage;
-typedef struct SerialState SerialState;
-typedef struct PCMCIACardState PCMCIACardState;
-typedef struct MouseTransformInfo MouseTransformInfo;
-typedef struct uWireSlave uWireSlave;
-typedef struct I2SCodec I2SCodec;
-typedef struct SSIBus SSIBus;
-typedef struct EventNotifier EventNotifier;
-typedef struct VirtIODevice VirtIODevice;
-typedef struct QEMUSGList QEMUSGList;
-typedef struct SHPCDevice SHPCDevice;
-
 typedef uint64_t pcibus_t;
 
 typedef enum LostTickPolicy {
diff --git a/qemu-types.h b/qemu-types.h
new file mode 100644
index 0000000..fd532a2
--- /dev/null
+++ b/qemu-types.h
@@ -0,0 +1,61 @@
+#ifndef QEMU_TYPEDEFS_H
+#define QEMU_TYPEDEFS_H
+
+/* A load of opaque types so that device init declarations don't have to
+   pull in all the real definitions.  */
+typedef struct QEMUTimer QEMUTimer;
+typedef struct QEMUFile QEMUFile;
+typedef struct QEMUBH QEMUBH;
+
+struct Monitor;
+typedef struct Monitor Monitor;
+typedef struct MigrationParams MigrationParams;
+
+typedef struct Property Property;
+typedef struct PropertyInfo PropertyInfo;
+typedef struct CompatProperty CompatProperty;
+typedef struct DeviceState DeviceState;
+typedef struct BusState BusState;
+typedef struct BusClass BusClass;
+
+typedef struct NICInfo NICInfo;
+typedef struct HCIInfo HCIInfo;
+typedef struct AudioState AudioState;
+typedef struct BlockDriverState BlockDriverState;
+typedef struct DriveInfo DriveInfo;
+typedef struct DisplayState DisplayState;
+typedef struct DisplayChangeListener DisplayChangeListener;
+typedef struct DisplaySurface DisplaySurface;
+typedef struct PixelFormat PixelFormat;
+typedef struct QemuConsole QemuConsole;
+typedef struct CharDriverState CharDriverState;
+typedef struct MACAddr MACAddr;
+typedef struct NetClientState NetClientState;
+typedef struct i2c_bus i2c_bus;
+typedef struct ISABus ISABus;
+typedef struct ISADevice ISADevice;
+typedef struct SMBusDevice SMBusDevice;
+typedef struct PCIHostState PCIHostState;
+typedef struct PCIExpressHost PCIExpressHost;
+typedef struct PCIBus PCIBus;
+typedef struct PCIDevice PCIDevice;
+typedef struct PCIExpressDevice PCIExpressDevice;
+typedef struct PCIBridge PCIBridge;
+typedef struct PCIEAERMsg PCIEAERMsg;
+typedef struct PCIEAERLog PCIEAERLog;
+typedef struct PCIEAERErr PCIEAERErr;
+typedef struct PCIEPort PCIEPort;
+typedef struct PCIESlot PCIESlot;
+typedef struct MSIMessage MSIMessage;
+typedef struct SerialState SerialState;
+typedef struct PCMCIACardState PCMCIACardState;
+typedef struct MouseTransformInfo MouseTransformInfo;
+typedef struct uWireSlave uWireSlave;
+typedef struct I2SCodec I2SCodec;
+typedef struct SSIBus SSIBus;
+typedef struct EventNotifier EventNotifier;
+typedef struct VirtIODevice VirtIODevice;
+typedef struct QEMUSGList QEMUSGList;
+typedef struct SHPCDevice SHPCDevice;
+
+#endif /* QEMU_TYPEDEFS_H */
-- 
1.7.11.7

[Qemu-devel] [PATCH 14/24] sysemu.h: include qemu-types.h instead of qemu-common.h

[Eduardo Habkost]

It just needs the Monitor and DeviceState typedefs, so it doesn't need
all of qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 sysemu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysemu.h b/sysemu.h
index f5ac664..ab1ef8b 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -2,7 +2,7 @@
 #define SYSEMU_H
 /* Misc. things related to the system emulator.  */
 
-#include "qemu-common.h"
+#include "qemu-types.h"
 #include "qemu-option.h"
 #include "qemu-queue.h"
 #include "qemu-timer.h"
-- 
1.7.11.7

[Qemu-devel] [PATCH 15/24] qlist.h: do not include qemu-common.h

[Eduardo Habkost]

I don't know why it was including it, as I don't see any code that
depends on anything from qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qlist.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/qlist.h b/qlist.h
index ae776f9..7408947 100644
--- a/qlist.h
+++ b/qlist.h
@@ -15,7 +15,6 @@
 
 #include "qobject.h"
 #include "qemu-queue.h"
-#include "qemu-common.h"
 #include "qemu-queue.h"
 
 typedef struct QListEntry {
-- 
1.7.11.7

[Qemu-devel] [PATCH 16/24] qapi-types.h: don't include qemu-common.h

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

needed to prevent build breakage when CPU becomes a child of DeviceState

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
[ehabkost: include <stdbool.h> too]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Yes, there is "changelog" data before the "---" mark, but I believe that
in this case they are important to indicate authorship and the scope of
the Signed-off-by lines (so they need to get into the git commit
message).

Changes v1 (from Igor) -> v2 (from Eduardo):
 - Include <stdboo.h> too

Changes v2 -> v3 (from Eduardo):
 - Only commit description changes
---
 scripts/qapi-types.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 1b84834..6bc2391 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -273,7 +273,8 @@ fdecl.write(mcgen('''
 #ifndef %(guard)s
 #define %(guard)s
 
-#include "qemu-common.h"
+#include <stdbool.h>
+#include <stdint.h>
 
 ''',
                   guard=guardname(h_file)))
-- 
1.7.11.7

[Qemu-devel] [PATCH 17/24] qdev-properties.c: add copyright/license information

[Eduardo Habkost]

Add copyright and license information, based on git log,
and discussion at:

 http://article.gmane.org/gmane.comp.emulators.qemu/176405
 Message-ID: <20121017201414.GA5665@otherpad.lan.raisama.net>

The list of Copyright owners is huge, but I didn't think I really could
leave any of the authors out of the list.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
The only reason I am doing this is because:
 - I am going to mvoe some of the qdev-properties.c file to another file
 - I need to add a copyright/license header to the new file.
   See http://article.gmane.org/gmane.comp.emulators.qemu/176367
 - So, I need to document the copyright/license of the current
   qdev-properties.c code

I would really love to avoid this whole archaeology/legalese work,
because I am not a lawyer. I am only doing that because of the "new
files should have a copyright header" requirement.

Cc: Amit Shah <amit.shah@redhat.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Christoph Egger <Christoph.Egger@amd.com>
Cc: David 'Digit' Turner <digit@google.com>
Cc: Donald Dutile <ddutile@redhat.com>
Cc: dunrong huang <riegamaths@gmail.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Isaku Yamahata <yamahata@valinux.co.jp>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Jan Kiszka <jan.kiszka@web.de>
Cc: Juan Quintela <quintela@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Kusanagi Kouichi <slash@ac.auone-net.jp>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Stefan Weil <sw@weilnetz.de>
Cc: Stefan Weil <weil@mail.berlios.de>
Cc: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 hw/qdev-properties.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 81d901c..ff5aa0a 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1,3 +1,33 @@
+/*
+ * qdev property parsing and global properties
+ *
+ * Copyright (c) 2009-2010 Gerd Hoffmann <kraxel@redhat.com>
+ * Copyright (c) 2009 Christoph Egger <Christoph.Egger@amd.com>
+ * Copyright (c) 2009-2010 Blue Swirl <blauwirbel@gmail.com>
+ * Copyright (c) 2009 Juan Quintela <quintela@redhat.com>
+ * Copyright (c) 2010 Michael S. Tsirkin <mst@redhat.com>
+ * Copyright (c) 2010,2012 Stefan Weil <weil@mail.berlios.de>
+ * Copyright (c) 2010-2012 Markus Armbruster <armbru@redhat.com>
+ * Copyright (c) 2010 Kevin Wolf <kwolf@redhat.com>
+ * Copyright (c) 2010 Isaku Yamahata <yamahata@valinux.co.jp>
+ * Copyright (c) 2011 David 'Digit' Turner <digit@google.com>
+ * Copyright (c) 2011-2012 Amit Shah <amit.shah@redhat.com>
+ * Copyright (c) 2011 Kusanagi Kouichi <slash@ac.auone-net.jp>
+ * Copyright (c) 2011-2012 Anthony Liguori <aliguori@us.ibm.com>
+ * Copyright (c) 2011 Donald Dutile <ddutile@redhat.com>
+ * Copyright (c) 2011-2012 Jan Kiszka <jan.kiszka@siemens.com>
+ * Copyright (c) 2011-2012 Paolo Bonzini <pbonzini@redhat.com>
+ * Copyright (c) 2012 Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
+ * Copyright (c) 2012 dunrong huang <riegamaths@gmail.com>
+ * Copyright (c) 2012 Michael Roth <mdroth@linux.vnet.ibm.com>
+ * Copyright (c) 2012 Anthony PERARD <anthony.perard@citrix.com>
+ * Copyright (c) 2012 Christian Borntraeger <borntraeger@de.ibm.com>
+ * Copyright (c) 2012 Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
 #include "net.h"
 #include "qdev.h"
 #include "qerror.h"
-- 
1.7.11.7

[Qemu-devel] [PATCH 18/24] qdev: qdev_create(): use error_report() instead of hw_error()

[Eduardo Habkost]

hw_error() is specific for fatal hardware emulation errors, not for
internal errors related to the qdev object/class abstraction or object
initialization.

Replace it with an error_report() call, followed by abort().

This will also help reduce dependencies of the qdev code (as hw_error()
is from cpus.o, and depends on the CPU list from exec.o).

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/qdev.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 7ddcd24..2cc6434 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -109,10 +109,12 @@ DeviceState *qdev_create(BusState *bus, const char *name)
     dev = qdev_try_create(bus, name);
     if (!dev) {
         if (bus) {
-            hw_error("Unknown device '%s' for bus '%s'\n", name,
-                     object_get_typename(OBJECT(bus)));
+            error_report("Unknown device '%s' for bus '%s'\n", name,
+                         object_get_typename(OBJECT(bus)));
+            abort();
         } else {
-            hw_error("Unknown device '%s' for default sysbus\n", name);
+            error_report("Unknown device '%s' for default sysbus\n", name);
+            abort();
         }
     }
 
-- 
1.7.11.7

[Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c

[Eduardo Habkost]

The core qdev code uses the reset handler list from vl.c, so move
qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
to qdev.c.

The function declarations were moved to a new qdev-reset.h file, that is
included by hw.h to keep compatibility, so we don't need to change all
files that use qemu_register_reset().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/hw.h         |  6 +-----
 hw/qdev-reset.h | 11 +++++++++++
 hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h       |  1 +
 sysemu.h        |  1 -
 vl.c            | 40 ----------------------------------------
 6 files changed, 54 insertions(+), 46 deletions(-)
 create mode 100644 hw/qdev-reset.h

diff --git a/hw/hw.h b/hw/hw.h
index f530f6f..622a157 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -14,6 +14,7 @@
 #include "qemu-file.h"
 #include "vmstate.h"
 #include "qemu-log.h"
+#include "qdev-reset.h"
 
 #ifdef NEED_CPU_H
 #if TARGET_LONG_BITS == 64
@@ -37,11 +38,6 @@
 #endif
 #endif
 
-typedef void QEMUResetHandler(void *opaque);
-
-void qemu_register_reset(QEMUResetHandler *func, void *opaque);
-void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
-
 /* handler to set the boot_device order for a specific type of QEMUMachine */
 /* return 0 if success */
 typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
new file mode 100644
index 0000000..40ae9a5
--- /dev/null
+++ b/hw/qdev-reset.h
@@ -0,0 +1,11 @@
+/* Device reset handler function registration, used by qdev */
+#ifndef QDEV_RESET_H
+#define QDEV_RESET_H
+
+typedef void QEMUResetHandler(void *opaque);
+
+void qemu_register_reset(QEMUResetHandler *func, void *opaque);
+void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
+void qemu_devices_reset(void);
+
+#endif /* QDEV_RESET_H */
diff --git a/hw/qdev.c b/hw/qdev.c
index 2cc6434..c242097 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -35,6 +35,47 @@ int qdev_hotplug = 0;
 static bool qdev_hot_added = false;
 static bool qdev_hot_removed = false;
 
+typedef struct QEMUResetEntry {
+    QTAILQ_ENTRY(QEMUResetEntry) entry;
+    QEMUResetHandler *func;
+    void *opaque;
+} QEMUResetEntry;
+
+static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
+    QTAILQ_HEAD_INITIALIZER(reset_handlers);
+
+void qemu_register_reset(QEMUResetHandler *func, void *opaque)
+{
+    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
+
+    re->func = func;
+    re->opaque = opaque;
+    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
+}
+
+void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
+{
+    QEMUResetEntry *re;
+
+    QTAILQ_FOREACH(re, &reset_handlers, entry) {
+        if (re->func == func && re->opaque == opaque) {
+            QTAILQ_REMOVE(&reset_handlers, re, entry);
+            g_free(re);
+            return;
+        }
+    }
+}
+
+void qemu_devices_reset(void)
+{
+    QEMUResetEntry *re, *nre;
+
+    /* reset all devices */
+    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
+        re->func(re->opaque);
+    }
+}
+
 const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
diff --git a/hw/qdev.h b/hw/qdev.h
index 365b8d6..2487b3b 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -5,5 +5,6 @@
 #include "qdev-core.h"
 #include "qdev-properties.h"
 #include "qdev-monitor.h"
+#include "qdev-reset.h"
 
 #endif
diff --git a/sysemu.h b/sysemu.h
index ab1ef8b..51f19cc 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -57,7 +57,6 @@ void qemu_system_vmstop_request(RunState reason);
 int qemu_shutdown_requested_get(void);
 int qemu_reset_requested_get(void);
 void qemu_system_killed(int signal, pid_t pid);
-void qemu_devices_reset(void);
 void qemu_system_reset(bool report);
 
 void qemu_add_exit_notifier(Notifier *notify);
diff --git a/vl.c b/vl.c
index 4f03a72..c7448a2 100644
--- a/vl.c
+++ b/vl.c
@@ -1456,14 +1456,6 @@ void vm_start(void)
 
 /* reset/shutdown handler */
 
-typedef struct QEMUResetEntry {
-    QTAILQ_ENTRY(QEMUResetEntry) entry;
-    QEMUResetHandler *func;
-    void *opaque;
-} QEMUResetEntry;
-
-static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
-    QTAILQ_HEAD_INITIALIZER(reset_handlers);
 static int reset_requested;
 static int shutdown_requested, shutdown_signal = -1;
 static pid_t shutdown_pid;
@@ -1560,38 +1552,6 @@ static bool qemu_vmstop_requested(RunState *r)
     return false;
 }
 
-void qemu_register_reset(QEMUResetHandler *func, void *opaque)
-{
-    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
-
-    re->func = func;
-    re->opaque = opaque;
-    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
-}
-
-void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
-{
-    QEMUResetEntry *re;
-
-    QTAILQ_FOREACH(re, &reset_handlers, entry) {
-        if (re->func == func && re->opaque == opaque) {
-            QTAILQ_REMOVE(&reset_handlers, re, entry);
-            g_free(re);
-            return;
-        }
-    }
-}
-
-void qemu_devices_reset(void)
-{
-    QEMUResetEntry *re, *nre;
-
-    /* reset all devices */
-    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
-        re->func(re->opaque);
-    }
-}
-
 void qemu_system_reset(bool report)
 {
     if (current_machine && current_machine->reset) {
-- 
1.7.11.7

[Qemu-devel] [PATCH 20/24] qdev: add weak aliases for vmstate handling on qdev.c

[Eduardo Habkost]

Add weak alias to vmstate stub functions on qdev.c, so that qdev.o can
be used without savevm.o when vmstate support is not necessary (i.e. by
*-user).

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Originally submitted as:
  Subject: qdev-core: isolate vmstate handling into separate functions

Changes v1 -> v2:
 - Add GCC_WEAK_DECL to function declarations

Changes v2 -> v3:
 - Make vmstate_register_with_alias_id()/vmstate_unregister()
   have GCC_WEAK versions, instead of creating a new function
 - Kept qdev_get_vmsd() inside qdev.c

Changess v3 -> v4:
 - Use the new QEMU_WEAK_ALIAS system instead of GCC_WEAK
---
 hw/qdev.c | 24 ++++++++++++++++++++++++
 vmstate.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/hw/qdev.c b/hw/qdev.c
index c242097..94ae1cd 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -76,6 +76,30 @@ void qemu_devices_reset(void)
     }
 }
 
+/* vmstate register/unregister:
+ *
+ * The real implementations are on qdev-system.c and savevm.c.
+ * Those are weak symbols used by *-user.
+ */
+
+static int stub_vmstate_register_with_alias_id(DeviceState *dev,
+                                               int instance_id,
+                                               const VMStateDescription *vmsd,
+                                               void *base, int alias_id,
+                                               int required_for_version)
+{
+    return 0;
+}
+QEMU_WEAK_ALIAS(vmstate_register_with_alias_id,
+                stub_vmstate_register_with_alias_id);
+
+static void stub_vmstate_unregister(DeviceState *dev,
+                                    const VMStateDescription *vmsd,
+                                    void *opaque)
+{
+}
+QEMU_WEAK_ALIAS(vmstate_unregister, stub_vmstate_unregister);
+
 const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
diff --git a/vmstate.h b/vmstate.h
index 623af0a..114c994 100644
--- a/vmstate.h
+++ b/vmstate.h
@@ -624,6 +624,8 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                         void *opaque);
 int vmstate_register(DeviceState *dev, int instance_id,
                      const VMStateDescription *vmsd, void *base);
+
+/* The following functions have weak aliases on qdev-core, for *-user: */
 int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
                                    const VMStateDescription *vmsd,
                                    void *base, int alias_id,
-- 
1.7.11.7

[Qemu-devel] [PATCH 21/24] qdev: add weak alias to sysbus_get_default() on qdev.c

[Eduardo Habkost]

This version will be used on cases where sysbus.c is not compiled in
(e.g. *-user).

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 - Use the new QEMU_WEAK_ALIAS mechanism, instead of GCC_WEAK
---
 hw/qdev-core.h |  5 ++++-
 hw/qdev.c      | 13 +++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/qdev-core.h b/hw/qdev-core.h
index 494b687..b64daf0 100644
--- a/hw/qdev-core.h
+++ b/hw/qdev-core.h
@@ -193,7 +193,10 @@ void qbus_free(BusState *bus);
 
 #define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
 
-/* This should go away once we get rid of the NULL bus hack */
+/* This should go away once we get rid of the NULL bus hack.
+ * This has a weak alias for a stub function on qdev.c (for *-user, that
+ * doesn't have a main system bus).
+ */
 BusState *sysbus_get_default(void);
 
 char *qdev_get_fw_dev_path(DeviceState *dev);
diff --git a/hw/qdev.c b/hw/qdev.c
index 94ae1cd..0702659 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -100,6 +100,19 @@ static void stub_vmstate_unregister(DeviceState *dev,
 }
 QEMU_WEAK_ALIAS(vmstate_unregister, stub_vmstate_unregister);
 
+
+/* sysbus stub functions
+ *
+ * The real implementation is on sysbus.c, but the stub functions will be used
+ * on cases where sysbus.c is not compiled in (e.g. *-user).
+ */
+
+static BusState *sysbus_get_none(void)
+{
+    return NULL;
+}
+QEMU_WEAK_ALIAS(sysbus_get_default, sysbus_get_none);
+
 const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
-- 
1.7.11.7

[Qemu-devel] [PATCH 22/24] qdev-properties.c: separate core from the code used only by qemu-system-*

[Eduardo Habkost]

This separates the qdev properties code in two parts:
 - qdev-properties.c, that contains most of the qdev properties code;
 - qdev-properties-system.c for code specific for qemu-system-*,
   containing:
   - Property types: drive, chr, netdev, vlan, that depend on code that
     won't be included on *-user
   - qemu_add_globals(), that depends on qemu-config.o.

This change should help on two things:
 - Allowing DeviceState to be used by *-user without pulling
   dependencies that are specific for qemu-system-*;
 - Writing qdev unit tests without pulling too many dependencies.

The copyright/license header for the new file is directly copied from
qdev-properties.c.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
[imammedo: keep qdev_get_child_bus() in hw/qdev.c]
[imammedo: put qdev_set_nic_properties() in hw/qdev-properties-system.c]
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
[ehabkost: updated the qdev_init_gpio_in() code on qdev-system.c to current
 version]
[ehabkost: added copyright/license information to new qdev*-system.c files]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Yes, there is "changelog" data before the "---" mark, but I believe that
in this case they are important to indicate authorship and the scope of
the Signed-off-by lines (so they need to get into the git commit
message).

Detailed changelog:

Changes v1 (ehabkost) -> v2 (imammedo):
 - keep qdev_get_child_bus() in hw/qdev.c
 - put qdev_set_nic_properties() in hw/qdev-properties-system.c

Changes v2 -> v3 (ehabkost):
 - updated the qdev_init_gpio_in() code on qdev-system.c to current
   version

Changes v3 -> v4 (ehabkost):
 - Added copyright/license information to qdev-properties-system.c
   (based on copyright/license of qdev-properties.c)
 - Whitespace change at the end of qdev-properties.c
 - Don't create qdev-system.c, now we can keep the qdev.c code as-is
   as the qdev.c dependencies were reduced
 - Rewrite patch description
---
 hw/Makefile.objs            |   1 +
 hw/qdev-properties-system.c | 371 ++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev-properties.c        | 321 +-------------------------------------
 hw/qdev-properties.h        |   1 +
 hw/qdev.c                   |  13 --
 5 files changed, 374 insertions(+), 333 deletions(-)
 create mode 100644 hw/qdev-properties-system.c

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index af4ab0c..4833b90 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -182,6 +182,7 @@ common-obj-y += bt.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o
 common-obj-y += bt-hci-csr.o
 common-obj-y += msmouse.o ps2.o
 common-obj-y += qdev.o qdev-properties.o qdev-monitor.o
+common-obj-y += qdev-properties-system.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 
 # xen backend driver support
diff --git a/hw/qdev-properties-system.c b/hw/qdev-properties-system.c
new file mode 100644
index 0000000..d7ea4e3
--- /dev/null
+++ b/hw/qdev-properties-system.c
@@ -0,0 +1,371 @@
+/*
+ * qdev property parsing and global properties
+ * (parts specific for qemu-system-*)
+ *
+ * Copyright (c) 2009-2010 Gerd Hoffmann <kraxel@redhat.com>
+ * Copyright (c) 2009 Christoph Egger <Christoph.Egger@amd.com>
+ * Copyright (c) 2009-2010 Blue Swirl <blauwirbel@gmail.com>
+ * Copyright (c) 2009 Juan Quintela <quintela@redhat.com>
+ * Copyright (c) 2010 Michael S. Tsirkin <mst@redhat.com>
+ * Copyright (c) 2010,2012 Stefan Weil <weil@mail.berlios.de>
+ * Copyright (c) 2010-2012 Markus Armbruster <armbru@redhat.com>
+ * Copyright (c) 2010 Kevin Wolf <kwolf@redhat.com>
+ * Copyright (c) 2010 Isaku Yamahata <yamahata@valinux.co.jp>
+ * Copyright (c) 2011 David 'Digit' Turner <digit@google.com>
+ * Copyright (c) 2011-2012 Amit Shah <amit.shah@redhat.com>
+ * Copyright (c) 2011 Kusanagi Kouichi <slash@ac.auone-net.jp>
+ * Copyright (c) 2011-2012 Anthony Liguori <aliguori@us.ibm.com>
+ * Copyright (c) 2011 Donald Dutile <ddutile@redhat.com>
+ * Copyright (c) 2011-2012 Jan Kiszka <jan.kiszka@siemens.com>
+ * Copyright (c) 2011-2012 Paolo Bonzini <pbonzini@redhat.com>
+ * Copyright (c) 2012 Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
+ * Copyright (c) 2012 dunrong huang <riegamaths@gmail.com>
+ * Copyright (c) 2012 Michael Roth <mdroth@linux.vnet.ibm.com>
+ * Copyright (c) 2012 Anthony PERARD <anthony.perard@citrix.com>
+ * Copyright (c) 2012 Christian Borntraeger <borntraeger@de.ibm.com>
+ * Copyright (c) 2012 Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "net.h"
+#include "qdev.h"
+#include "qerror.h"
+#include "blockdev.h"
+#include "hw/block-common.h"
+#include "net/hub.h"
+#include "qapi/qapi-visit-core.h"
+
+static void get_pointer(Object *obj, Visitor *v, Property *prop,
+                        const char *(*print)(void *ptr),
+                        const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    void **ptr = qdev_get_prop_ptr(dev, prop);
+    char *p;
+
+    p = (char *) (*ptr ? print(*ptr) : "");
+    visit_type_str(v, &p, name, errp);
+}
+
+static void set_pointer(Object *obj, Visitor *v, Property *prop,
+                        int (*parse)(DeviceState *dev, const char *str,
+                                     void **ptr),
+                        const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Error *local_err = NULL;
+    void **ptr = qdev_get_prop_ptr(dev, prop);
+    char *str;
+    int ret;
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    visit_type_str(v, &str, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    if (!*str) {
+        g_free(str);
+        *ptr = NULL;
+        return;
+    }
+    ret = parse(dev, str, ptr);
+    error_set_from_qdev_prop_error(errp, ret, dev, prop, str);
+    g_free(str);
+}
+
+/* --- drive --- */
+
+static int parse_drive(DeviceState *dev, const char *str, void **ptr)
+{
+    BlockDriverState *bs;
+
+    bs = bdrv_find(str);
+    if (bs == NULL)
+        return -ENOENT;
+    if (bdrv_attach_dev(bs, dev) < 0)
+        return -EEXIST;
+    *ptr = bs;
+    return 0;
+}
+
+static void release_drive(Object *obj, const char *name, void *opaque)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (*ptr) {
+        bdrv_detach_dev(*ptr, dev);
+        blockdev_auto_del(*ptr);
+    }
+}
+
+static const char *print_drive(void *ptr)
+{
+    return bdrv_get_device_name(ptr);
+}
+
+static void get_drive(Object *obj, Visitor *v, void *opaque,
+                      const char *name, Error **errp)
+{
+    get_pointer(obj, v, opaque, print_drive, name, errp);
+}
+
+static void set_drive(Object *obj, Visitor *v, void *opaque,
+                      const char *name, Error **errp)
+{
+    set_pointer(obj, v, opaque, parse_drive, name, errp);
+}
+
+PropertyInfo qdev_prop_drive = {
+    .name  = "drive",
+    .get   = get_drive,
+    .set   = set_drive,
+    .release = release_drive,
+};
+
+/* --- character device --- */
+
+static int parse_chr(DeviceState *dev, const char *str, void **ptr)
+{
+    CharDriverState *chr = qemu_chr_find(str);
+    if (chr == NULL) {
+        return -ENOENT;
+    }
+    if (chr->avail_connections < 1) {
+        return -EEXIST;
+    }
+    *ptr = chr;
+    --chr->avail_connections;
+    return 0;
+}
+
+static void release_chr(Object *obj, const char *name, void *opaque)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (*ptr) {
+        qemu_chr_add_handlers(*ptr, NULL, NULL, NULL, NULL);
+    }
+}
+
+
+static const char *print_chr(void *ptr)
+{
+    CharDriverState *chr = ptr;
+
+    return chr->label ? chr->label : "";
+}
+
+static void get_chr(Object *obj, Visitor *v, void *opaque,
+                    const char *name, Error **errp)
+{
+    get_pointer(obj, v, opaque, print_chr, name, errp);
+}
+
+static void set_chr(Object *obj, Visitor *v, void *opaque,
+                    const char *name, Error **errp)
+{
+    set_pointer(obj, v, opaque, parse_chr, name, errp);
+}
+
+PropertyInfo qdev_prop_chr = {
+    .name  = "chr",
+    .get   = get_chr,
+    .set   = set_chr,
+    .release = release_chr,
+};
+
+/* --- netdev device --- */
+
+static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
+{
+    NetClientState *netdev = qemu_find_netdev(str);
+
+    if (netdev == NULL) {
+        return -ENOENT;
+    }
+    if (netdev->peer) {
+        return -EEXIST;
+    }
+    *ptr = netdev;
+    return 0;
+}
+
+static const char *print_netdev(void *ptr)
+{
+    NetClientState *netdev = ptr;
+
+    return netdev->name ? netdev->name : "";
+}
+
+static void get_netdev(Object *obj, Visitor *v, void *opaque,
+                       const char *name, Error **errp)
+{
+    get_pointer(obj, v, opaque, print_netdev, name, errp);
+}
+
+static void set_netdev(Object *obj, Visitor *v, void *opaque,
+                       const char *name, Error **errp)
+{
+    set_pointer(obj, v, opaque, parse_netdev, name, errp);
+}
+
+PropertyInfo qdev_prop_netdev = {
+    .name  = "netdev",
+    .get   = get_netdev,
+    .set   = set_netdev,
+};
+
+void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
+{
+    qdev_prop_set_macaddr(dev, "mac", nd->macaddr.a);
+    if (nd->netdev)
+        qdev_prop_set_netdev(dev, "netdev", nd->netdev);
+    if (nd->nvectors != DEV_NVECTORS_UNSPECIFIED &&
+        object_property_find(OBJECT(dev), "vectors", NULL)) {
+        qdev_prop_set_uint32(dev, "vectors", nd->nvectors);
+    }
+    nd->instantiated = 1;
+}
+
+/* --- vlan --- */
+
+static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (*ptr) {
+        int id;
+        if (!net_hub_id_for_client(*ptr, &id)) {
+            return snprintf(dest, len, "%d", id);
+        }
+    }
+
+    return snprintf(dest, len, "<null>");
+}
+
+static void get_vlan(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
+    int32_t id = -1;
+
+    if (*ptr) {
+        int hub_id;
+        if (!net_hub_id_for_client(*ptr, &hub_id)) {
+            id = hub_id;
+        }
+    }
+
+    visit_type_int32(v, &id, name, errp);
+}
+
+static void set_vlan(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
+    Error *local_err = NULL;
+    int32_t id;
+    NetClientState *hubport;
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    visit_type_int32(v, &id, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    if (id == -1) {
+        *ptr = NULL;
+        return;
+    }
+
+    hubport = net_hub_port_find(id);
+    if (!hubport) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  name, prop->info->name);
+        return;
+    }
+    *ptr = hubport;
+}
+
+PropertyInfo qdev_prop_vlan = {
+    .name  = "vlan",
+    .print = print_vlan,
+    .get   = get_vlan,
+    .set   = set_vlan,
+};
+
+
+int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value)
+{
+    Error *errp = NULL;
+    const char *bdrv_name = value ? bdrv_get_device_name(value) : "";
+    object_property_set_str(OBJECT(dev), bdrv_name,
+                            name, &errp);
+    if (errp) {
+        qerror_report_err(errp);
+        error_free(errp);
+        return -1;
+    }
+    return 0;
+}
+
+void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverState *value)
+{
+    if (qdev_prop_set_drive(dev, name, value) < 0) {
+        exit(1);
+    }
+}
+
+void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value)
+{
+    Error *errp = NULL;
+    assert(!value || value->label);
+    object_property_set_str(OBJECT(dev),
+                            value ? value->label : "", name, &errp);
+    assert_no_error(errp);
+}
+
+void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value)
+{
+    Error *errp = NULL;
+    assert(!value || value->name);
+    object_property_set_str(OBJECT(dev),
+                            value ? value->name : "", name, &errp);
+    assert_no_error(errp);
+}
+
+static int qdev_add_one_global(QemuOpts *opts, void *opaque)
+{
+    GlobalProperty *g;
+
+    g = g_malloc0(sizeof(*g));
+    g->driver   = qemu_opt_get(opts, "driver");
+    g->property = qemu_opt_get(opts, "property");
+    g->value    = qemu_opt_get(opts, "value");
+    qdev_prop_register_global(g);
+    return 0;
+}
+
+void qemu_add_globals(void)
+{
+    qemu_opts_foreach(qemu_find_opts("global"), qdev_add_one_global, NULL, 0);
+}
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index ff5aa0a..d363971 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -43,49 +43,6 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
     return ptr;
 }
 
-static void get_pointer(Object *obj, Visitor *v, Property *prop,
-                        const char *(*print)(void *ptr),
-                        const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    void **ptr = qdev_get_prop_ptr(dev, prop);
-    char *p;
-
-    p = (char *) (*ptr ? print(*ptr) : "");
-    visit_type_str(v, &p, name, errp);
-}
-
-static void set_pointer(Object *obj, Visitor *v, Property *prop,
-                        int (*parse)(DeviceState *dev, const char *str,
-                                     void **ptr),
-                        const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Error *local_err = NULL;
-    void **ptr = qdev_get_prop_ptr(dev, prop);
-    char *str;
-    int ret;
-
-    if (dev->state != DEV_STATE_CREATED) {
-        error_set(errp, QERR_PERMISSION_DENIED);
-        return;
-    }
-
-    visit_type_str(v, &str, name, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-    if (!*str) {
-        g_free(str);
-        *ptr = NULL;
-        return;
-    }
-    ret = parse(dev, str, ptr);
-    error_set_from_qdev_prop_error(errp, ret, dev, prop, str);
-    g_free(str);
-}
-
 static void get_enum(Object *obj, Visitor *v, void *opaque,
                      const char *name, Error **errp)
 {
@@ -506,227 +463,6 @@ PropertyInfo qdev_prop_string = {
     .set   = set_string,
 };
 
-/* --- drive --- */
-
-static int parse_drive(DeviceState *dev, const char *str, void **ptr)
-{
-    BlockDriverState *bs;
-
-    bs = bdrv_find(str);
-    if (bs == NULL)
-        return -ENOENT;
-    if (bdrv_attach_dev(bs, dev) < 0)
-        return -EEXIST;
-    *ptr = bs;
-    return 0;
-}
-
-static void release_drive(Object *obj, const char *name, void *opaque)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
-
-    if (*ptr) {
-        bdrv_detach_dev(*ptr, dev);
-        blockdev_auto_del(*ptr);
-    }
-}
-
-static const char *print_drive(void *ptr)
-{
-    return bdrv_get_device_name(ptr);
-}
-
-static void get_drive(Object *obj, Visitor *v, void *opaque,
-                      const char *name, Error **errp)
-{
-    get_pointer(obj, v, opaque, print_drive, name, errp);
-}
-
-static void set_drive(Object *obj, Visitor *v, void *opaque,
-                      const char *name, Error **errp)
-{
-    set_pointer(obj, v, opaque, parse_drive, name, errp);
-}
-
-PropertyInfo qdev_prop_drive = {
-    .name  = "drive",
-    .get   = get_drive,
-    .set   = set_drive,
-    .release = release_drive,
-};
-
-/* --- character device --- */
-
-static int parse_chr(DeviceState *dev, const char *str, void **ptr)
-{
-    CharDriverState *chr = qemu_chr_find(str);
-    if (chr == NULL) {
-        return -ENOENT;
-    }
-    if (chr->avail_connections < 1) {
-        return -EEXIST;
-    }
-    *ptr = chr;
-    --chr->avail_connections;
-    return 0;
-}
-
-static void release_chr(Object *obj, const char *name, void *opaque)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);
-
-    if (*ptr) {
-        qemu_chr_add_handlers(*ptr, NULL, NULL, NULL, NULL);
-    }
-}
-
-
-static const char *print_chr(void *ptr)
-{
-    CharDriverState *chr = ptr;
-
-    return chr->label ? chr->label : "";
-}
-
-static void get_chr(Object *obj, Visitor *v, void *opaque,
-                    const char *name, Error **errp)
-{
-    get_pointer(obj, v, opaque, print_chr, name, errp);
-}
-
-static void set_chr(Object *obj, Visitor *v, void *opaque,
-                    const char *name, Error **errp)
-{
-    set_pointer(obj, v, opaque, parse_chr, name, errp);
-}
-
-PropertyInfo qdev_prop_chr = {
-    .name  = "chr",
-    .get   = get_chr,
-    .set   = set_chr,
-    .release = release_chr,
-};
-
-/* --- netdev device --- */
-
-static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
-{
-    NetClientState *netdev = qemu_find_netdev(str);
-
-    if (netdev == NULL) {
-        return -ENOENT;
-    }
-    if (netdev->peer) {
-        return -EEXIST;
-    }
-    *ptr = netdev;
-    return 0;
-}
-
-static const char *print_netdev(void *ptr)
-{
-    NetClientState *netdev = ptr;
-
-    return netdev->name ? netdev->name : "";
-}
-
-static void get_netdev(Object *obj, Visitor *v, void *opaque,
-                       const char *name, Error **errp)
-{
-    get_pointer(obj, v, opaque, print_netdev, name, errp);
-}
-
-static void set_netdev(Object *obj, Visitor *v, void *opaque,
-                       const char *name, Error **errp)
-{
-    set_pointer(obj, v, opaque, parse_netdev, name, errp);
-}
-
-PropertyInfo qdev_prop_netdev = {
-    .name  = "netdev",
-    .get   = get_netdev,
-    .set   = set_netdev,
-};
-
-/* --- vlan --- */
-
-static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
-
-    if (*ptr) {
-        int id;
-        if (!net_hub_id_for_client(*ptr, &id)) {
-            return snprintf(dest, len, "%d", id);
-        }
-    }
-
-    return snprintf(dest, len, "<null>");
-}
-
-static void get_vlan(Object *obj, Visitor *v, void *opaque,
-                     const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
-    int32_t id = -1;
-
-    if (*ptr) {
-        int hub_id;
-        if (!net_hub_id_for_client(*ptr, &hub_id)) {
-            id = hub_id;
-        }
-    }
-
-    visit_type_int32(v, &id, name, errp);
-}
-
-static void set_vlan(Object *obj, Visitor *v, void *opaque,
-                     const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
-    Error *local_err = NULL;
-    int32_t id;
-    NetClientState *hubport;
-
-    if (dev->state != DEV_STATE_CREATED) {
-        error_set(errp, QERR_PERMISSION_DENIED);
-        return;
-    }
-
-    visit_type_int32(v, &id, name, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-    if (id == -1) {
-        *ptr = NULL;
-        return;
-    }
-
-    hubport = net_hub_port_find(id);
-    if (!hubport) {
-        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
-                  name, prop->info->name);
-        return;
-    }
-    *ptr = hubport;
-}
-
-PropertyInfo qdev_prop_vlan = {
-    .name  = "vlan",
-    .print = print_vlan,
-    .get   = get_vlan,
-    .set   = set_vlan,
-};
-
 /* --- pointer --- */
 
 /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
@@ -1188,44 +924,6 @@ void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value)
     assert_no_error(errp);
 }
 
-int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value)
-{
-    Error *errp = NULL;
-    const char *bdrv_name = value ? bdrv_get_device_name(value) : "";
-    object_property_set_str(OBJECT(dev), bdrv_name,
-                            name, &errp);
-    if (errp) {
-        qerror_report_err(errp);
-        error_free(errp);
-        return -1;
-    }
-    return 0;
-}
-
-void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverState *value)
-{
-    if (qdev_prop_set_drive(dev, name, value) < 0) {
-        exit(1);
-    }
-}
-void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value)
-{
-    Error *errp = NULL;
-    assert(!value || value->label);
-    object_property_set_str(OBJECT(dev),
-                            value ? value->label : "", name, &errp);
-    assert_no_error(errp);
-}
-
-void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value)
-{
-    Error *errp = NULL;
-    assert(!value || value->name);
-    object_property_set_str(OBJECT(dev),
-                            value ? value->name : "", name, &errp);
-    assert_no_error(errp);
-}
-
 void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value)
 {
     Error *errp = NULL;
@@ -1261,7 +959,7 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
 
 static QTAILQ_HEAD(, GlobalProperty) global_props = QTAILQ_HEAD_INITIALIZER(global_props);
 
-static void qdev_prop_register_global(GlobalProperty *prop)
+void qdev_prop_register_global(GlobalProperty *prop)
 {
     QTAILQ_INSERT_TAIL(&global_props, prop, next);
 }
@@ -1292,20 +990,3 @@ void qdev_prop_set_globals(DeviceState *dev)
         class = object_class_get_parent(class);
     } while (class);
 }
-
-static int qdev_add_one_global(QemuOpts *opts, void *opaque)
-{
-    GlobalProperty *g;
-
-    g = g_malloc0(sizeof(*g));
-    g->driver   = qemu_opt_get(opts, "driver");
-    g->property = qemu_opt_get(opts, "property");
-    g->value    = qemu_opt_get(opts, "value");
-    qdev_prop_register_global(g);
-    return 0;
-}
-
-void qemu_add_globals(void)
-{
-    qemu_opts_foreach(qemu_find_opts("global"), qdev_add_one_global, NULL, 0);
-}
diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
index 5b046ab..ddcf774 100644
--- a/hw/qdev-properties.h
+++ b/hw/qdev-properties.h
@@ -116,6 +116,7 @@ void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
 /* FIXME: Remove opaque pointer properties.  */
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
 
+void qdev_prop_register_global(GlobalProperty *prop);
 void qdev_prop_register_global_list(GlobalProperty *props);
 void qdev_prop_set_globals(DeviceState *dev);
 void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
diff --git a/hw/qdev.c b/hw/qdev.c
index 0702659..f00c029 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -25,7 +25,6 @@
    inherit from a particular bus (e.g. PCI or I2C) rather than
    this API directly.  */
 
-#include "net.h"
 #include "qdev.h"
 #include "sysemu.h"
 #include "error.h"
@@ -390,18 +389,6 @@ void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
     dev->gpio_out[n] = pin;
 }
 
-void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
-{
-    qdev_prop_set_macaddr(dev, "mac", nd->macaddr.a);
-    if (nd->netdev)
-        qdev_prop_set_netdev(dev, "netdev", nd->netdev);
-    if (nd->nvectors != DEV_NVECTORS_UNSPECIFIED &&
-        object_property_find(OBJECT(dev), "vectors", NULL)) {
-        qdev_prop_set_uint32(dev, "vectors", nd->nvectors);
-    }
-    nd->instantiated = 1;
-}
-
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
-- 
1.7.11.7

[Qemu-devel] [PATCH 23/24] include qdev code into *-user, too

[Eduardo Habkost]

The code depends on some functions from qemu-option.o, so add
qemu-option.o to universal-obj-y to make sure it's included.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 - Keep files on the hw/ directory
   (it's simply easier to keep them there, as qdev.o depends on irq.o)
 - Add a $(hw-core-obj-y) variable to Makefile.objs for the qdev core code
 - Add irq.o to the list of core qdev files
   (as now the gpio code is being kept inside qdev.c)
---
 Makefile.objs    | 8 ++++++++
 hw/Makefile.objs | 8 ++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 593a592..be3c8ef 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -18,6 +18,13 @@ qom-obj-y = qom/
 universal-obj-y += $(qom-obj-y)
 
 #######################################################################
+# Core hw code (qdev core)
+hw-core-obj-y += hw/
+hw-core-obj-y += qemu-option.o
+
+universal-obj-y += $(hw-core-obj-y)
+
+#######################################################################
 # oslib-obj-y is code depending on the OS (win32 vs posix)
 oslib-obj-y = osdep.o cutils.o qemu-timer-common.o
 oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
@@ -244,5 +251,6 @@ nested-vars += \
 	block-obj-y \
 	user-obj-y \
 	common-obj-y \
+	hw-core-obj-y \
 	extra-obj-y
 dummy := $(call unnest-vars)
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 4833b90..2c931e9 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,3 +1,8 @@
+# core qdev-related obj files, also used by *-user:
+hw-core-obj-y += qdev.o qdev-properties.o
+# irq.o needed for qdev GPIO handling:
+hw-core-obj-y += irq.o
+
 common-obj-y = usb/ ide/
 common-obj-y += loader.o
 common-obj-$(CONFIG_VIRTIO) += virtio-console.o
@@ -155,7 +160,6 @@ common-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 common-obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
 
 common-obj-y += usb/
-common-obj-y += irq.o
 common-obj-$(CONFIG_PTIMER) += ptimer.o
 common-obj-$(CONFIG_MAX7310) += max7310.o
 common-obj-$(CONFIG_WM8750) += wm8750.o
@@ -181,7 +185,7 @@ common-obj-$(CONFIG_SD) += sd.o
 common-obj-y += bt.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o
 common-obj-y += bt-hci-csr.o
 common-obj-y += msmouse.o ps2.o
-common-obj-y += qdev.o qdev-properties.o qdev-monitor.o
+common-obj-y += qdev-monitor.o
 common-obj-y += qdev-properties-system.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 
-- 
1.7.11.7

[Qemu-devel] [PATCH 24/24] qom: make CPU a child of DeviceState

[Eduardo Habkost]

From: Igor Mammedov <imammedo@redhat.com>

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
[ehabkost: change CPU type declaration to hae TYPE_DEVICE as parent]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Yes, there is "changelog" data before the "---" mark, but I believe that
in this case they are important to indicate authorship and the scope of
the Signed-off-by lines (so they need to get into the git commit
message).
---
 include/qemu/cpu.h | 6 +++---
 qom/cpu.c          | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
index 61b7698..bc004fd 100644
--- a/include/qemu/cpu.h
+++ b/include/qemu/cpu.h
@@ -20,7 +20,7 @@
 #ifndef QEMU_CPU_H
 #define QEMU_CPU_H
 
-#include "qemu/object.h"
+#include "hw/qdev-core.h"
 #include "qemu-thread.h"
 
 /**
@@ -46,7 +46,7 @@ typedef struct CPUState CPUState;
  */
 typedef struct CPUClass {
     /*< private >*/
-    ObjectClass parent_class;
+    DeviceClass parent_class;
     /*< public >*/
 
     void (*reset)(CPUState *cpu);
@@ -62,7 +62,7 @@ typedef struct CPUClass {
  */
 struct CPUState {
     /*< private >*/
-    Object parent_obj;
+    DeviceState parent_obj;
     /*< public >*/
 
     struct QemuThread *thread;
diff --git a/qom/cpu.c b/qom/cpu.c
index 5b36046..f59db7d 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -20,6 +20,7 @@
 
 #include "qemu/cpu.h"
 #include "qemu-common.h"
+#include "hw/qdev-core.h"
 
 void cpu_reset(CPUState *cpu)
 {
@@ -43,7 +44,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
 
 static TypeInfo cpu_type_info = {
     .name = TYPE_CPU,
-    .parent = TYPE_OBJECT,
+    .parent = TYPE_DEVICE,
     .instance_size = sizeof(CPUState),
     .abstract = true,
     .class_size = sizeof(CPUClass),
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 09/24] ui/vnc-pallete.c: include headers it needs

[Peter Maydell]

On 9 November 2012 15:56, Eduardo Habkost <ehabkost@redhat.com> wrote:
> Include:
>  - <glib.h> for g_malloc0()
>  - <string.h> for memset()
>
> Some of those headers were probably being included by accident because
> some other headers were including qemu-common.h, but those headers
> should eventually stop including qemu-common.h.

Typo in subject: should be 'palette'.

-- PMM

Re: [Qemu-devel] [PATCH 01/24] user: move *-user/qemu-types.h to main directory

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> The bsd-user/qemu-types.h and linux-user/qemu-types.h files are almost
> the same, but linux-user have the additional definitions of tswapal().
> 
> This moves the linux-user file to the main directory, so the same file
> can be used by linux-user and bsd-user.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

A quick diff -u confirms that this is (still) the case. I had suggested
unifying these files long time ago when I tried fixing darwin-user, but
hit resistance in favor of having the *-users separate. I'm still in
favor of not duplicating identical code, so

Acked-by: Andreas Färber <afaerber@suse.de>

Riku, do you agree?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 02/24] user: rename qemu-types.h to qemu-user-types.h

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> The header file is specific for *-user, but I plan to introduce a more
> generic qemu-types.h file, so I'm renaming it.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

linux-user builds okay and the bsd-user change looks fine, too.
git-grep shows no more occurrences of qemu-types.h.

Acked-by: Andreas Färber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 03/24] qemu-common.h: comment about usage rules

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> Every time we make a tiny change on a header file, we often find
> circular header dependency problems. To avoid this nightmare, we need to
> stop including qemu-common.h on other headers, and we should gradually

"from other headers" as below?

> move the declarations from the catchall qemu-common.h header to their
> specific headers.
> 
> This simply adds a comment documenting the rules about qemu-common.h,
> hoping that people will see it before including qemu-common.h from other
> header files, and before adding more declarations to qemu-common.h.

This reminds me that I had once posted a patch moving a declaration I
had once added for Cocoa to a new ui/ui.h... seems it never made it to
master, I'll go search, maybe we can smuggle that in now. ;)

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qemu-common.h | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-common.h b/qemu-common.h
> index ac9985c..ea43bfa 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -1,5 +1,14 @@
>  
> -/* Common header file that is included by all of qemu.  */
> +/* Common header file that is included by all of qemu.

"QEMU", while at it.

> + *
> + * This file is supposed to be included only by .c files. No header file should
> + * depend on qemu-common.h, as this would easily lead to circular header
> + * dependencies.
> + *
> + * If a header files uses a definition from qemu-common.h, that definition

"a header file"

> + * must be moved to a separate header file, and the header that uses it
> + * must include that header.
> + */
>  #ifndef QEMU_COMMON_H
>  #define QEMU_COMMON_H
>  

I'll fix this up myself to spare you a resend and me another full review.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 03/24] qemu-common.h: comment about usage rules

[Eduardo Habkost]

On Mon, Nov 12, 2012 at 10:57:42PM +0100, Andreas Färber wrote:
> Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > Every time we make a tiny change on a header file, we often find
> > circular header dependency problems. To avoid this nightmare, we need to
> > stop including qemu-common.h on other headers, and we should gradually
> 
> "from other headers" as below?

Both forms sounds equivalent, to my non-native-speaker ears. :-)

But I guess "including from other headers" is better.

> 
> > move the declarations from the catchall qemu-common.h header to their
> > specific headers.
> > 
> > This simply adds a comment documenting the rules about qemu-common.h,
> > hoping that people will see it before including qemu-common.h from other
> > header files, and before adding more declarations to qemu-common.h.
> 
> This reminds me that I had once posted a patch moving a declaration I
> had once added for Cocoa to a new ui/ui.h... seems it never made it to
> master, I'll go search, maybe we can smuggle that in now. ;)
> 
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  qemu-common.h | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qemu-common.h b/qemu-common.h
> > index ac9985c..ea43bfa 100644
> > --- a/qemu-common.h
> > +++ b/qemu-common.h
> > @@ -1,5 +1,14 @@
> >  
> > -/* Common header file that is included by all of qemu.  */
> > +/* Common header file that is included by all of qemu.
> 
> "QEMU", while at it.
> 
> > + *
> > + * This file is supposed to be included only by .c files. No header file should
> > + * depend on qemu-common.h, as this would easily lead to circular header
> > + * dependencies.
> > + *
> > + * If a header files uses a definition from qemu-common.h, that definition
> 
> "a header file"

Oops. Thanks.

> 
> > + * must be moved to a separate header file, and the header that uses it
> > + * must include that header.
> > + */
> >  #ifndef QEMU_COMMON_H
> >  #define QEMU_COMMON_H
> >  
> 
> I'll fix this up myself to spare you a resend and me another full review.

Thanks!

> 
> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 06/24] move I/O-related definitions from qemu-common.h to a new header (qemu-stdio.h)

[Igor Mammedov]

On Fri,  9 Nov 2012 12:56:34 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> This will help reduce the qemu-common.h dependency hell.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> --
> Changes v1 -> v2:
>  - move qemu_open() & qemu_close() to qemu-stdio.h, too
> ---
>  qemu-common.h | 59 ++--------------------------------------------
>  qemu-stdio.h  | 76
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files
> changed, 78 insertions(+), 57 deletions(-) create mode 100644 qemu-stdio.h
> 
> diff --git a/qemu-common.h b/qemu-common.h
> index 6441bee..5080382 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -15,6 +15,8 @@
>  #include "compiler.h"
>  #include "config-host.h"
>  
> +#include "qemu-stdio.h"
> +
>  #if defined(__arm__) || defined(__sparc__) || defined(__mips__) ||
> defined(__hppa__) || defined(__ia64__) #define WORDS_ALIGNED
>  #endif
> @@ -58,28 +60,6 @@ typedef struct MigrationParams MigrationParams;
>  #include "qemu-os-posix.h"
>  #endif
>  
> -#ifndef O_LARGEFILE
> -#define O_LARGEFILE 0
> -#endif
> -#ifndef O_BINARY
> -#define O_BINARY 0
> -#endif
> -#ifndef MAP_ANONYMOUS
> -#define MAP_ANONYMOUS MAP_ANON
> -#endif
> -#ifndef ENOMEDIUM
> -#define ENOMEDIUM ENODEV
> -#endif
> -#if !defined(ENOTSUP)
> -#define ENOTSUP 4096
> -#endif
> -#if !defined(ECANCELED)
> -#define ECANCELED 4097
> -#endif
> -#ifndef TIME_MAX
> -#define TIME_MAX LONG_MAX
> -#endif
> -
>  /* HOST_LONG_BITS is the size of a native pointer in bits. */
>  #if UINTPTR_MAX == UINT32_MAX
>  # define HOST_LONG_BITS 32
> @@ -89,39 +69,6 @@ typedef struct MigrationParams MigrationParams;
>  # error Unknown pointer size
>  #endif
>  
> -#ifndef CONFIG_IOVEC
> -#define CONFIG_IOVEC
> -struct iovec {
> -    void *iov_base;
> -    size_t iov_len;
> -};
> -/*
> - * Use the same value as Linux for now.
> - */
> -#define IOV_MAX		1024
> -#else
> -#include <sys/uio.h>
> -#endif
> -
> -typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> -    GCC_FMT_ATTR(2, 3);
Is there any particular reason to move fprintf_function from qemu-common.h?

> -
> -#ifdef _WIN32
> -#define fsync _commit
> -#if !defined(lseek)
> -# define lseek _lseeki64
> -#endif
> -int qemu_ftruncate64(int, int64_t);
> -#if !defined(ftruncate)
> -# define ftruncate qemu_ftruncate64
> -#endif
> -
> -static inline char *realpath(const char *path, char *resolved_path)
> -{
> -    _fullpath(resolved_path, path, _MAX_PATH);
> -    return resolved_path;
> -}
> -#endif
>  
>  /* icount */
>  void configure_icount(const char *option);
> @@ -217,8 +164,6 @@ const char *path(const char *pathname);
>  
>  void *qemu_oom_check(void *ptr);
>  
> -int qemu_open(const char *name, int flags, ...);
> -int qemu_close(int fd);
>  ssize_t qemu_write_full(int fd, const void *buf, size_t count)
>      QEMU_WARN_UNUSED_RESULT;
>  ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
2 above funcs could be moved along with qemu_open() to osdep.h

> diff --git a/qemu-stdio.h b/qemu-stdio.h
> new file mode 100644
> index 0000000..b2e8eda
> --- /dev/null
> +++ b/qemu-stdio.h
> @@ -0,0 +1,76 @@
> +/* Some basic definitions related to stdio.h or other I/O interfaces
> + */
> +#ifndef QEMU_STDIO_H
> +#define QEMU_STDIO_H
> +
> +#include "compiler.h"
> +#include "config-host.h"
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/time.h>
> +#include <sys/mman.h>
> +
> +#ifndef O_LARGEFILE
> +#define O_LARGEFILE 0
> +#endif
> +#ifndef O_BINARY
> +#define O_BINARY 0
> +#endif
> +#ifndef MAP_ANONYMOUS
> +#define MAP_ANONYMOUS MAP_ANON
> +#endif
> +#ifndef ENOMEDIUM
> +#define ENOMEDIUM ENODEV
> +#endif
> +#if !defined(ENOTSUP)
> +#define ENOTSUP 4096
> +#endif
> +#if !defined(ECANCELED)
> +#define ECANCELED 4097
> +#endif
> +#ifndef TIME_MAX
> +#define TIME_MAX LONG_MAX
> +#endif
> +
> +#ifndef CONFIG_IOVEC
> +#define CONFIG_IOVEC
> +struct iovec {
> +    void *iov_base;
> +    size_t iov_len;
> +};
> +/*
> + * Use the same value as Linux for now.
> + */
> +#define IOV_MAX     1024
> +#else
> +#include <sys/uio.h>
> +#endif
> +
> +typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> +    GCC_FMT_ATTR(2, 3);
> +
> +#ifdef _WIN32
> +#define fsync _commit
> +#if !defined(lseek)
> +# define lseek _lseeki64
> +#endif
> +int qemu_ftruncate64(int, int64_t);
> +#if !defined(ftruncate)
> +# define ftruncate qemu_ftruncate64
> +#endif
> +
> +static inline char *realpath(const char *path, char *resolved_path)
> +{
> +    _fullpath(resolved_path, path, _MAX_PATH);
> +    return resolved_path;
> +}
> +#endif
> +
> +int qemu_open(const char *name, int flags, ...);
> +int qemu_close(int fd);
qemu_open() and qemu_close() are defined in osdep.c so perhaps it would be
better to move their declaration to osdep.h.
The rest looks like it fits a purpose of osdep.h as well, so probably it
could be moved there and we could avoid creating a new header.

> +
> +#endif /* QEMU_STDIO_H */

Re: [Qemu-devel] [PATCH 06/24] move I/O-related definitions from qemu-common.h to a new header (qemu-stdio.h)

[Eduardo Habkost]

On Tue, Nov 13, 2012 at 04:30:17PM +0100, Igor Mammedov wrote:
> On Fri,  9 Nov 2012 12:56:34 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > This will help reduce the qemu-common.h dependency hell.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > --
> > Changes v1 -> v2:
> >  - move qemu_open() & qemu_close() to qemu-stdio.h, too
> > ---
> >  qemu-common.h | 59 ++--------------------------------------------
> >  qemu-stdio.h  | 76
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files
> > changed, 78 insertions(+), 57 deletions(-) create mode 100644 qemu-stdio.h
> > 
> > diff --git a/qemu-common.h b/qemu-common.h
> > index 6441bee..5080382 100644
> > --- a/qemu-common.h
> > +++ b/qemu-common.h
> > @@ -15,6 +15,8 @@
> >  #include "compiler.h"
> >  #include "config-host.h"
> >  
> > +#include "qemu-stdio.h"
> > +
> >  #if defined(__arm__) || defined(__sparc__) || defined(__mips__) ||
> > defined(__hppa__) || defined(__ia64__) #define WORDS_ALIGNED
> >  #endif
> > @@ -58,28 +60,6 @@ typedef struct MigrationParams MigrationParams;
> >  #include "qemu-os-posix.h"
> >  #endif
> >  
> > -#ifndef O_LARGEFILE
> > -#define O_LARGEFILE 0
> > -#endif
> > -#ifndef O_BINARY
> > -#define O_BINARY 0
> > -#endif
> > -#ifndef MAP_ANONYMOUS
> > -#define MAP_ANONYMOUS MAP_ANON
> > -#endif
> > -#ifndef ENOMEDIUM
> > -#define ENOMEDIUM ENODEV
> > -#endif
> > -#if !defined(ENOTSUP)
> > -#define ENOTSUP 4096
> > -#endif
> > -#if !defined(ECANCELED)
> > -#define ECANCELED 4097
> > -#endif
> > -#ifndef TIME_MAX
> > -#define TIME_MAX LONG_MAX
> > -#endif
> > -
> >  /* HOST_LONG_BITS is the size of a native pointer in bits. */
> >  #if UINTPTR_MAX == UINT32_MAX
> >  # define HOST_LONG_BITS 32
> > @@ -89,39 +69,6 @@ typedef struct MigrationParams MigrationParams;
> >  # error Unknown pointer size
> >  #endif
> >  
> > -#ifndef CONFIG_IOVEC
> > -#define CONFIG_IOVEC
> > -struct iovec {
> > -    void *iov_base;
> > -    size_t iov_len;
> > -};
> > -/*
> > - * Use the same value as Linux for now.
> > - */
> > -#define IOV_MAX		1024
> > -#else
> > -#include <sys/uio.h>
> > -#endif
> > -
> > -typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > -    GCC_FMT_ATTR(2, 3);
> Is there any particular reason to move fprintf_function from qemu-common.h?

Yes: the reason is that qemu-common.h should die.  ;-)

Long explanation:
1) qemu-common.h has too much stuff inside it;
2) Due to (1), having headers that include qemu-common.h easily lead to
   circular header dependencies;
3) Due to (2), header files should not include qemu-common.h;
4) Headers that use fprintf_function can't make sure fprintf_function is
   defined, because of (3).


> 
> > -
> > -#ifdef _WIN32
> > -#define fsync _commit
> > -#if !defined(lseek)
> > -# define lseek _lseeki64
> > -#endif
> > -int qemu_ftruncate64(int, int64_t);
> > -#if !defined(ftruncate)
> > -# define ftruncate qemu_ftruncate64
> > -#endif
> > -
> > -static inline char *realpath(const char *path, char *resolved_path)
> > -{
> > -    _fullpath(resolved_path, path, _MAX_PATH);
> > -    return resolved_path;
> > -}
> > -#endif
> >  
> >  /* icount */
> >  void configure_icount(const char *option);
> > @@ -217,8 +164,6 @@ const char *path(const char *pathname);
> >  
> >  void *qemu_oom_check(void *ptr);
> >  
> > -int qemu_open(const char *name, int flags, ...);
> > -int qemu_close(int fd);
> >  ssize_t qemu_write_full(int fd, const void *buf, size_t count)
> >      QEMU_WARN_UNUSED_RESULT;
> >  ssize_t qemu_send_full(int fd, const void *buf, size_t count, int flags)
> 2 above funcs could be moved along with qemu_open() to osdep.h

I would happily move them, if somebody gave me an one-line description
of what's the purpose of osdep.h (so I know what is supposed to be
there, and what's not supposed to be there).

> 
> > diff --git a/qemu-stdio.h b/qemu-stdio.h
> > new file mode 100644
> > index 0000000..b2e8eda
> > --- /dev/null
> > +++ b/qemu-stdio.h
> > @@ -0,0 +1,76 @@
> > +/* Some basic definitions related to stdio.h or other I/O interfaces
> > + */
> > +#ifndef QEMU_STDIO_H
> > +#define QEMU_STDIO_H
> > +
> > +#include "compiler.h"
> > +#include "config-host.h"
> > +
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <fcntl.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <sys/time.h>
> > +#include <sys/mman.h>
> > +
> > +#ifndef O_LARGEFILE
> > +#define O_LARGEFILE 0
> > +#endif
> > +#ifndef O_BINARY
> > +#define O_BINARY 0
> > +#endif
> > +#ifndef MAP_ANONYMOUS
> > +#define MAP_ANONYMOUS MAP_ANON
> > +#endif
> > +#ifndef ENOMEDIUM
> > +#define ENOMEDIUM ENODEV
> > +#endif
> > +#if !defined(ENOTSUP)
> > +#define ENOTSUP 4096
> > +#endif
> > +#if !defined(ECANCELED)
> > +#define ECANCELED 4097
> > +#endif
> > +#ifndef TIME_MAX
> > +#define TIME_MAX LONG_MAX
> > +#endif
> > +
> > +#ifndef CONFIG_IOVEC
> > +#define CONFIG_IOVEC
> > +struct iovec {
> > +    void *iov_base;
> > +    size_t iov_len;
> > +};
> > +/*
> > + * Use the same value as Linux for now.
> > + */
> > +#define IOV_MAX     1024
> > +#else
> > +#include <sys/uio.h>
> > +#endif
> > +
> > +typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > +    GCC_FMT_ATTR(2, 3);
> > +
> > +#ifdef _WIN32
> > +#define fsync _commit
> > +#if !defined(lseek)
> > +# define lseek _lseeki64
> > +#endif
> > +int qemu_ftruncate64(int, int64_t);
> > +#if !defined(ftruncate)
> > +# define ftruncate qemu_ftruncate64
> > +#endif
> > +
> > +static inline char *realpath(const char *path, char *resolved_path)
> > +{
> > +    _fullpath(resolved_path, path, _MAX_PATH);
> > +    return resolved_path;
> > +}
> > +#endif
> > +
> > +int qemu_open(const char *name, int flags, ...);
> > +int qemu_close(int fd);
> qemu_open() and qemu_close() are defined in osdep.c so perhaps it would be
> better to move their declaration to osdep.h.
> The rest looks like it fits a purpose of osdep.h as well, so probably it
> could be moved there and we could avoid creating a new header.

Maybe, but I really would like to have a good description of what's the
meaning/purpose of osdep.h, first. osdep.h seems to have lots of
unrelated stuff inside it, already, and I don't want to risk making
osdep.h become a new catchall header file, like qemu-common.h is today.

> 
> > +
> > +#endif /* QEMU_STDIO_H */
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 06/24] move I/O-related definitions from qemu-common.h to a new header (qemu-stdio.h)

[Igor Mammedov]

On Tue, 13 Nov 2012 13:52:16 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Nov 13, 2012 at 04:30:17PM +0100, Igor Mammedov wrote:
> > On Fri,  9 Nov 2012 12:56:34 -0200
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 
> > > This will help reduce the qemu-common.h dependency hell.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > --
> > > Changes v1 -> v2:
> > >  - move qemu_open() & qemu_close() to qemu-stdio.h, too
> > > ---
> > >  qemu-common.h | 59 ++--------------------------------------------
> > >  qemu-stdio.h  | 76
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files
> > > changed, 78 insertions(+), 57 deletions(-) create mode 100644
> > > qemu-stdio.h
> > > 
> > > diff --git a/qemu-common.h b/qemu-common.h
> > > index 6441bee..5080382 100644
> > > --- a/qemu-common.h
> > > +++ b/qemu-common.h
> > > @@ -15,6 +15,8 @@
> > >  #include "compiler.h"
> > >  #include "config-host.h"
> > >  
> > > +#include "qemu-stdio.h"
> > > +
> > >  #if defined(__arm__) || defined(__sparc__) || defined(__mips__) ||
> > > defined(__hppa__) || defined(__ia64__) #define WORDS_ALIGNED
> > >  #endif
> > > @@ -58,28 +60,6 @@ typedef struct MigrationParams MigrationParams;
> > >  #include "qemu-os-posix.h"
> > >  #endif
> > >  
> > > -#ifndef O_LARGEFILE
> > > -#define O_LARGEFILE 0
> > > -#endif
> > > -#ifndef O_BINARY
> > > -#define O_BINARY 0
> > > -#endif
> > > -#ifndef MAP_ANONYMOUS
> > > -#define MAP_ANONYMOUS MAP_ANON
> > > -#endif
> > > -#ifndef ENOMEDIUM
> > > -#define ENOMEDIUM ENODEV
> > > -#endif
> > > -#if !defined(ENOTSUP)
> > > -#define ENOTSUP 4096
> > > -#endif
> > > -#if !defined(ECANCELED)
> > > -#define ECANCELED 4097
> > > -#endif
> > > -#ifndef TIME_MAX
> > > -#define TIME_MAX LONG_MAX
> > > -#endif
> > > -
> > >  /* HOST_LONG_BITS is the size of a native pointer in bits. */
> > >  #if UINTPTR_MAX == UINT32_MAX
> > >  # define HOST_LONG_BITS 32
> > > @@ -89,39 +69,6 @@ typedef struct MigrationParams MigrationParams;
> > >  # error Unknown pointer size
> > >  #endif
> > >  
> > > -#ifndef CONFIG_IOVEC
> > > -#define CONFIG_IOVEC
> > > -struct iovec {
> > > -    void *iov_base;
> > > -    size_t iov_len;
> > > -};
> > > -/*
> > > - * Use the same value as Linux for now.
> > > - */
> > > -#define IOV_MAX		1024
> > > -#else
> > > -#include <sys/uio.h>
> > > -#endif
> > > -
> > > -typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > > -    GCC_FMT_ATTR(2, 3);
> > Is there any particular reason to move fprintf_function from
> > qemu-common.h?
> 
> Yes: the reason is that qemu-common.h should die.  ;-)
> 
> Long explanation:
> 1) qemu-common.h has too much stuff inside it;
> 2) Due to (1), having headers that include qemu-common.h easily lead to
>    circular header dependencies;
> 3) Due to (2), header files should not include qemu-common.h;
> 4) Headers that use fprintf_function can't make sure fprintf_function is
>    defined, because of (3).
Strictly speaking this movement is not related/required for this series to
work, it compiles without it.
But if there is no complaints it's fine by me.

> 
> > 
> > > -
> > > -#ifdef _WIN32
> > > -#define fsync _commit
> > > -#if !defined(lseek)
> > > -# define lseek _lseeki64
> > > -#endif
> > > -int qemu_ftruncate64(int, int64_t);
> > > -#if !defined(ftruncate)
> > > -# define ftruncate qemu_ftruncate64
> > > -#endif
> > > -
> > > -static inline char *realpath(const char *path, char *resolved_path)
> > > -{
> > > -    _fullpath(resolved_path, path, _MAX_PATH);
> > > -    return resolved_path;
> > > -}
> > > -#endif
> > >  
> > >  /* icount */
> > >  void configure_icount(const char *option);
> > > @@ -217,8 +164,6 @@ const char *path(const char *pathname);
> > >  
> > >  void *qemu_oom_check(void *ptr);
> > >  
> > > -int qemu_open(const char *name, int flags, ...);
> > > -int qemu_close(int fd);
> > >  ssize_t qemu_write_full(int fd, const void *buf, size_t count)
> > >      QEMU_WARN_UNUSED_RESULT;
> > >  ssize_t qemu_send_full(int fd, const void *buf, size_t count, int
> > > flags)
> > 2 above funcs could be moved along with qemu_open() to osdep.h
> 
> I would happily move them, if somebody gave me an one-line description
> of what's the purpose of osdep.h (so I know what is supposed to be
> there, and what's not supposed to be there).
they are defined in osdep.c, it's logical to have declarations in osdep.h

> 
> > 
> > > diff --git a/qemu-stdio.h b/qemu-stdio.h
> > > new file mode 100644
> > > index 0000000..b2e8eda
> > > --- /dev/null
> > > +++ b/qemu-stdio.h
> > > @@ -0,0 +1,76 @@
> > > +/* Some basic definitions related to stdio.h or other I/O interfaces
> > > + */
> > > +#ifndef QEMU_STDIO_H
> > > +#define QEMU_STDIO_H
> > > +
> > > +#include "compiler.h"
> > > +#include "config-host.h"
> > > +
> > > +#include <stdlib.h>
> > > +#include <stdio.h>
> > > +#include <fcntl.h>
> > > +#include <sys/types.h>
> > > +#include <sys/stat.h>
> > > +#include <sys/time.h>
> > > +#include <sys/mman.h>
> > > +
> > > +#ifndef O_LARGEFILE
> > > +#define O_LARGEFILE 0
> > > +#endif
> > > +#ifndef O_BINARY
> > > +#define O_BINARY 0
> > > +#endif
> > > +#ifndef MAP_ANONYMOUS
> > > +#define MAP_ANONYMOUS MAP_ANON
> > > +#endif
> > > +#ifndef ENOMEDIUM
> > > +#define ENOMEDIUM ENODEV
> > > +#endif
> > > +#if !defined(ENOTSUP)
> > > +#define ENOTSUP 4096
> > > +#endif
> > > +#if !defined(ECANCELED)
> > > +#define ECANCELED 4097
> > > +#endif
> > > +#ifndef TIME_MAX
> > > +#define TIME_MAX LONG_MAX
> > > +#endif
> > > +
> > > +#ifndef CONFIG_IOVEC
> > > +#define CONFIG_IOVEC
> > > +struct iovec {
> > > +    void *iov_base;
> > > +    size_t iov_len;
> > > +};
> > > +/*
> > > + * Use the same value as Linux for now.
> > > + */
> > > +#define IOV_MAX     1024
> > > +#else
> > > +#include <sys/uio.h>
> > > +#endif
> > > +
> > > +typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > > +    GCC_FMT_ATTR(2, 3);
> > > +
> > > +#ifdef _WIN32
> > > +#define fsync _commit
> > > +#if !defined(lseek)
> > > +# define lseek _lseeki64
> > > +#endif
> > > +int qemu_ftruncate64(int, int64_t);
> > > +#if !defined(ftruncate)
> > > +# define ftruncate qemu_ftruncate64
> > > +#endif
> > > +
> > > +static inline char *realpath(const char *path, char *resolved_path)
> > > +{
> > > +    _fullpath(resolved_path, path, _MAX_PATH);
> > > +    return resolved_path;
> > > +}
> > > +#endif
> > > +
> > > +int qemu_open(const char *name, int flags, ...);
> > > +int qemu_close(int fd);
> > qemu_open() and qemu_close() are defined in osdep.c so perhaps it would be
> > better to move their declaration to osdep.h.
> > The rest looks like it fits a purpose of osdep.h as well, so probably it
> > could be moved there and we could avoid creating a new header.
> 
> Maybe, but I really would like to have a good description of what's the
> meaning/purpose of osdep.h, first. osdep.h seems to have lots of
Judging from commit logs its purpose is in abstracting/unifying OS depended
features to some common denominator. And the code you are moving is
exactly does this.

> unrelated stuff inside it, already, and I don't want to risk making
> osdep.h become a new catchall header file, like qemu-common.h is today.
> 
So far osdep.h is pretty simple in terms of header dependences and doesn't
include catch-all headers, moving this declarations and defines there
doesn't complicate it in this aspect.

> > 
> > > +
> > > +#endif /* QEMU_STDIO_H */
> > 
> 

Re: [Qemu-devel] [PATCH 06/24] move I/O-related definitions from qemu-common.h to a new header (qemu-stdio.h)

[Eduardo Habkost]

On Tue, Nov 13, 2012 at 05:43:42PM +0100, Igor Mammedov wrote:
> On Tue, 13 Nov 2012 13:52:16 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Tue, Nov 13, 2012 at 04:30:17PM +0100, Igor Mammedov wrote:
> > > On Fri,  9 Nov 2012 12:56:34 -0200
> > > Eduardo Habkost <ehabkost@redhat.com> wrote:
> > > 
> > > > This will help reduce the qemu-common.h dependency hell.
> > > > 
> > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > --
> > > > Changes v1 -> v2:
> > > >  - move qemu_open() & qemu_close() to qemu-stdio.h, too
> > > > ---
> > > >  qemu-common.h | 59 ++--------------------------------------------
> > > >  qemu-stdio.h  | 76
> > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files
> > > > changed, 78 insertions(+), 57 deletions(-) create mode 100644
> > > > qemu-stdio.h
> > > > 
> > > > diff --git a/qemu-common.h b/qemu-common.h
> > > > index 6441bee..5080382 100644
> > > > --- a/qemu-common.h
> > > > +++ b/qemu-common.h
> > > > @@ -15,6 +15,8 @@
> > > >  #include "compiler.h"
> > > >  #include "config-host.h"
> > > >  
> > > > +#include "qemu-stdio.h"
> > > > +
> > > >  #if defined(__arm__) || defined(__sparc__) || defined(__mips__) ||
> > > > defined(__hppa__) || defined(__ia64__) #define WORDS_ALIGNED
> > > >  #endif
> > > > @@ -58,28 +60,6 @@ typedef struct MigrationParams MigrationParams;
> > > >  #include "qemu-os-posix.h"
> > > >  #endif
> > > >  
> > > > -#ifndef O_LARGEFILE
> > > > -#define O_LARGEFILE 0
> > > > -#endif
> > > > -#ifndef O_BINARY
> > > > -#define O_BINARY 0
> > > > -#endif
> > > > -#ifndef MAP_ANONYMOUS
> > > > -#define MAP_ANONYMOUS MAP_ANON
> > > > -#endif
> > > > -#ifndef ENOMEDIUM
> > > > -#define ENOMEDIUM ENODEV
> > > > -#endif
> > > > -#if !defined(ENOTSUP)
> > > > -#define ENOTSUP 4096
> > > > -#endif
> > > > -#if !defined(ECANCELED)
> > > > -#define ECANCELED 4097
> > > > -#endif
> > > > -#ifndef TIME_MAX
> > > > -#define TIME_MAX LONG_MAX
> > > > -#endif
> > > > -
> > > >  /* HOST_LONG_BITS is the size of a native pointer in bits. */
> > > >  #if UINTPTR_MAX == UINT32_MAX
> > > >  # define HOST_LONG_BITS 32
> > > > @@ -89,39 +69,6 @@ typedef struct MigrationParams MigrationParams;
> > > >  # error Unknown pointer size
> > > >  #endif
> > > >  
> > > > -#ifndef CONFIG_IOVEC
> > > > -#define CONFIG_IOVEC
> > > > -struct iovec {
> > > > -    void *iov_base;
> > > > -    size_t iov_len;
> > > > -};
> > > > -/*
> > > > - * Use the same value as Linux for now.
> > > > - */
> > > > -#define IOV_MAX		1024
> > > > -#else
> > > > -#include <sys/uio.h>
> > > > -#endif
> > > > -
> > > > -typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > > > -    GCC_FMT_ATTR(2, 3);
> > > Is there any particular reason to move fprintf_function from
> > > qemu-common.h?
> > 
> > Yes: the reason is that qemu-common.h should die.  ;-)
> > 
> > Long explanation:
> > 1) qemu-common.h has too much stuff inside it;
> > 2) Due to (1), having headers that include qemu-common.h easily lead to
> >    circular header dependencies;
> > 3) Due to (2), header files should not include qemu-common.h;
> > 4) Headers that use fprintf_function can't make sure fprintf_function is
> >    defined, because of (3).
> Strictly speaking this movement is not related/required for this series to
> work, it compiles without it.

I think it was necessary when we were adding a new '#include "cpu.h"'
line somewhere, on previous versions of the series, but it may be not
necessary anymore.


> But if there is no complaints it's fine by me.

If the code compiles fine without the patch, I'm OK with dropping it by
now.  Then I can send a new version later that moves that code to
osdep.h.

> 
> > 
> > > 
> > > > -
> > > > -#ifdef _WIN32
> > > > -#define fsync _commit
> > > > -#if !defined(lseek)
> > > > -# define lseek _lseeki64
> > > > -#endif
> > > > -int qemu_ftruncate64(int, int64_t);
> > > > -#if !defined(ftruncate)
> > > > -# define ftruncate qemu_ftruncate64
> > > > -#endif
> > > > -
> > > > -static inline char *realpath(const char *path, char *resolved_path)
> > > > -{
> > > > -    _fullpath(resolved_path, path, _MAX_PATH);
> > > > -    return resolved_path;
> > > > -}
> > > > -#endif
> > > >  
> > > >  /* icount */
> > > >  void configure_icount(const char *option);
> > > > @@ -217,8 +164,6 @@ const char *path(const char *pathname);
> > > >  
> > > >  void *qemu_oom_check(void *ptr);
> > > >  
> > > > -int qemu_open(const char *name, int flags, ...);
> > > > -int qemu_close(int fd);
> > > >  ssize_t qemu_write_full(int fd, const void *buf, size_t count)
> > > >      QEMU_WARN_UNUSED_RESULT;
> > > >  ssize_t qemu_send_full(int fd, const void *buf, size_t count, int
> > > > flags)
> > > 2 above funcs could be moved along with qemu_open() to osdep.h
> > 
> > I would happily move them, if somebody gave me an one-line description
> > of what's the purpose of osdep.h (so I know what is supposed to be
> > there, and what's not supposed to be there).
> they are defined in osdep.c, it's logical to have declarations in osdep.h
> 
> > 
> > > 
> > > > diff --git a/qemu-stdio.h b/qemu-stdio.h
> > > > new file mode 100644
> > > > index 0000000..b2e8eda
> > > > --- /dev/null
> > > > +++ b/qemu-stdio.h
> > > > @@ -0,0 +1,76 @@
> > > > +/* Some basic definitions related to stdio.h or other I/O interfaces
> > > > + */
> > > > +#ifndef QEMU_STDIO_H
> > > > +#define QEMU_STDIO_H
> > > > +
> > > > +#include "compiler.h"
> > > > +#include "config-host.h"
> > > > +
> > > > +#include <stdlib.h>
> > > > +#include <stdio.h>
> > > > +#include <fcntl.h>
> > > > +#include <sys/types.h>
> > > > +#include <sys/stat.h>
> > > > +#include <sys/time.h>
> > > > +#include <sys/mman.h>
> > > > +
> > > > +#ifndef O_LARGEFILE
> > > > +#define O_LARGEFILE 0
> > > > +#endif
> > > > +#ifndef O_BINARY
> > > > +#define O_BINARY 0
> > > > +#endif
> > > > +#ifndef MAP_ANONYMOUS
> > > > +#define MAP_ANONYMOUS MAP_ANON
> > > > +#endif
> > > > +#ifndef ENOMEDIUM
> > > > +#define ENOMEDIUM ENODEV
> > > > +#endif
> > > > +#if !defined(ENOTSUP)
> > > > +#define ENOTSUP 4096
> > > > +#endif
> > > > +#if !defined(ECANCELED)
> > > > +#define ECANCELED 4097
> > > > +#endif
> > > > +#ifndef TIME_MAX
> > > > +#define TIME_MAX LONG_MAX
> > > > +#endif
> > > > +
> > > > +#ifndef CONFIG_IOVEC
> > > > +#define CONFIG_IOVEC
> > > > +struct iovec {
> > > > +    void *iov_base;
> > > > +    size_t iov_len;
> > > > +};
> > > > +/*
> > > > + * Use the same value as Linux for now.
> > > > + */
> > > > +#define IOV_MAX     1024
> > > > +#else
> > > > +#include <sys/uio.h>
> > > > +#endif
> > > > +
> > > > +typedef int (*fprintf_function)(FILE *f, const char *fmt, ...)
> > > > +    GCC_FMT_ATTR(2, 3);
> > > > +
> > > > +#ifdef _WIN32
> > > > +#define fsync _commit
> > > > +#if !defined(lseek)
> > > > +# define lseek _lseeki64
> > > > +#endif
> > > > +int qemu_ftruncate64(int, int64_t);
> > > > +#if !defined(ftruncate)
> > > > +# define ftruncate qemu_ftruncate64
> > > > +#endif
> > > > +
> > > > +static inline char *realpath(const char *path, char *resolved_path)
> > > > +{
> > > > +    _fullpath(resolved_path, path, _MAX_PATH);
> > > > +    return resolved_path;
> > > > +}
> > > > +#endif
> > > > +
> > > > +int qemu_open(const char *name, int flags, ...);
> > > > +int qemu_close(int fd);
> > > qemu_open() and qemu_close() are defined in osdep.c so perhaps it would be
> > > better to move their declaration to osdep.h.
> > > The rest looks like it fits a purpose of osdep.h as well, so probably it
> > > could be moved there and we could avoid creating a new header.
> > 
> > Maybe, but I really would like to have a good description of what's the
> > meaning/purpose of osdep.h, first. osdep.h seems to have lots of
> Judging from commit logs its purpose is in abstracting/unifying OS depended
> features to some common denominator. And the code you are moving is
> exactly does this.
> 
> > unrelated stuff inside it, already, and I don't want to risk making
> > osdep.h become a new catchall header file, like qemu-common.h is today.
> > 
> So far osdep.h is pretty simple in terms of header dependences and doesn't
> include catch-all headers, moving this declarations and defines there
> doesn't complicate it in this aspect.

Sounds good to me. I guess the description you put above makes sure
osdep.h won't start including every single header out there, like
qemu-common.h does.

> 
> > > 
> > > > +
> > > > +#endif /* QEMU_STDIO_H */
> > > 
> > 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 04/24] move qemu_irq typedef out of cpu-common.h

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> From: Igor Mammedov <imammedo@redhat.com>
> 
> it's necessary for making CPU child of DEVICE without
> causing circular header deps.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> [ehabkost: re-added the typedef to hw/irq.h after rebasing]
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Acked-by: Andreas Färber <afaerber@suse.de>

but obviously qemu-common.h. ;)

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 04/24] move qemu_irq typedef out of cpu-common.h

[Eduardo Habkost]

On Wed, Nov 14, 2012 at 01:03:46AM +0100, Andreas Färber wrote:
> Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > From: Igor Mammedov <imammedo@redhat.com>
> > 
> > it's necessary for making CPU child of DEVICE without
> > causing circular header deps.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > [ehabkost: re-added the typedef to hw/irq.h after rebasing]
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Acked-by: Andreas Färber <afaerber@suse.de>

Thanks!

> 
> but obviously qemu-common.h. ;)

Not the first commit message error in this series. I need to pay
more attention to my commit messages. :-)

-- 
Eduardo

Re: [Qemu-devel] [PATCH 05/24] qdev: split up header so it can be used in cpu.h

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> From: Anthony Liguori <aliguori@us.ibm.com>
> 
> Header file dependency is a frickin' nightmare right now.  cpu.h tends to get
> included in our 'include everything' header files but qdev also needs to include
> those headers mainly for qdev-properties since it knows about CharDriverState
> and friends.
> 
> We can solve this for now by splitting out qdev.h along the same lines that we
> previously split the C file.  Then cpu.h just needs to include qdev-core.h
> 
> hw/qdev.h is split into following new headers:
>     hw/qdev-core.h
>     hw/qdev-properties.h
>     hw/qdev-monitor.h
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> 
> [ehabkost: re-add DEFINE_PROP_PCI_HOST_DEVADDR, that was removed on the
>  original patch (by mistake, I guess)]
> [ehabkost: kill qdev_prop_set_vlan() declaration]
> [ehabkost: moved get_fw_dev_path() comment to the original location
>  (I don't know why it was moved)]
> [ehabkost: removed qdev_exists() declaration]
> [ehabkost: keep using 'QemuOpts' instead of 'struct QemuOpts', as
>  qdev-core.h includes qemu-option.h]
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Yay, I made it through this patch line by line! Perfect now,

Acked-by: Andreas Färber <afaerber@suse.de>

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 07/24] qemu-fsdev-dummy.c: include module.h

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> module.h is where machine_init() is defined, but qemu-fsdev-dummy.c
> doesn't include it.
> 
> The header is probably being included by accident because some other
> headers are including qemu-common.h, but those headers should eventually
> stop including qemu-common.h.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Acked-by: Andreas Färber <afaerber@suse.de>

I'm surprised about this machine_init() use case outside a machine though...

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 08/24] vnc-palette.h: include <stdbool.h>

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> <stdbool.h> is needed for the 'bool' type, used in the header.
> 
> The header is probably being included by accident because some other
> headers are including qemu-common.h, but those headers should eventually
> stop including qemu-common.h.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Acked-by: Andreas Färber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 09/24] ui/vnc-pallete.c: include headers it needs

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> Include:
>  - <glib.h> for g_malloc0()
>  - <string.h> for memset()
> 
> Some of those headers were probably being included by accident because
> some other headers were including qemu-common.h, but those headers
> should eventually stop including qemu-common.h.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Acked-by: Andreas Färber <afaerber@suse.de>

Subject fixed.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 10/24] qemu-config.h: include headers it needs

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> Include:
> - <stdio.h> for FILE
> - qemu-option.h for QemuOptsList
> 
> Some of those headers were probably being included by accident because
> some other headers were including qemu-common.h, but those headers
> should eventually stop including qemu-common.h.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Acked-by: Andreas Färber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 11/24] qapi/qmp-registry.c: include headers it needs

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> Include:
> - <glib.h> for g_malloc0()
> - <string.h> for strcmp()
> 
> Some of those headers were probably being included by accident because
> some other headers were including qemu-common.h, but those headers
> should eventually stop including qemu-common.h.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Acked-by: Andreas Färber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 12/24] qga/channel-posix.c: include headers it needs

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> Include:
>  - <errno.h> for errno
>  - <unistd.h> & <fcntl.h> for fcntl()
>  - "qemu-stdio.h" for qemu_open()
> 
> Some of those headers were probably being included by accident because
> some other headers were including qemu-common.h, but those headers
> should eventually stop including qemu-common.h.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qga/channel-posix.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
> index d152827..2a68451 100644
> --- a/qga/channel-posix.c
> +++ b/qga/channel-posix.c
> @@ -1,5 +1,9 @@
>  #include <glib.h>
>  #include <termios.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include "qemu-stdio.h"
>  #include "qemu_socket.h"
>  #include "qga/channel.h"
>  

Since the qemu-stdio.h patch was under debate, I've posted a patch
moving qemu_open() and its companion qemu_close() to osdep.h as
suggested by Igor - uses osdep.h here works fine then. Rest looks good.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v5 22/24] qdev-properties.c: separate core from the code used only by qemu-system-*

[Eduardo Habkost]

This separates the qdev properties code in two parts:
 - qdev-properties.c, that contains most of the qdev properties code;
 - qdev-properties-system.c for code specific for qemu-system-*,
   containing:
   - Property types: drive, chr, netdev, vlan, that depend on code that
     won't be included on *-user
   - qemu_add_globals(), that depends on qemu-config.o.

This change should help on two things:
 - Allowing DeviceState to be used by *-user without pulling
   dependencies that are specific for qemu-system-*;
 - Writing qdev unit tests without pulling too many dependencies.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
[imammedo: keep qdev_get_child_bus() in hw/qdev.c]
[imammedo: put qdev_set_nic_properties() in hw/qdev-properties-system.c]
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
[ehabkost: updated the qdev_init_gpio_in() code on qdev-system.c to current
 version]
[ehabkost: added copyright/license information to new qdev*-system.c files]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Yes, there is "changelog" data before the "---" mark, but I believe that
in this case they are important to indicate authorship and the scope of
the Signed-off-by lines (so they need to get into the git commit
message).

Detailed changelog:

Changes v1 (ehabkost) -> v2 (imammedo):
 - keep qdev_get_child_bus() in hw/qdev.c
 - put qdev_set_nic_properties() in hw/qdev-properties-system.c

Changes v2 -> v3 (ehabkost):
 - updated the qdev_init_gpio_in() code on qdev-system.c to current
   version

Changes v3 -> v4 (ehabkost):
 - Added copyright/license information to qdev-properties-system.c
   (based on copyright/license of qdev-properties.c)
 - Whitespace change at the end of qdev-properties.c
 - Don't create qdev-system.c, now we can keep the qdev.c code as-is
   as the qdev.c dependencies were reduced
 - Rewrite patch description

Changes v4 -> v5 (ehabkost):
 - Remove large copyright header and instead just point to the original
   file it was based on
---
 hw/Makefile.objs            |   1 +
 hw/qdev-properties-system.c | 352 ++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev-properties.c        | 321 +---------------------------------------
 hw/qdev-properties.h        |   1 +
 hw/qdev.c                   |  13 --
 5 files changed, 355 insertions(+), 333 deletions(-)
 create mode 100644 hw/qdev-properties-system.c

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index af4ab0c..4833b90 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -182,6 +182,7 @@ common-obj-y += bt.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o
 common-obj-y += bt-hci-csr.o
 common-obj-y += msmouse.o ps2.o
 common-obj-y += qdev.o qdev-properties.o qdev-monitor.o
+common-obj-y += qdev-properties-system.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 
 # xen backend driver support
diff --git a/hw/qdev-properties-system.c b/hw/qdev-properties-system.c
new file mode 100644
index 0000000..9a7e0b3
--- /dev/null
+++ b/hw/qdev-properties-system.c
@@ -0,0 +1,352 @@
+/*
+ * qdev property parsing and global properties
+ * (parts specific for qemu-system-*)
+ *
+ * This file is based on code from hw/qdev-properties.c from
+ * commit 4e68f7a0819f179c2ff90a60611806c789911cc2,
+ * Copyright (c) Gerd Hoffmann <kraxel@redhat.com> and other contributors.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "net.h"
+#include "qdev.h"
+#include "qerror.h"
+#include "blockdev.h"
+#include "hw/block-common.h"
+#include "net/hub.h"
+#include "qapi/qapi-visit-core.h"
+
+static void get_pointer(Object *obj, Visitor *v, Property *prop,
+                        const char *(*print)(void *ptr),
+                        const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    void **ptr = qdev_get_prop_ptr(dev, prop);
+    char *p;
+
+    p = (char *) (*ptr ? print(*ptr) : "");
+    visit_type_str(v, &p, name, errp);
+}
+
+static void set_pointer(Object *obj, Visitor *v, Property *prop,
+                        int (*parse)(DeviceState *dev, const char *str,
+                                     void **ptr),
+                        const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Error *local_err = NULL;
+    void **ptr = qdev_get_prop_ptr(dev, prop);
+    char *str;
+    int ret;
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    visit_type_str(v, &str, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    if (!*str) {
+        g_free(str);
+        *ptr = NULL;
+        return;
+    }
+    ret = parse(dev, str, ptr);
+    error_set_from_qdev_prop_error(errp, ret, dev, prop, str);
+    g_free(str);
+}
+
+/* --- drive --- */
+
+static int parse_drive(DeviceState *dev, const char *str, void **ptr)
+{
+    BlockDriverState *bs;
+
+    bs = bdrv_find(str);
+    if (bs == NULL)
+        return -ENOENT;
+    if (bdrv_attach_dev(bs, dev) < 0)
+        return -EEXIST;
+    *ptr = bs;
+    return 0;
+}
+
+static void release_drive(Object *obj, const char *name, void *opaque)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (*ptr) {
+        bdrv_detach_dev(*ptr, dev);
+        blockdev_auto_del(*ptr);
+    }
+}
+
+static const char *print_drive(void *ptr)
+{
+    return bdrv_get_device_name(ptr);
+}
+
+static void get_drive(Object *obj, Visitor *v, void *opaque,
+                      const char *name, Error **errp)
+{
+    get_pointer(obj, v, opaque, print_drive, name, errp);
+}
+
+static void set_drive(Object *obj, Visitor *v, void *opaque,
+                      const char *name, Error **errp)
+{
+    set_pointer(obj, v, opaque, parse_drive, name, errp);
+}
+
+PropertyInfo qdev_prop_drive = {
+    .name  = "drive",
+    .get   = get_drive,
+    .set   = set_drive,
+    .release = release_drive,
+};
+
+/* --- character device --- */
+
+static int parse_chr(DeviceState *dev, const char *str, void **ptr)
+{
+    CharDriverState *chr = qemu_chr_find(str);
+    if (chr == NULL) {
+        return -ENOENT;
+    }
+    if (chr->avail_connections < 1) {
+        return -EEXIST;
+    }
+    *ptr = chr;
+    --chr->avail_connections;
+    return 0;
+}
+
+static void release_chr(Object *obj, const char *name, void *opaque)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (*ptr) {
+        qemu_chr_add_handlers(*ptr, NULL, NULL, NULL, NULL);
+    }
+}
+
+
+static const char *print_chr(void *ptr)
+{
+    CharDriverState *chr = ptr;
+
+    return chr->label ? chr->label : "";
+}
+
+static void get_chr(Object *obj, Visitor *v, void *opaque,
+                    const char *name, Error **errp)
+{
+    get_pointer(obj, v, opaque, print_chr, name, errp);
+}
+
+static void set_chr(Object *obj, Visitor *v, void *opaque,
+                    const char *name, Error **errp)
+{
+    set_pointer(obj, v, opaque, parse_chr, name, errp);
+}
+
+PropertyInfo qdev_prop_chr = {
+    .name  = "chr",
+    .get   = get_chr,
+    .set   = set_chr,
+    .release = release_chr,
+};
+
+/* --- netdev device --- */
+
+static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
+{
+    NetClientState *netdev = qemu_find_netdev(str);
+
+    if (netdev == NULL) {
+        return -ENOENT;
+    }
+    if (netdev->peer) {
+        return -EEXIST;
+    }
+    *ptr = netdev;
+    return 0;
+}
+
+static const char *print_netdev(void *ptr)
+{
+    NetClientState *netdev = ptr;
+
+    return netdev->name ? netdev->name : "";
+}
+
+static void get_netdev(Object *obj, Visitor *v, void *opaque,
+                       const char *name, Error **errp)
+{
+    get_pointer(obj, v, opaque, print_netdev, name, errp);
+}
+
+static void set_netdev(Object *obj, Visitor *v, void *opaque,
+                       const char *name, Error **errp)
+{
+    set_pointer(obj, v, opaque, parse_netdev, name, errp);
+}
+
+PropertyInfo qdev_prop_netdev = {
+    .name  = "netdev",
+    .get   = get_netdev,
+    .set   = set_netdev,
+};
+
+void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
+{
+    qdev_prop_set_macaddr(dev, "mac", nd->macaddr.a);
+    if (nd->netdev)
+        qdev_prop_set_netdev(dev, "netdev", nd->netdev);
+    if (nd->nvectors != DEV_NVECTORS_UNSPECIFIED &&
+        object_property_find(OBJECT(dev), "vectors", NULL)) {
+        qdev_prop_set_uint32(dev, "vectors", nd->nvectors);
+    }
+    nd->instantiated = 1;
+}
+
+/* --- vlan --- */
+
+static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
+
+    if (*ptr) {
+        int id;
+        if (!net_hub_id_for_client(*ptr, &id)) {
+            return snprintf(dest, len, "%d", id);
+        }
+    }
+
+    return snprintf(dest, len, "<null>");
+}
+
+static void get_vlan(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
+    int32_t id = -1;
+
+    if (*ptr) {
+        int hub_id;
+        if (!net_hub_id_for_client(*ptr, &hub_id)) {
+            id = hub_id;
+        }
+    }
+
+    visit_type_int32(v, &id, name, errp);
+}
+
+static void set_vlan(Object *obj, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    DeviceState *dev = DEVICE(obj);
+    Property *prop = opaque;
+    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
+    Error *local_err = NULL;
+    int32_t id;
+    NetClientState *hubport;
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    visit_type_int32(v, &id, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    if (id == -1) {
+        *ptr = NULL;
+        return;
+    }
+
+    hubport = net_hub_port_find(id);
+    if (!hubport) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  name, prop->info->name);
+        return;
+    }
+    *ptr = hubport;
+}
+
+PropertyInfo qdev_prop_vlan = {
+    .name  = "vlan",
+    .print = print_vlan,
+    .get   = get_vlan,
+    .set   = set_vlan,
+};
+
+
+int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value)
+{
+    Error *errp = NULL;
+    const char *bdrv_name = value ? bdrv_get_device_name(value) : "";
+    object_property_set_str(OBJECT(dev), bdrv_name,
+                            name, &errp);
+    if (errp) {
+        qerror_report_err(errp);
+        error_free(errp);
+        return -1;
+    }
+    return 0;
+}
+
+void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverState *value)
+{
+    if (qdev_prop_set_drive(dev, name, value) < 0) {
+        exit(1);
+    }
+}
+
+void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value)
+{
+    Error *errp = NULL;
+    assert(!value || value->label);
+    object_property_set_str(OBJECT(dev),
+                            value ? value->label : "", name, &errp);
+    assert_no_error(errp);
+}
+
+void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value)
+{
+    Error *errp = NULL;
+    assert(!value || value->name);
+    object_property_set_str(OBJECT(dev),
+                            value ? value->name : "", name, &errp);
+    assert_no_error(errp);
+}
+
+static int qdev_add_one_global(QemuOpts *opts, void *opaque)
+{
+    GlobalProperty *g;
+
+    g = g_malloc0(sizeof(*g));
+    g->driver   = qemu_opt_get(opts, "driver");
+    g->property = qemu_opt_get(opts, "property");
+    g->value    = qemu_opt_get(opts, "value");
+    qdev_prop_register_global(g);
+    return 0;
+}
+
+void qemu_add_globals(void)
+{
+    qemu_opts_foreach(qemu_find_opts("global"), qdev_add_one_global, NULL, 0);
+}
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 81d901c..9ec04be 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -13,49 +13,6 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
     return ptr;
 }
 
-static void get_pointer(Object *obj, Visitor *v, Property *prop,
-                        const char *(*print)(void *ptr),
-                        const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    void **ptr = qdev_get_prop_ptr(dev, prop);
-    char *p;
-
-    p = (char *) (*ptr ? print(*ptr) : "");
-    visit_type_str(v, &p, name, errp);
-}
-
-static void set_pointer(Object *obj, Visitor *v, Property *prop,
-                        int (*parse)(DeviceState *dev, const char *str,
-                                     void **ptr),
-                        const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Error *local_err = NULL;
-    void **ptr = qdev_get_prop_ptr(dev, prop);
-    char *str;
-    int ret;
-
-    if (dev->state != DEV_STATE_CREATED) {
-        error_set(errp, QERR_PERMISSION_DENIED);
-        return;
-    }
-
-    visit_type_str(v, &str, name, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-    if (!*str) {
-        g_free(str);
-        *ptr = NULL;
-        return;
-    }
-    ret = parse(dev, str, ptr);
-    error_set_from_qdev_prop_error(errp, ret, dev, prop, str);
-    g_free(str);
-}
-
 static void get_enum(Object *obj, Visitor *v, void *opaque,
                      const char *name, Error **errp)
 {
@@ -476,227 +433,6 @@ PropertyInfo qdev_prop_string = {
     .set   = set_string,
 };
 
-/* --- drive --- */
-
-static int parse_drive(DeviceState *dev, const char *str, void **ptr)
-{
-    BlockDriverState *bs;
-
-    bs = bdrv_find(str);
-    if (bs == NULL)
-        return -ENOENT;
-    if (bdrv_attach_dev(bs, dev) < 0)
-        return -EEXIST;
-    *ptr = bs;
-    return 0;
-}
-
-static void release_drive(Object *obj, const char *name, void *opaque)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    BlockDriverState **ptr = qdev_get_prop_ptr(dev, prop);
-
-    if (*ptr) {
-        bdrv_detach_dev(*ptr, dev);
-        blockdev_auto_del(*ptr);
-    }
-}
-
-static const char *print_drive(void *ptr)
-{
-    return bdrv_get_device_name(ptr);
-}
-
-static void get_drive(Object *obj, Visitor *v, void *opaque,
-                      const char *name, Error **errp)
-{
-    get_pointer(obj, v, opaque, print_drive, name, errp);
-}
-
-static void set_drive(Object *obj, Visitor *v, void *opaque,
-                      const char *name, Error **errp)
-{
-    set_pointer(obj, v, opaque, parse_drive, name, errp);
-}
-
-PropertyInfo qdev_prop_drive = {
-    .name  = "drive",
-    .get   = get_drive,
-    .set   = set_drive,
-    .release = release_drive,
-};
-
-/* --- character device --- */
-
-static int parse_chr(DeviceState *dev, const char *str, void **ptr)
-{
-    CharDriverState *chr = qemu_chr_find(str);
-    if (chr == NULL) {
-        return -ENOENT;
-    }
-    if (chr->avail_connections < 1) {
-        return -EEXIST;
-    }
-    *ptr = chr;
-    --chr->avail_connections;
-    return 0;
-}
-
-static void release_chr(Object *obj, const char *name, void *opaque)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);
-
-    if (*ptr) {
-        qemu_chr_add_handlers(*ptr, NULL, NULL, NULL, NULL);
-    }
-}
-
-
-static const char *print_chr(void *ptr)
-{
-    CharDriverState *chr = ptr;
-
-    return chr->label ? chr->label : "";
-}
-
-static void get_chr(Object *obj, Visitor *v, void *opaque,
-                    const char *name, Error **errp)
-{
-    get_pointer(obj, v, opaque, print_chr, name, errp);
-}
-
-static void set_chr(Object *obj, Visitor *v, void *opaque,
-                    const char *name, Error **errp)
-{
-    set_pointer(obj, v, opaque, parse_chr, name, errp);
-}
-
-PropertyInfo qdev_prop_chr = {
-    .name  = "chr",
-    .get   = get_chr,
-    .set   = set_chr,
-    .release = release_chr,
-};
-
-/* --- netdev device --- */
-
-static int parse_netdev(DeviceState *dev, const char *str, void **ptr)
-{
-    NetClientState *netdev = qemu_find_netdev(str);
-
-    if (netdev == NULL) {
-        return -ENOENT;
-    }
-    if (netdev->peer) {
-        return -EEXIST;
-    }
-    *ptr = netdev;
-    return 0;
-}
-
-static const char *print_netdev(void *ptr)
-{
-    NetClientState *netdev = ptr;
-
-    return netdev->name ? netdev->name : "";
-}
-
-static void get_netdev(Object *obj, Visitor *v, void *opaque,
-                       const char *name, Error **errp)
-{
-    get_pointer(obj, v, opaque, print_netdev, name, errp);
-}
-
-static void set_netdev(Object *obj, Visitor *v, void *opaque,
-                       const char *name, Error **errp)
-{
-    set_pointer(obj, v, opaque, parse_netdev, name, errp);
-}
-
-PropertyInfo qdev_prop_netdev = {
-    .name  = "netdev",
-    .get   = get_netdev,
-    .set   = set_netdev,
-};
-
-/* --- vlan --- */
-
-static int print_vlan(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
-
-    if (*ptr) {
-        int id;
-        if (!net_hub_id_for_client(*ptr, &id)) {
-            return snprintf(dest, len, "%d", id);
-        }
-    }
-
-    return snprintf(dest, len, "<null>");
-}
-
-static void get_vlan(Object *obj, Visitor *v, void *opaque,
-                     const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
-    int32_t id = -1;
-
-    if (*ptr) {
-        int hub_id;
-        if (!net_hub_id_for_client(*ptr, &hub_id)) {
-            id = hub_id;
-        }
-    }
-
-    visit_type_int32(v, &id, name, errp);
-}
-
-static void set_vlan(Object *obj, Visitor *v, void *opaque,
-                     const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    NetClientState **ptr = qdev_get_prop_ptr(dev, prop);
-    Error *local_err = NULL;
-    int32_t id;
-    NetClientState *hubport;
-
-    if (dev->state != DEV_STATE_CREATED) {
-        error_set(errp, QERR_PERMISSION_DENIED);
-        return;
-    }
-
-    visit_type_int32(v, &id, name, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-    if (id == -1) {
-        *ptr = NULL;
-        return;
-    }
-
-    hubport = net_hub_port_find(id);
-    if (!hubport) {
-        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
-                  name, prop->info->name);
-        return;
-    }
-    *ptr = hubport;
-}
-
-PropertyInfo qdev_prop_vlan = {
-    .name  = "vlan",
-    .print = print_vlan,
-    .get   = get_vlan,
-    .set   = set_vlan,
-};
-
 /* --- pointer --- */
 
 /* Not a proper property, just for dirty hacks.  TODO Remove it!  */
@@ -1158,44 +894,6 @@ void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value)
     assert_no_error(errp);
 }
 
-int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value)
-{
-    Error *errp = NULL;
-    const char *bdrv_name = value ? bdrv_get_device_name(value) : "";
-    object_property_set_str(OBJECT(dev), bdrv_name,
-                            name, &errp);
-    if (errp) {
-        qerror_report_err(errp);
-        error_free(errp);
-        return -1;
-    }
-    return 0;
-}
-
-void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverState *value)
-{
-    if (qdev_prop_set_drive(dev, name, value) < 0) {
-        exit(1);
-    }
-}
-void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value)
-{
-    Error *errp = NULL;
-    assert(!value || value->label);
-    object_property_set_str(OBJECT(dev),
-                            value ? value->label : "", name, &errp);
-    assert_no_error(errp);
-}
-
-void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value)
-{
-    Error *errp = NULL;
-    assert(!value || value->name);
-    object_property_set_str(OBJECT(dev),
-                            value ? value->name : "", name, &errp);
-    assert_no_error(errp);
-}
-
 void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value)
 {
     Error *errp = NULL;
@@ -1231,7 +929,7 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
 
 static QTAILQ_HEAD(, GlobalProperty) global_props = QTAILQ_HEAD_INITIALIZER(global_props);
 
-static void qdev_prop_register_global(GlobalProperty *prop)
+void qdev_prop_register_global(GlobalProperty *prop)
 {
     QTAILQ_INSERT_TAIL(&global_props, prop, next);
 }
@@ -1262,20 +960,3 @@ void qdev_prop_set_globals(DeviceState *dev)
         class = object_class_get_parent(class);
     } while (class);
 }
-
-static int qdev_add_one_global(QemuOpts *opts, void *opaque)
-{
-    GlobalProperty *g;
-
-    g = g_malloc0(sizeof(*g));
-    g->driver   = qemu_opt_get(opts, "driver");
-    g->property = qemu_opt_get(opts, "property");
-    g->value    = qemu_opt_get(opts, "value");
-    qdev_prop_register_global(g);
-    return 0;
-}
-
-void qemu_add_globals(void)
-{
-    qemu_opts_foreach(qemu_find_opts("global"), qdev_add_one_global, NULL, 0);
-}
diff --git a/hw/qdev-properties.h b/hw/qdev-properties.h
index 5b046ab..ddcf774 100644
--- a/hw/qdev-properties.h
+++ b/hw/qdev-properties.h
@@ -116,6 +116,7 @@ void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
 /* FIXME: Remove opaque pointer properties.  */
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
 
+void qdev_prop_register_global(GlobalProperty *prop);
 void qdev_prop_register_global_list(GlobalProperty *props);
 void qdev_prop_set_globals(DeviceState *dev);
 void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
diff --git a/hw/qdev.c b/hw/qdev.c
index 0702659..f00c029 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -25,7 +25,6 @@
    inherit from a particular bus (e.g. PCI or I2C) rather than
    this API directly.  */
 
-#include "net.h"
 #include "qdev.h"
 #include "sysemu.h"
 #include "error.h"
@@ -390,18 +389,6 @@ void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
     dev->gpio_out[n] = pin;
 }
 
-void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
-{
-    qdev_prop_set_macaddr(dev, "mac", nd->macaddr.a);
-    if (nd->netdev)
-        qdev_prop_set_netdev(dev, "netdev", nd->netdev);
-    if (nd->nvectors != DEV_NVECTORS_UNSPECIFIED &&
-        object_property_find(OBJECT(dev), "vectors", NULL)) {
-        qdev_prop_set_uint32(dev, "vectors", nd->nvectors);
-    }
-    nd->instantiated = 1;
-}
-
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 13/24] create qemu-types.h for struct typedefs

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> Instead of keeping all those struct typedefs on qemu-common.h, move it
> to a header that can be safely included by other headers, containing
> only the struct typedefs and not pulling other dependencies.
> 
> Also, move some of the qdev-core.h typedefs to the new file, too, so
> other headers don't need to include qdev-core.h only because of
> DeviceState and other typedefs.
> 
> This will help us remove qemu-common.h dependencies from some headers
> later.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Acked-by: Andreas Färber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 14/24] sysemu.h: include qemu-types.h instead of qemu-common.h

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> It just needs the Monitor and DeviceState typedefs, so it doesn't need
> all of qemu-common.h.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  sysemu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysemu.h b/sysemu.h
> index f5ac664..ab1ef8b 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -2,7 +2,7 @@
>  #define SYSEMU_H
>  /* Misc. things related to the system emulator.  */
>  
> -#include "qemu-common.h"
> +#include "qemu-types.h"
>  #include "qemu-option.h"
>  #include "qemu-queue.h"
>  #include "qemu-timer.h"

  CC    i386-softmmu/hw/i386/../kvmvapic.o
In file included from /home/andreas/QEMU/qemu-rcar/target-i386/cpu.h:839:0,
                 from /home/andreas/QEMU/qemu-rcar/qemu-common.h:130,
                 from ../qapi-types.h:19,
                 from /home/andreas/QEMU/qemu-rcar/error.h:16,
                 from /home/andreas/QEMU/qemu-rcar/qemu-option.h:31,
                 from /home/andreas/QEMU/qemu-rcar/sysemu.h:6,
                 from /home/andreas/QEMU/qemu-rcar/hw/i386/../kvmvapic.c:11:
/home/andreas/QEMU/qemu-rcar/target-i386/cpu-qom.h:76:35: error: unknown
type name ‘Error’
make[1]: *** [hw/i386/../kvmvapic.o] Fehler 1
make: *** [subdir-i386-softmmu] Fehler 2

Probably "error.h" needed, too.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 16/24] qapi-types.h: don't include qemu-common.h

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> From: Igor Mammedov <imammedo@redhat.com>
> 
> needed to prevent build breakage when CPU becomes a child of DeviceState
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> [ehabkost: include <stdbool.h> too]
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Acked-by: Andreas Färber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 14/24] sysemu.h: include qemu-types.h instead of qemu-common.h

[Andreas Färber]

Am 14.11.2012 22:56, schrieb Andreas Färber:
> Am 09.11.2012 15:56, schrieb Eduardo Habkost:
>> It just needs the Monitor and DeviceState typedefs, so it doesn't need
>> all of qemu-common.h.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> ---
>>  sysemu.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/sysemu.h b/sysemu.h
>> index f5ac664..ab1ef8b 100644
>> --- a/sysemu.h
>> +++ b/sysemu.h
>> @@ -2,7 +2,7 @@
>>  #define SYSEMU_H
>>  /* Misc. things related to the system emulator.  */
>>  
>> -#include "qemu-common.h"
>> +#include "qemu-types.h"
>>  #include "qemu-option.h"
>>  #include "qemu-queue.h"
>>  #include "qemu-timer.h"
> 
>   CC    i386-softmmu/hw/i386/../kvmvapic.o
> In file included from /home/andreas/QEMU/qemu-rcar/target-i386/cpu.h:839:0,
>                  from /home/andreas/QEMU/qemu-rcar/qemu-common.h:130,
>                  from ../qapi-types.h:19,
>                  from /home/andreas/QEMU/qemu-rcar/error.h:16,
>                  from /home/andreas/QEMU/qemu-rcar/qemu-option.h:31,
>                  from /home/andreas/QEMU/qemu-rcar/sysemu.h:6,
>                  from /home/andreas/QEMU/qemu-rcar/hw/i386/../kvmvapic.c:11:
> /home/andreas/QEMU/qemu-rcar/target-i386/cpu-qom.h:76:35: error: unknown
> type name ‘Error’
> make[1]: *** [hw/i386/../kvmvapic.o] Fehler 1
> make: *** [subdir-i386-softmmu] Fehler 2
> 
> Probably "error.h" needed, too.

Scratch that last part. There's a cyclic inclusion of qemu-common.h,
solved by the qapi-types.h patch 16/24. Reordering solves this issue.
Given that,

Acked-by: Andreas Färber <afaerber@suse.de>

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 15/24] qlist.h: do not include qemu-common.h

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> I don't know why it was including it, as I don't see any code that
> depends on anything from qemu-common.h.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qlist.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/qlist.h b/qlist.h
> index ae776f9..7408947 100644
> --- a/qlist.h
> +++ b/qlist.h
> @@ -15,7 +15,6 @@
>  
>  #include "qobject.h"
>  #include "qemu-queue.h"
> -#include "qemu-common.h"
>  #include "qemu-queue.h"
>  
>  typedef struct QListEntry {

  CC    qga/channel-posix.o
/home/andreas/QEMU/qemu-rcar/qga/channel-posix.c: In function
‘ga_channel_open’:
/home/andreas/QEMU/qemu-rcar/qga/channel-posix.c:141:13: error: implicit
declaration of function ‘exit’ [-Werror=implicit-function-declaration]
/home/andreas/QEMU/qemu-rcar/qga/channel-posix.c:141:13: error:
incompatible implicit declaration of built-in function ‘exit’ [-Werror]
/home/andreas/QEMU/qemu-rcar/qga/channel-posix.c:141:18: error:
‘EXIT_FAILURE’ undeclared (first use in this function)
/home/andreas/QEMU/qemu-rcar/qga/channel-posix.c:141:18: note: each
undeclared identifier is reported only once for each function it appears in
/home/andreas/QEMU/qemu-rcar/qga/channel-posix.c:163:13: error:
incompatible implicit declaration of built-in function ‘exit’ [-Werror]
cc1: all warnings being treated as errors
make: *** [qga/channel-posix.o] Fehler 1

Sounds related to not doing qemu-stdio.h...

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH v2] qga/channel-posix.c: include headers it needs

[Igor Mammedov]

From: Eduardo Habkost <ehabkost@redhat.com>

Include:
 - <errno.h> for errno
 - <unistd.h> & <fcntl.h> for fcntl()
 - "qemu-stdio.h" for qemu_open()

Some of those headers were probably being included by accident because
some other headers were including qemu-common.h, but those headers
should eventually stop including qemu-common.h.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2
  - include qemu-common.h for EXIT_FAILURE, exit and qemu_open definitions
---
 qga/channel-posix.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index d152827..bbbbcc9 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -1,7 +1,11 @@
 #include <glib.h>
 #include <termios.h>
+#include <errno.h>
+#include <unistd.h>
+#include <fcntl.h>
 #include "qemu_socket.h"
 #include "qga/channel.h"
+#include "qemu-common.h"
 
 #ifdef CONFIG_SOLARIS
 #include <stropts.h>
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> The core qdev code uses the reset handler list from vl.c, so move
> qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
> to qdev.c.
> 
> The function declarations were moved to a new qdev-reset.h file, that is
> included by hw.h to keep compatibility, so we don't need to change all
> files that use qemu_register_reset().
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  hw/hw.h         |  6 +-----
>  hw/qdev-reset.h | 11 +++++++++++
>  hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
>  hw/qdev.h       |  1 +
>  sysemu.h        |  1 -
>  vl.c            | 40 ----------------------------------------
>  6 files changed, 54 insertions(+), 46 deletions(-)
>  create mode 100644 hw/qdev-reset.h
> 
> diff --git a/hw/hw.h b/hw/hw.h
> index f530f6f..622a157 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -14,6 +14,7 @@
>  #include "qemu-file.h"
>  #include "vmstate.h"
>  #include "qemu-log.h"
> +#include "qdev-reset.h"
>  
>  #ifdef NEED_CPU_H
>  #if TARGET_LONG_BITS == 64
> @@ -37,11 +38,6 @@
>  #endif
>  #endif
>  
> -typedef void QEMUResetHandler(void *opaque);
> -
> -void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> -
>  /* handler to set the boot_device order for a specific type of QEMUMachine */
>  /* return 0 if success */
>  typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
> diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
> new file mode 100644
> index 0000000..40ae9a5
> --- /dev/null
> +++ b/hw/qdev-reset.h
> @@ -0,0 +1,11 @@
> +/* Device reset handler function registration, used by qdev */
> +#ifndef QDEV_RESET_H
> +#define QDEV_RESET_H
> +
> +typedef void QEMUResetHandler(void *opaque);
> +
> +void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_devices_reset(void);
> +
> +#endif /* QDEV_RESET_H */
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 2cc6434..c242097 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -35,6 +35,47 @@ int qdev_hotplug = 0;
>  static bool qdev_hot_added = false;
>  static bool qdev_hot_removed = false;
>  
> +typedef struct QEMUResetEntry {
> +    QTAILQ_ENTRY(QEMUResetEntry) entry;
> +    QEMUResetHandler *func;
> +    void *opaque;
> +} QEMUResetEntry;
> +
> +static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> +    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> +
> +void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> +
> +    re->func = func;
> +    re->opaque = opaque;
> +    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> +}
> +
> +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    QEMUResetEntry *re;
> +
> +    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> +        if (re->func == func && re->opaque == opaque) {
> +            QTAILQ_REMOVE(&reset_handlers, re, entry);
> +            g_free(re);
> +            return;
> +        }
> +    }
> +}

My tired mind does not like this move and the naming qdev-reset.h.
The reset handling infrastructure is not limited to DeviceState (qdev),
it takes an opaque and is limited to softmmu whereas qdev prefers its
own DeviceClass::reset hook.

Andreas

> +
> +void qemu_devices_reset(void)
> +{
> +    QEMUResetEntry *re, *nre;
> +
> +    /* reset all devices */
> +    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> +        re->func(re->opaque);
> +    }
> +}
> +
>  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 365b8d6..2487b3b 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -5,5 +5,6 @@
>  #include "qdev-core.h"
>  #include "qdev-properties.h"
>  #include "qdev-monitor.h"
> +#include "qdev-reset.h"
>  
>  #endif
> diff --git a/sysemu.h b/sysemu.h
> index ab1ef8b..51f19cc 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -57,7 +57,6 @@ void qemu_system_vmstop_request(RunState reason);
>  int qemu_shutdown_requested_get(void);
>  int qemu_reset_requested_get(void);
>  void qemu_system_killed(int signal, pid_t pid);
> -void qemu_devices_reset(void);
>  void qemu_system_reset(bool report);
>  
>  void qemu_add_exit_notifier(Notifier *notify);
> diff --git a/vl.c b/vl.c
> index 4f03a72..c7448a2 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1456,14 +1456,6 @@ void vm_start(void)
>  
>  /* reset/shutdown handler */
>  
> -typedef struct QEMUResetEntry {
> -    QTAILQ_ENTRY(QEMUResetEntry) entry;
> -    QEMUResetHandler *func;
> -    void *opaque;
> -} QEMUResetEntry;
> -
> -static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> -    QTAILQ_HEAD_INITIALIZER(reset_handlers);
>  static int reset_requested;
>  static int shutdown_requested, shutdown_signal = -1;
>  static pid_t shutdown_pid;
> @@ -1560,38 +1552,6 @@ static bool qemu_vmstop_requested(RunState *r)
>      return false;
>  }
>  
> -void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> -{
> -    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> -
> -    re->func = func;
> -    re->opaque = opaque;
> -    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> -}
> -
> -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> -{
> -    QEMUResetEntry *re;
> -
> -    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> -        if (re->func == func && re->opaque == opaque) {
> -            QTAILQ_REMOVE(&reset_handlers, re, entry);
> -            g_free(re);
> -            return;
> -        }
> -    }
> -}
> -
> -void qemu_devices_reset(void)
> -{
> -    QEMUResetEntry *re, *nre;
> -
> -    /* reset all devices */
> -    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> -        re->func(re->opaque);
> -    }
> -}
> -
>  void qemu_system_reset(bool report)
>  {
>      if (current_machine && current_machine->reset) {
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v2] qga/channel-posix.c: include headers it needs

[Eduardo Habkost]

On Thu, Nov 15, 2012 at 02:19:31AM +0100, Igor Mammedov wrote:
> From: Eduardo Habkost <ehabkost@redhat.com>
> 
> Include:
>  - <errno.h> for errno
>  - <unistd.h> & <fcntl.h> for fcntl()
>  - "qemu-stdio.h" for qemu_open()
> 
> Some of those headers were probably being included by accident because
> some other headers were including qemu-common.h, but those headers
> should eventually stop including qemu-common.h.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2
>   - include qemu-common.h for EXIT_FAILURE, exit and qemu_open definitions

Thanks! The commit message above needs to be edited, too.  :-)

> ---
>  qga/channel-posix.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
> index d152827..bbbbcc9 100644
> --- a/qga/channel-posix.c
> +++ b/qga/channel-posix.c
> @@ -1,7 +1,11 @@
>  #include <glib.h>
>  #include <termios.h>
> +#include <errno.h>
> +#include <unistd.h>
> +#include <fcntl.h>
>  #include "qemu_socket.h"
>  #include "qga/channel.h"
> +#include "qemu-common.h"
>  
>  #ifdef CONFIG_SOLARIS
>  #include <stropts.h>
> -- 
> 1.7.11.7
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c

[Eduardo Habkost]

On Thu, Nov 15, 2012 at 02:54:57AM +0100, Andreas Färber wrote:
> Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > The core qdev code uses the reset handler list from vl.c, so move
> > qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
> > to qdev.c.
> > 
> > The function declarations were moved to a new qdev-reset.h file, that is
> > included by hw.h to keep compatibility, so we don't need to change all
> > files that use qemu_register_reset().
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  hw/hw.h         |  6 +-----
> >  hw/qdev-reset.h | 11 +++++++++++
> >  hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
> >  hw/qdev.h       |  1 +
> >  sysemu.h        |  1 -
> >  vl.c            | 40 ----------------------------------------
> >  6 files changed, 54 insertions(+), 46 deletions(-)
> >  create mode 100644 hw/qdev-reset.h
> > 
> > diff --git a/hw/hw.h b/hw/hw.h
> > index f530f6f..622a157 100644
> > --- a/hw/hw.h
> > +++ b/hw/hw.h
> > @@ -14,6 +14,7 @@
> >  #include "qemu-file.h"
> >  #include "vmstate.h"
> >  #include "qemu-log.h"
> > +#include "qdev-reset.h"
> >  
> >  #ifdef NEED_CPU_H
> >  #if TARGET_LONG_BITS == 64
> > @@ -37,11 +38,6 @@
> >  #endif
> >  #endif
> >  
> > -typedef void QEMUResetHandler(void *opaque);
> > -
> > -void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > -
> >  /* handler to set the boot_device order for a specific type of QEMUMachine */
> >  /* return 0 if success */
> >  typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
> > diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
> > new file mode 100644
> > index 0000000..40ae9a5
> > --- /dev/null
> > +++ b/hw/qdev-reset.h
> > @@ -0,0 +1,11 @@
> > +/* Device reset handler function registration, used by qdev */
> > +#ifndef QDEV_RESET_H
> > +#define QDEV_RESET_H
> > +
> > +typedef void QEMUResetHandler(void *opaque);
> > +
> > +void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > +void qemu_devices_reset(void);
> > +
> > +#endif /* QDEV_RESET_H */
> > diff --git a/hw/qdev.c b/hw/qdev.c
> > index 2cc6434..c242097 100644
> > --- a/hw/qdev.c
> > +++ b/hw/qdev.c
> > @@ -35,6 +35,47 @@ int qdev_hotplug = 0;
> >  static bool qdev_hot_added = false;
> >  static bool qdev_hot_removed = false;
> >  
> > +typedef struct QEMUResetEntry {
> > +    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > +    QEMUResetHandler *func;
> > +    void *opaque;
> > +} QEMUResetEntry;
> > +
> > +static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > +    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > +
> > +void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > +{
> > +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > +
> > +    re->func = func;
> > +    re->opaque = opaque;
> > +    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > +}
> > +
> > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > +{
> > +    QEMUResetEntry *re;
> > +
> > +    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > +        if (re->func == func && re->opaque == opaque) {
> > +            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > +            g_free(re);
> > +            return;
> > +        }
> > +    }
> > +}
> 
> My tired mind does not like this move and the naming qdev-reset.h.
> The reset handling infrastructure is not limited to DeviceState (qdev),
> it takes an opaque and is limited to softmmu whereas qdev prefers its
> own DeviceClass::reset hook.

True, it isn't qdev-specific. But it doesn't belong to vl.c either.
Should we create a reset.o file just for those few lines of code, or is
there any obvious place where this code can go?

DeviceState CPUs have to be reset too, and DeviceState uses the
reset-handler system to make sure DeviceState objects are reset, so it
won't be softmmu-specific.

An alternative is to make empty stubs for qemu_[un]register_reset(), and
not move the reset-handler system to *-user. But somehow I feel better
having a working qemu_register_reset() function (and then adding a
qemu_devices_reset() call to *-user) than having a fake
qemu_register_reset() function and having to use a different API to
reset the CPUs on on *-user.

> 
> Andreas
> 
> > +
> > +void qemu_devices_reset(void)
> > +{
> > +    QEMUResetEntry *re, *nre;
> > +
> > +    /* reset all devices */
> > +    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > +        re->func(re->opaque);
> > +    }
> > +}
> > +
> >  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> >  {
> >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > diff --git a/hw/qdev.h b/hw/qdev.h
> > index 365b8d6..2487b3b 100644
> > --- a/hw/qdev.h
> > +++ b/hw/qdev.h
> > @@ -5,5 +5,6 @@
> >  #include "qdev-core.h"
> >  #include "qdev-properties.h"
> >  #include "qdev-monitor.h"
> > +#include "qdev-reset.h"
> >  
> >  #endif
> > diff --git a/sysemu.h b/sysemu.h
> > index ab1ef8b..51f19cc 100644
> > --- a/sysemu.h
> > +++ b/sysemu.h
> > @@ -57,7 +57,6 @@ void qemu_system_vmstop_request(RunState reason);
> >  int qemu_shutdown_requested_get(void);
> >  int qemu_reset_requested_get(void);
> >  void qemu_system_killed(int signal, pid_t pid);
> > -void qemu_devices_reset(void);
> >  void qemu_system_reset(bool report);
> >  
> >  void qemu_add_exit_notifier(Notifier *notify);
> > diff --git a/vl.c b/vl.c
> > index 4f03a72..c7448a2 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1456,14 +1456,6 @@ void vm_start(void)
> >  
> >  /* reset/shutdown handler */
> >  
> > -typedef struct QEMUResetEntry {
> > -    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > -    QEMUResetHandler *func;
> > -    void *opaque;
> > -} QEMUResetEntry;
> > -
> > -static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > -    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> >  static int reset_requested;
> >  static int shutdown_requested, shutdown_signal = -1;
> >  static pid_t shutdown_pid;
> > @@ -1560,38 +1552,6 @@ static bool qemu_vmstop_requested(RunState *r)
> >      return false;
> >  }
> >  
> > -void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > -{
> > -    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > -
> > -    re->func = func;
> > -    re->opaque = opaque;
> > -    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > -}
> > -
> > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > -{
> > -    QEMUResetEntry *re;
> > -
> > -    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > -        if (re->func == func && re->opaque == opaque) {
> > -            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > -            g_free(re);
> > -            return;
> > -        }
> > -    }
> > -}
> > -
> > -void qemu_devices_reset(void)
> > -{
> > -    QEMUResetEntry *re, *nre;
> > -
> > -    /* reset all devices */
> > -    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > -        re->func(re->opaque);
> > -    }
> > -}
> > -
> >  void qemu_system_reset(bool report)
> >  {
> >      if (current_machine && current_machine->reset) {
> > 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

-- 
Eduardo

Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c

[Peter Maydell]

On 15 November 2012 18:42, Eduardo Habkost <ehabkost@redhat.com> wrote:
> DeviceState CPUs have to be reset too, and DeviceState uses the
> reset-handler system to make sure DeviceState objects are reset, so it
> won't be softmmu-specific.
>
> An alternative is to make empty stubs for qemu_[un]register_reset(), and
> not move the reset-handler system to *-user. But somehow I feel better
> having a working qemu_register_reset() function (and then adding a
> qemu_devices_reset() call to *-user) than having a fake
> qemu_register_reset() function and having to use a different API to
> reset the CPUs on on *-user.

cf the pretty nasty stuff in linux-user/main.c which currently
calls cpu_reset() but only for some targets.

-- PMM

Re: [Qemu-devel] [PATCH 01/24] user: move *-user/qemu-types.h to main directory

[Blue Swirl]

On Mon, Nov 12, 2012 at 9:38 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 09.11.2012 15:56, schrieb Eduardo Habkost:
>> The bsd-user/qemu-types.h and linux-user/qemu-types.h files are almost
>> the same, but linux-user have the additional definitions of tswapal().
>>
>> This moves the linux-user file to the main directory, so the same file
>> can be used by linux-user and bsd-user.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>
> A quick diff -u confirms that this is (still) the case. I had suggested
> unifying these files long time ago when I tried fixing darwin-user, but
> hit resistance in favor of having the *-users separate. I'm still in
> favor of not duplicating identical code, so

The problem in general with merging user emulator code is that the
changes may cause regressions since there are subtle differences. In
this specific case it should be OK.

>
> Acked-by: Andreas Färber <afaerber@suse.de>
>
> Riku, do you agree?
>
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c

[Igor Mammedov]

On Thu, 15 Nov 2012 16:42:41 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Thu, Nov 15, 2012 at 02:54:57AM +0100, Andreas Färber wrote:
> > Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > > The core qdev code uses the reset handler list from vl.c, so move
> > > qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
> > > to qdev.c.
> > > 
> > > The function declarations were moved to a new qdev-reset.h file, that is
> > > included by hw.h to keep compatibility, so we don't need to change all
> > > files that use qemu_register_reset().
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > >  hw/hw.h         |  6 +-----
> > >  hw/qdev-reset.h | 11 +++++++++++
> > >  hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
> > >  hw/qdev.h       |  1 +
> > >  sysemu.h        |  1 -
> > >  vl.c            | 40 ----------------------------------------
> > >  6 files changed, 54 insertions(+), 46 deletions(-)
> > >  create mode 100644 hw/qdev-reset.h
> > > 
> > > diff --git a/hw/hw.h b/hw/hw.h
> > > index f530f6f..622a157 100644
> > > --- a/hw/hw.h
> > > +++ b/hw/hw.h
> > > @@ -14,6 +14,7 @@
> > >  #include "qemu-file.h"
> > >  #include "vmstate.h"
> > >  #include "qemu-log.h"
> > > +#include "qdev-reset.h"
> > >  
> > >  #ifdef NEED_CPU_H
> > >  #if TARGET_LONG_BITS == 64
> > > @@ -37,11 +38,6 @@
> > >  #endif
> > >  #endif
> > >  
> > > -typedef void QEMUResetHandler(void *opaque);
> > > -
> > > -void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > > -
> > >  /* handler to set the boot_device order for a specific type of
> > > QEMUMachine */ /* return 0 if success */
> > >  typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
> > > diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
> > > new file mode 100644
> > > index 0000000..40ae9a5
> > > --- /dev/null
> > > +++ b/hw/qdev-reset.h
> > > @@ -0,0 +1,11 @@
> > > +/* Device reset handler function registration, used by qdev */
> > > +#ifndef QDEV_RESET_H
> > > +#define QDEV_RESET_H
> > > +
> > > +typedef void QEMUResetHandler(void *opaque);
> > > +
> > > +void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > > +void qemu_devices_reset(void);
> > > +
> > > +#endif /* QDEV_RESET_H */
> > > diff --git a/hw/qdev.c b/hw/qdev.c
> > > index 2cc6434..c242097 100644
> > > --- a/hw/qdev.c
> > > +++ b/hw/qdev.c
> > > @@ -35,6 +35,47 @@ int qdev_hotplug = 0;
> > >  static bool qdev_hot_added = false;
> > >  static bool qdev_hot_removed = false;
> > >  
> > > +typedef struct QEMUResetEntry {
> > > +    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > > +    QEMUResetHandler *func;
> > > +    void *opaque;
> > > +} QEMUResetEntry;
> > > +
> > > +static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > > +    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > > +
> > > +void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > > +{
> > > +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > > +
> > > +    re->func = func;
> > > +    re->opaque = opaque;
> > > +    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > > +}
> > > +
> > > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > > +{
> > > +    QEMUResetEntry *re;
> > > +
> > > +    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > > +        if (re->func == func && re->opaque == opaque) {
> > > +            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > > +            g_free(re);
> > > +            return;
> > > +        }
> > > +    }
> > > +}
> > 
> > My tired mind does not like this move and the naming qdev-reset.h.
> > The reset handling infrastructure is not limited to DeviceState (qdev),
> > it takes an opaque and is limited to softmmu whereas qdev prefers its
> > own DeviceClass::reset hook.
> 
> True, it isn't qdev-specific. But it doesn't belong to vl.c either.
> Should we create a reset.o file just for those few lines of code, or is
> there any obvious place where this code can go?
> 
> DeviceState CPUs have to be reset too, and DeviceState uses the
> reset-handler system to make sure DeviceState objects are reset, so it
> won't be softmmu-specific.
> 
> An alternative is to make empty stubs for qemu_[un]register_reset(), and
> not move the reset-handler system to *-user. But somehow I feel better
> having a working qemu_register_reset() function (and then adding a
> qemu_devices_reset() call to *-user) than having a fake
> qemu_register_reset() function and having to use a different API to
> reset the CPUs on on *-user.
There is no real need to add anything, *-user uses pretty much well defined
cpu_reset() and doesn't need anything else to improve it.

PS:
reset-handler callbacks is not much loved thing in qemu, and people would
like to get rid of it eventually. If one checks target-i386/cpu.c:

/* TODO: remove me, when reset over QOM tree is implemented */
static void x86_cpu_machine_reset_cb(void *opaque)

some day in far far (or maybe not far) future there won't be reset cb for
x86-cpu.
Could you just leave reset-handler be a *-softmmu only part.
> 
> > 
> > Andreas
> > 
> > > +
> > > +void qemu_devices_reset(void)
> > > +{
> > > +    QEMUResetEntry *re, *nre;
> > > +
> > > +    /* reset all devices */
> > > +    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > > +        re->func(re->opaque);
> > > +    }
> > > +}
> > > +
> > >  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> > >  {
> > >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > > diff --git a/hw/qdev.h b/hw/qdev.h
> > > index 365b8d6..2487b3b 100644
> > > --- a/hw/qdev.h
> > > +++ b/hw/qdev.h
> > > @@ -5,5 +5,6 @@
> > >  #include "qdev-core.h"
> > >  #include "qdev-properties.h"
> > >  #include "qdev-monitor.h"
> > > +#include "qdev-reset.h"
> > >  
> > >  #endif
> > > diff --git a/sysemu.h b/sysemu.h
> > > index ab1ef8b..51f19cc 100644
> > > --- a/sysemu.h
> > > +++ b/sysemu.h
> > > @@ -57,7 +57,6 @@ void qemu_system_vmstop_request(RunState reason);
> > >  int qemu_shutdown_requested_get(void);
> > >  int qemu_reset_requested_get(void);
> > >  void qemu_system_killed(int signal, pid_t pid);
> > > -void qemu_devices_reset(void);
> > >  void qemu_system_reset(bool report);
> > >  
> > >  void qemu_add_exit_notifier(Notifier *notify);
> > > diff --git a/vl.c b/vl.c
> > > index 4f03a72..c7448a2 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -1456,14 +1456,6 @@ void vm_start(void)
> > >  
> > >  /* reset/shutdown handler */
> > >  
> > > -typedef struct QEMUResetEntry {
> > > -    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > > -    QEMUResetHandler *func;
> > > -    void *opaque;
> > > -} QEMUResetEntry;
> > > -
> > > -static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > > -    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > >  static int reset_requested;
> > >  static int shutdown_requested, shutdown_signal = -1;
> > >  static pid_t shutdown_pid;
> > > @@ -1560,38 +1552,6 @@ static bool qemu_vmstop_requested(RunState *r)
> > >      return false;
> > >  }
> > >  
> > > -void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > > -{
> > > -    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > > -
> > > -    re->func = func;
> > > -    re->opaque = opaque;
> > > -    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > > -}
> > > -
> > > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > > -{
> > > -    QEMUResetEntry *re;
> > > -
> > > -    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > > -        if (re->func == func && re->opaque == opaque) {
> > > -            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > > -            g_free(re);
> > > -            return;
> > > -        }
> > > -    }
> > > -}
> > > -
> > > -void qemu_devices_reset(void)
> > > -{
> > > -    QEMUResetEntry *re, *nre;
> > > -
> > > -    /* reset all devices */
> > > -    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > > -        re->func(re->opaque);
> > > -    }
> > > -}
> > > -
> > >  void qemu_system_reset(bool report)
> > >  {
> > >      if (current_machine && current_machine->reset) {
> > > 
> > 
> > 
> > -- 
> > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 

Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c

[Eduardo Habkost]

On Fri, Nov 30, 2012 at 05:56:26PM +0100, Igor Mammedov wrote:
> On Thu, 15 Nov 2012 16:42:41 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Thu, Nov 15, 2012 at 02:54:57AM +0100, Andreas Färber wrote:
> > > Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > > > The core qdev code uses the reset handler list from vl.c, so move
> > > > qemu_register_reset(), qemu_unregister_reset() and qemu_devices_reset()
> > > > to qdev.c.
> > > > 
> > > > The function declarations were moved to a new qdev-reset.h file, that is
> > > > included by hw.h to keep compatibility, so we don't need to change all
> > > > files that use qemu_register_reset().
> > > > 
> > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > > ---
> > > >  hw/hw.h         |  6 +-----
> > > >  hw/qdev-reset.h | 11 +++++++++++
> > > >  hw/qdev.c       | 41 +++++++++++++++++++++++++++++++++++++++++
> > > >  hw/qdev.h       |  1 +
> > > >  sysemu.h        |  1 -
> > > >  vl.c            | 40 ----------------------------------------
> > > >  6 files changed, 54 insertions(+), 46 deletions(-)
> > > >  create mode 100644 hw/qdev-reset.h
> > > > 
> > > > diff --git a/hw/hw.h b/hw/hw.h
> > > > index f530f6f..622a157 100644
> > > > --- a/hw/hw.h
> > > > +++ b/hw/hw.h
> > > > @@ -14,6 +14,7 @@
> > > >  #include "qemu-file.h"
> > > >  #include "vmstate.h"
> > > >  #include "qemu-log.h"
> > > > +#include "qdev-reset.h"
> > > >  
> > > >  #ifdef NEED_CPU_H
> > > >  #if TARGET_LONG_BITS == 64
> > > > @@ -37,11 +38,6 @@
> > > >  #endif
> > > >  #endif
> > > >  
> > > > -typedef void QEMUResetHandler(void *opaque);
> > > > -
> > > > -void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > > > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > > > -
> > > >  /* handler to set the boot_device order for a specific type of
> > > > QEMUMachine */ /* return 0 if success */
> > > >  typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices);
> > > > diff --git a/hw/qdev-reset.h b/hw/qdev-reset.h
> > > > new file mode 100644
> > > > index 0000000..40ae9a5
> > > > --- /dev/null
> > > > +++ b/hw/qdev-reset.h
> > > > @@ -0,0 +1,11 @@
> > > > +/* Device reset handler function registration, used by qdev */
> > > > +#ifndef QDEV_RESET_H
> > > > +#define QDEV_RESET_H
> > > > +
> > > > +typedef void QEMUResetHandler(void *opaque);
> > > > +
> > > > +void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> > > > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> > > > +void qemu_devices_reset(void);
> > > > +
> > > > +#endif /* QDEV_RESET_H */
> > > > diff --git a/hw/qdev.c b/hw/qdev.c
> > > > index 2cc6434..c242097 100644
> > > > --- a/hw/qdev.c
> > > > +++ b/hw/qdev.c
> > > > @@ -35,6 +35,47 @@ int qdev_hotplug = 0;
> > > >  static bool qdev_hot_added = false;
> > > >  static bool qdev_hot_removed = false;
> > > >  
> > > > +typedef struct QEMUResetEntry {
> > > > +    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > > > +    QEMUResetHandler *func;
> > > > +    void *opaque;
> > > > +} QEMUResetEntry;
> > > > +
> > > > +static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > > > +    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > > > +
> > > > +void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > > > +{
> > > > +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > > > +
> > > > +    re->func = func;
> > > > +    re->opaque = opaque;
> > > > +    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > > > +}
> > > > +
> > > > +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > > > +{
> > > > +    QEMUResetEntry *re;
> > > > +
> > > > +    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > > > +        if (re->func == func && re->opaque == opaque) {
> > > > +            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > > > +            g_free(re);
> > > > +            return;
> > > > +        }
> > > > +    }
> > > > +}
> > > 
> > > My tired mind does not like this move and the naming qdev-reset.h.
> > > The reset handling infrastructure is not limited to DeviceState (qdev),
> > > it takes an opaque and is limited to softmmu whereas qdev prefers its
> > > own DeviceClass::reset hook.
> > 
> > True, it isn't qdev-specific. But it doesn't belong to vl.c either.
> > Should we create a reset.o file just for those few lines of code, or is
> > there any obvious place where this code can go?
> > 
> > DeviceState CPUs have to be reset too, and DeviceState uses the
> > reset-handler system to make sure DeviceState objects are reset, so it
> > won't be softmmu-specific.
> > 
> > An alternative is to make empty stubs for qemu_[un]register_reset(), and
> > not move the reset-handler system to *-user. But somehow I feel better
> > having a working qemu_register_reset() function (and then adding a
> > qemu_devices_reset() call to *-user) than having a fake
> > qemu_register_reset() function and having to use a different API to
> > reset the CPUs on on *-user.
> There is no real need to add anything, *-user uses pretty much well defined
> cpu_reset() and doesn't need anything else to improve it.

cpu_reset() is not that well-defined, otherwise we wouldn't have this on
linux-user:

#if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
    cpu_reset(ENV_GET_CPU(env));
#endif

(I have no idea why we have that #ifdef).

But that can be fixed, eventually, and:

> 
> PS:
> reset-handler callbacks is not much loved thing in qemu, and people would
> like to get rid of it eventually. If one checks target-i386/cpu.c:
> 
> /* TODO: remove me, when reset over QOM tree is implemented */
> static void x86_cpu_machine_reset_cb(void *opaque)
> 
> some day in far far (or maybe not far) future there won't be reset cb for
> x86-cpu.
> Could you just leave reset-handler be a *-softmmu only part.

I was considering this as well: even if cpu_reset() is not in great
shape, qemu_register_reset() is something we would like to kill as well.

I sent a v8 DeviceState series including reset.o on *-user, already, but
I will try removing it on a v9 series, next week.


> > 
> > > 
> > > Andreas
> > > 
> > > > +
> > > > +void qemu_devices_reset(void)
> > > > +{
> > > > +    QEMUResetEntry *re, *nre;
> > > > +
> > > > +    /* reset all devices */
> > > > +    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > > > +        re->func(re->opaque);
> > > > +    }
> > > > +}
> > > > +
> > > >  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
> > > >  {
> > > >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > > > diff --git a/hw/qdev.h b/hw/qdev.h
> > > > index 365b8d6..2487b3b 100644
> > > > --- a/hw/qdev.h
> > > > +++ b/hw/qdev.h
> > > > @@ -5,5 +5,6 @@
> > > >  #include "qdev-core.h"
> > > >  #include "qdev-properties.h"
> > > >  #include "qdev-monitor.h"
> > > > +#include "qdev-reset.h"
> > > >  
> > > >  #endif
> > > > diff --git a/sysemu.h b/sysemu.h
> > > > index ab1ef8b..51f19cc 100644
> > > > --- a/sysemu.h
> > > > +++ b/sysemu.h
> > > > @@ -57,7 +57,6 @@ void qemu_system_vmstop_request(RunState reason);
> > > >  int qemu_shutdown_requested_get(void);
> > > >  int qemu_reset_requested_get(void);
> > > >  void qemu_system_killed(int signal, pid_t pid);
> > > > -void qemu_devices_reset(void);
> > > >  void qemu_system_reset(bool report);
> > > >  
> > > >  void qemu_add_exit_notifier(Notifier *notify);
> > > > diff --git a/vl.c b/vl.c
> > > > index 4f03a72..c7448a2 100644
> > > > --- a/vl.c
> > > > +++ b/vl.c
> > > > @@ -1456,14 +1456,6 @@ void vm_start(void)
> > > >  
> > > >  /* reset/shutdown handler */
> > > >  
> > > > -typedef struct QEMUResetEntry {
> > > > -    QTAILQ_ENTRY(QEMUResetEntry) entry;
> > > > -    QEMUResetHandler *func;
> > > > -    void *opaque;
> > > > -} QEMUResetEntry;
> > > > -
> > > > -static QTAILQ_HEAD(reset_handlers, QEMUResetEntry) reset_handlers =
> > > > -    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> > > >  static int reset_requested;
> > > >  static int shutdown_requested, shutdown_signal = -1;
> > > >  static pid_t shutdown_pid;
> > > > @@ -1560,38 +1552,6 @@ static bool qemu_vmstop_requested(RunState *r)
> > > >      return false;
> > > >  }
> > > >  
> > > > -void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> > > > -{
> > > > -    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> > > > -
> > > > -    re->func = func;
> > > > -    re->opaque = opaque;
> > > > -    QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> > > > -}
> > > > -
> > > > -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> > > > -{
> > > > -    QEMUResetEntry *re;
> > > > -
> > > > -    QTAILQ_FOREACH(re, &reset_handlers, entry) {
> > > > -        if (re->func == func && re->opaque == opaque) {
> > > > -            QTAILQ_REMOVE(&reset_handlers, re, entry);
> > > > -            g_free(re);
> > > > -            return;
> > > > -        }
> > > > -    }
> > > > -}
> > > > -
> > > > -void qemu_devices_reset(void)
> > > > -{
> > > > -    QEMUResetEntry *re, *nre;
> > > > -
> > > > -    /* reset all devices */
> > > > -    QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> > > > -        re->func(re->opaque);
> > > > -    }
> > > > -}
> > > > -
> > > >  void qemu_system_reset(bool report)
> > > >  {
> > > >      if (current_machine && current_machine->reset) {
> > > > 
> > > 
> > > 
> > > -- 
> > > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> > 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c

[Peter Maydell]

On 30 November 2012 21:38, Eduardo Habkost <ehabkost@redhat.com> wrote:
> cpu_reset() is not that well-defined, otherwise we wouldn't have this on
> linux-user:
>
> #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>     cpu_reset(ENV_GET_CPU(env));
> #endif
>
> (I have no idea why we have that #ifdef).

I think this is because the different targets disagree about whether
the CPU should be reset on initial construction or whether it needs
a specific reset call. (The current setup with #ifdefs is among other
things a historical effect as a result of various refactorings in
the past; you can trace the git history if you're interested.)

-- PMM

Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c

[Andreas Färber]

Am 01.12.2012 12:26, schrieb Peter Maydell:
> On 30 November 2012 21:38, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> cpu_reset() is not that well-defined, otherwise we wouldn't have this on
>> linux-user:
>>
>> #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>>     cpu_reset(ENV_GET_CPU(env));
>> #endif
>>
>> (I have no idea why we have that #ifdef).
> 
> I think this is because the different targets disagree about whether
> the CPU should be reset on initial construction or whether it needs
> a specific reset call. (The current setup with #ifdefs is among other
> things a historical effect as a result of various refactorings in
> the past; you can trace the git history if you're interested.)

Peter and me had long IRC discussions about how to fix this in the past:

* On my qom-cpu-copy branch I have a patch queued that drops the
  #ifdef above, accepting that CPUs may get reset twice then.
  => Dispels doubt for target authors; doubts about correctness though.

* PMM suggested to move cpu_clone_regs() from target-*/cpu.h to *-user/.
  => Would lead to duplication between linux-user and bsd-user; ABI?

* PMM suggested to replace cpu_copy() with ABI-specific code in *-user/.
  Unfortunately I don't quite remember the details of how... ;)

The x86 APIC refactorings that Iguardo have done do bring us very close
to sane cpu_reset() semantics (ignoring the two hands full of
hard/soft/... reset variants that ppc and other architectures feature).

Declaring cpu_reset() inferior to reset handlers due to the linux-user
mess is going into the wrong direction - some targets seem to ignore
reset or fork/clone completely at the moment, so the state we see cannot
be considered fully correct.
In particular the above reset is being performed *after* cpu_copy()
memcpy()'ed random memory contents (which for some targets may contain
pointers), undoing the copying in large parts. Therefore, when all
targets reset as part of cpu_init() (or by moving the cpu_reset() call
into early cpu_copy()?) we could get rid of it in do_fork() and of its
weird conditions.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 19/24] qdev: move reset handler list from vl.c to qdev.c

[Peter Maydell]

On 2 December 2012 05:44, Andreas Färber <afaerber@suse.de> wrote:
> Am 01.12.2012 12:26, schrieb Peter Maydell:
>> On 30 November 2012 21:38, Eduardo Habkost <ehabkost@redhat.com> wrote:
>>> cpu_reset() is not that well-defined, otherwise we wouldn't have this on
>>> linux-user:
>>>
>>> #if defined(TARGET_I386) || defined(TARGET_SPARC) || defined(TARGET_PPC)
>>>     cpu_reset(ENV_GET_CPU(env));
>>> #endif
>>>
>>> (I have no idea why we have that #ifdef).
>>
>> I think this is because the different targets disagree about whether
>> the CPU should be reset on initial construction or whether it needs
>> a specific reset call. (The current setup with #ifdefs is among other
>> things a historical effect as a result of various refactorings in
>> the past; you can trace the git history if you're interested.)
>
> Peter and me had long IRC discussions about how to fix this in the past:
>
> * On my qom-cpu-copy branch I have a patch queued that drops the
>   #ifdef above, accepting that CPUs may get reset twice then.
>   => Dispels doubt for target authors; doubts about correctness though.

So on its own, this will break things, because the cpu_reset() happens
after the cpu_copy, so we'll end up not in fact copying most of the
registers across from the original thread. In fact this looks broken
for i386/sparc/ppc at the moment, and is probably only working to the
extent that user programs don't actually care about the register state
across the clone() syscall. [and i386 threads are busted anyhow.]
If we want to retain cpu_copy for the moment, the correct place for
the cpu_reset() is in cpu_copy() itself, after the cpu_init but before
the memcpy. [I see actually you mention this later in your email.]

> * PMM suggested to move cpu_clone_regs() from target-*/cpu.h to *-user/.
>   => Would lead to duplication between linux-user and bsd-user; ABI?

clone_regs should definitely be in *-user/, because it is in fact
ABI dependent -- which register the 0 return value from the clone()
syscall ends up in is up to the ABI. Also, there won't in fact be
any duplication problem here because bsd-user doesn't implement
threads and doesn't call cpu_clone_regs().

> * PMM suggested to replace cpu_copy() with ABI-specific code in *-user/.
>   Unfortunately I don't quite remember the details of how... ;)

I don't know that I got as far as the details, but the general idea
is that the CPU state that needs to be propagated to the CPU in the
new thread is only the user-space-facing bits, which *-user/ already
needs to know about (and fish directly in CPUState for) to get things
like signal handlers right. So rather than doing a huge memcpy of
all the CPU's internal state, we just create and reset a CPU as normal,
and then have the *-user/ code copy across those bits of register
state which matter to userspace. I think this is much cleaner because
it means the CPU emulation itself has a simple interface ("create
cpu" and "reset cpu") and the details of copying thread state across
are restricted to the user-mode code.

-- PMM

Re: [Qemu-devel] [PATCH 18/24] qdev: qdev_create(): use error_report() instead of hw_error()

[Andreas Färber]

Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> hw_error() is specific for fatal hardware emulation errors, not for
> internal errors related to the qdev object/class abstraction or object
> initialization.
> 
> Replace it with an error_report() call, followed by abort().
> 
> This will also help reduce dependencies of the qdev code (as hw_error()
> is from cpus.o, and depends on the CPU list from exec.o).
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg