Patchwork [RFC,6/8] qdev-properties.c: separate core from the code used only by qemu-system-*

login
register
mail settings
Submitter Eduardo Habkost
Date Dec. 4, 2012, 1:19 p.m.
Message ID <1354627180-25704-7-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/203633/
State New
Headers show

Comments

Eduardo Habkost - Dec. 4, 2012, 1:19 p.m.
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>
---
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

Changes v5 -> v6 (ehabkost):
 - Removed inter-SoB line changelog from commit message
---
 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
Blue Swirl - Dec. 4, 2012, 6:55 p.m.
On Tue, Dec 4, 2012 at 1:19 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> 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>
> ---
> 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
>
> Changes v5 -> v6 (ehabkost):
>  - Removed inter-SoB line changelog from commit message
> ---
>  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 d581d8d..96a8365 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -185,6 +185,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)

Please add braces, use checkpatch.pl.

> +        return -ENOENT;
> +    if (bdrv_attach_dev(bs, dev) < 0)

Also here.

> +        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);

Ditto.

> +    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 599382c..fa0af21 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"
> @@ -312,18 +311,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
>
>
Eduardo Habkost - Dec. 4, 2012, 7:01 p.m.
On Tue, Dec 04, 2012 at 06:55:17PM +0000, Blue Swirl wrote:
> On Tue, Dec 4, 2012 at 1:19 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> > 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>
> > ---
> > 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
> >
> > Changes v5 -> v6 (ehabkost):
> >  - Removed inter-SoB line changelog from commit message
> > ---
> >  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 d581d8d..96a8365 100644
> > --- a/hw/Makefile.objs
> > +++ b/hw/Makefile.objs
> > @@ -185,6 +185,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)
> 
> Please add braces, use checkpatch.pl.
> 
> > +        return -ENOENT;
> > +    if (bdrv_attach_dev(bs, dev) < 0)
> 
> Also here.


This is pure code movement, and I won't introduce changes in the code
while it is being moved to not make patch review harder.

If you want to send coding style patches for that code after it is
moved, be my guest.


> 
> > +        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);
> 
> Ditto.
> 
> > +    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 599382c..fa0af21 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"
> > @@ -312,18 +311,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
> >
> >
>
Blue Swirl - Dec. 4, 2012, 7:04 p.m.
On Tue, Dec 4, 2012 at 7:01 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Tue, Dec 04, 2012 at 06:55:17PM +0000, Blue Swirl wrote:
>> On Tue, Dec 4, 2012 at 1:19 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> > 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>
>> > ---
>> > 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
>> >
>> > Changes v5 -> v6 (ehabkost):
>> >  - Removed inter-SoB line changelog from commit message
>> > ---
>> >  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 d581d8d..96a8365 100644
>> > --- a/hw/Makefile.objs
>> > +++ b/hw/Makefile.objs
>> > @@ -185,6 +185,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)
>>
>> Please add braces, use checkpatch.pl.
>>
>> > +        return -ENOENT;
>> > +    if (bdrv_attach_dev(bs, dev) < 0)
>>
>> Also here.
>
>
> This is pure code movement, and I won't introduce changes in the code
> while it is being moved to not make patch review harder.

That's OK.

> If you want to send coding style patches for that code after it is
> moved, be my guest.

But that would not be OK because git blame information would be lost,
otherwise we could just reformat everything.

Please fix the code first as the first patch, then move.

>
>
>>
>> > +        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);
>>
>> Ditto.
>>
>> > +    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 599382c..fa0af21 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"
>> > @@ -312,18 +311,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
>> >
>> >
>>
>
> --
> Eduardo
Andreas Färber - Dec. 4, 2012, 7:05 p.m.
Am 04.12.2012 20:01, schrieb Eduardo Habkost:
> On Tue, Dec 04, 2012 at 06:55:17PM +0000, Blue Swirl wrote:
>> On Tue, Dec 4, 2012 at 1:19 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
>>> +static int parse_drive(DeviceState *dev, const char *str, void **ptr)
>>> +{
>>> +    BlockDriverState *bs;
>>> +
>>> +    bs = bdrv_find(str);
>>> +    if (bs == NULL)
>>
>> Please add braces, use checkpatch.pl.
>>
>>> +        return -ENOENT;
>>> +    if (bdrv_attach_dev(bs, dev) < 0)
>>
>> Also here.
> 
> 
> This is pure code movement, and I won't introduce changes in the code
> while it is being moved to not make patch review harder.

> If you want to send coding style patches for that code after it is
> moved, be my guest.

No. Patches are required to pass checkpatch.pl (i.e., + lines in the
patch need braces). That means if not in the same patch for movement
reasons, Coding Style cleanups need to be done in advance, not as
followup. (same issue as in his AREG0 series)

Andreas

P.S. Dropping unrelated parts of the quotes makes requested changes
easier to spot. There was a ditto hidden somewhere down. ;)
Eduardo Habkost - Dec. 4, 2012, 7:13 p.m.
On Tue, Dec 04, 2012 at 08:05:43PM +0100, Andreas Färber wrote:
> Am 04.12.2012 20:01, schrieb Eduardo Habkost:
> > On Tue, Dec 04, 2012 at 06:55:17PM +0000, Blue Swirl wrote:
> >> On Tue, Dec 4, 2012 at 1:19 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> >>> +static int parse_drive(DeviceState *dev, const char *str, void **ptr)
> >>> +{
> >>> +    BlockDriverState *bs;
> >>> +
> >>> +    bs = bdrv_find(str);
> >>> +    if (bs == NULL)
> >>
> >> Please add braces, use checkpatch.pl.
> >>
> >>> +        return -ENOENT;
> >>> +    if (bdrv_attach_dev(bs, dev) < 0)
> >>
> >> Also here.
> > 
> > 
> > This is pure code movement, and I won't introduce changes in the code
> > while it is being moved to not make patch review harder.
> 
> > If you want to send coding style patches for that code after it is
> > moved, be my guest.
> 
> No. Patches are required to pass checkpatch.pl (i.e., + lines in the
> patch need braces). That means if not in the same patch for movement
> reasons, Coding Style cleanups need to be done in advance, not as
> followup. (same issue as in his AREG0 series)

I had the feeling that the amount of cleanup (included but not limited
to coding style changes) I was doing before introducing actual code
changes was making some of my series harder to review. If making coding
style changes first is preferred, I can do it. I will try to send v10 of
this series today.

> 
> Andreas
> 
> P.S. Dropping unrelated parts of the quotes makes requested changes
> easier to spot. There was a ditto hidden somewhere down. ;)

ACK.  :)

Patch

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index d581d8d..96a8365 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -185,6 +185,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 599382c..fa0af21 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"
@@ -312,18 +311,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;