[Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

Hi,

This series is the rough beginnings of the QEMU Object Model.  This is basically
qdev generalized on steroids.

This series includes the core infrastructure, a strawman Device type, and
the beginnings of the conversion of CharDriverState.  This is in a rougher
state than I would like it to be but I wanted to get the concepts on the list
as soon as possible.

My plan is to drop the Device parts from future versions of the series and just
focus on backends to start with.

Please note that this series has an awful lot of ramifications.  Most of our
current command line options would become deprecated, the build system will
change significantly, and a lot of our QMP functions will become deprecated.

It seems like a lot of change, but hopefully this series illustrates how we
can do it very incrementally with value being added at each stage of the
conversion.

[Qemu-devel] [PATCH 01/21] qom: add make infrastructure

[Anthony Liguori]

QOM provides a unified model for separating modules into independently buildable
components.  To fully take advantage of this, we need a bit more make
infrastructure that lets us describe these modules and their dependencies.
Ultimately this will enable us to support tristate modules that can be demand
loaded as shared libraries.

For now, only support boolean modules and don't do any dependency resolution.  I
expect that we'll figure out how to borrow the kernel's Kconfig processing
scripts to make this all work.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.objs        |    3 +++
 Makefile.qom         |    9 +++++++++
 Qconfig              |    1 +
 scripts/genconfig.sh |   30 ++++++++++++++++++++++++++++++
 4 files changed, 43 insertions(+), 0 deletions(-)
 create mode 100644 Makefile.qom
 create mode 100644 Qconfig
 create mode 100755 scripts/genconfig.sh

diff --git a/Makefile.objs b/Makefile.objs
index 6991a9f..9cc87fd 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -158,6 +158,9 @@ common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
 common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
 common-obj-$(CONFIG_XEN_BACKEND) += xen_console.o xenfb.o xen_disk.o xen_nic.o
 
+# QEMU Object Model
+include $(SRC_PATH)/Makefile.qom
+
 ######################################################################
 # libuser
 
diff --git a/Makefile.qom b/Makefile.qom
new file mode 100644
index 0000000..8b14952
--- /dev/null
+++ b/Makefile.qom
@@ -0,0 +1,9 @@
+QEMU_CFLAGS += -I$(SRC_PATH)/include
+
+QCONFIGS = $(shell find $(SRC_PATH) -name "Qconfig" -print)
+
+config-qom.mak: $(SRC_PATH)/Qconfig $(QCONFIGS)
+	$(SRC_PATH)/scripts/genconfig.sh $< > $@
+
+-include config-qom.mak
+
diff --git a/Qconfig b/Qconfig
new file mode 100644
index 0000000..62b15d7
--- /dev/null
+++ b/Qconfig
@@ -0,0 +1 @@
+# Do nothing for now
diff --git a/scripts/genconfig.sh b/scripts/genconfig.sh
new file mode 100755
index 0000000..14fef23
--- /dev/null
+++ b/scripts/genconfig.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+#
+# Simple Qconfig parser
+#
+# Copyright IBM, Corp. 2011
+#
+# Authors:
+#  Anthony Liguori   <aliguori@us.ibm.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+# This is just intended as a placeholder.  Please do not enhance this script.
+# Instead, please try to port the Linux kernel's Kconfig parser.
+
+process() {
+    local basedir=`dirname $1`
+    cat "$1" | while read LINE; do
+	local first_word=`echo $LINE | cut -f1 -d' '`
+	if test "$first_word" = "config"; then
+	    echo $LINE | sed -e 's:^config \(.*\):CONFIG_\1=y:g'
+	elif test "$first_word" = "source"; then
+	    local rest=`echo $LINE | cut -f2- -d' '`
+	    process "$basedir/$rest"
+	fi
+    done
+}
+
+process "$1"
-- 
1.7.4.1

[Qemu-devel] [PATCH 02/21] qom: convert QAPI to use Qconfig build system

[Anthony Liguori]

Plug depends on QAPI and in order to express that dependency, QAPI needs to use
the Qconfig build system.  Right now, QAPI is only built for guest agent, but
this changes the generic build to now include QAPI.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile      |   10 ++++++----
 Makefile.objs |    4 ----
 Makefile.qom  |    2 ++
 Qconfig       |    2 +-
 qapi/Makefile |    3 +++
 qapi/Qconfig  |   19 +++++++++++++++++++
 6 files changed, 31 insertions(+), 9 deletions(-)
 create mode 100644 qapi/Makefile
 create mode 100644 qapi/Qconfig

diff --git a/Makefile b/Makefile
index f3a03ad..81d864f 100644
--- a/Makefile
+++ b/Makefile
@@ -185,16 +185,18 @@ $(qapi-dir)/qga-qapi-visit.h: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scr
 $(qapi-dir)/qga-qmp-marshal.c: $(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
 	$(call quiet-command,python $(SRC_PATH)/scripts/qapi-commands.py -o "$(qapi-dir)" -p "qga-" < $<, "  GEN   $@")
 
-test-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
+qga-obj-y := $(addprefix qapi/,$(qapi-obj-y))
+
+test-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qga-obj-y)
 test-visitor: test-visitor.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qapi-obj-y) error.o osdep.o qemu-malloc.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
 
 test-qmp-commands.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h test-qmp-marshal.c test-qmp-commands.h) $(qapi-obj-y)
-test-qmp-commands: test-qmp-commands.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qapi-obj-y) error.o osdep.o qemu-malloc.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o $(qapi-dir)/test-qmp-marshal.o module.o
+test-qmp-commands: test-qmp-commands.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o $(qga-obj-y) error.o osdep.o qemu-malloc.o $(oslib-obj-y) qjson.o json-streamer.o json-lexer.o json-parser.o qerror.o qemu-error.o qemu-tool.o $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o $(qapi-dir)/test-qmp-marshal.o module.o
 
 QGALIB=qga/guest-agent-command-state.o qga/guest-agent-commands.o
 
-qemu-ga.o: $(addprefix $(qapi-dir)/, qga-qapi-types.c qga-qapi-types.h qga-qapi-visit.c qga-qmp-marshal.c) $(qapi-obj-y)
-qemu-ga$(EXESUF): qemu-ga.o $(QGALIB) qemu-tool.o qemu-error.o error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) $(qapi-obj-y) qemu-timer-common.o qemu-sockets.o module.o qapi/qmp-dispatch.o qapi/qmp-registry.o $(qapi-dir)/qga-qapi-visit.o $(qapi-dir)/qga-qapi-types.o $(qapi-dir)/qga-qmp-marshal.o
+qemu-ga.o: $(addprefix $(qapi-dir)/, qga-qapi-types.c qga-qapi-types.h qga-qapi-visit.c qga-qmp-marshal.c) $(qga-obj-y)
+qemu-ga$(EXESUF): qemu-ga.o $(QGALIB) qemu-tool.o qemu-error.o error.o $(oslib-obj-y) $(trace-obj-y) $(block-obj-y) $(qobject-obj-y) $(version-obj-y) $(qga-obj-y) qemu-timer-common.o qemu-sockets.o module.o qapi/qmp-dispatch.o qapi/qmp-registry.o $(qapi-dir)/qga-qapi-visit.o $(qapi-dir)/qga-qapi-types.o $(qapi-dir)/qga-qmp-marshal.o
 
 QEMULIBS=libhw32 libhw64 libuser libdis libdis-user
 
diff --git a/Makefile.objs b/Makefile.objs
index 9cc87fd..ef6030a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -379,10 +379,6 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
 ######################################################################
 # qapi
 
-qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o qmp-output-visitor.o qapi-dealloc-visitor.o
-qapi-nested-y += qmp-registry.o qmp-dispatch.o
-qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
-
 vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
 
 vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
diff --git a/Makefile.qom b/Makefile.qom
index 8b14952..1b06970 100644
--- a/Makefile.qom
+++ b/Makefile.qom
@@ -7,3 +7,5 @@ config-qom.mak: $(SRC_PATH)/Qconfig $(QCONFIGS)
 
 -include config-qom.mak
 
+include $(SRC_PATH)/qapi/Makefile
+common-obj-y += $(addprefix qapi/,$(qapi-obj-y))
diff --git a/Qconfig b/Qconfig
index 62b15d7..cdf8f6c 100644
--- a/Qconfig
+++ b/Qconfig
@@ -1 +1 @@
-# Do nothing for now
+source qapi/Qconfig
diff --git a/qapi/Makefile b/qapi/Makefile
new file mode 100644
index 0000000..11efdfe
--- /dev/null
+++ b/qapi/Makefile
@@ -0,0 +1,3 @@
+qapi-obj-$(CONFIG_QAPI) += qapi-dealloc-visitor.o qapi-visit-core.o
+qapi-obj-$(CONFIG_QAPI_QMP) += qmp-input-visitor.o qmp-output-visitor.o
+qapi-obj-$(CONFIG_QAPI_QMP_SERVER) += qmp-dispatch.o qmp-registry.o
diff --git a/qapi/Qconfig b/qapi/Qconfig
new file mode 100644
index 0000000..aaa9f01
--- /dev/null
+++ b/qapi/Qconfig
@@ -0,0 +1,19 @@
+config QAPI
+       bool "QEMU API Support"
+       default y
+       help
+         This provides a generic marshalling framework for converting C types
+         to other data structures.  If unsure, say y here.
+
+config QAPI_QMP
+       bool "QAPI support for QObjects"
+       default y
+       help
+         This allows QAPI to convert to and from QObjects.  QObjects are mainly
+         used to marshal to and from JSON.
+
+config QAPI_QMP_SERVER
+       bool "QAPI based QMP server"
+       default y
+       help
+         This provides a QMP server framework using QAPI.
-- 
1.7.4.1

[Qemu-devel] [PATCH 03/21] qom: Add core type system

[Anthony Liguori]

This patch implements the basic type system used by QEMU Object Model.  This
infrastructure supports the registration of classes and instantiation of
objects through a generic factory interface.  Classes support polymophism and
single inheritance.

Interfaces can also be used to approximate multiple inheritance.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.qom        |    4 +
 Qconfig             |    1 +
 configure           |    2 +-
 include/qemu/type.h |  513 ++++++++++++++++++++++++++++++++++++++++++++++++++
 qom/Makefile        |    1 +
 qom/Qconfig         |    6 +
 qom/type.c          |  523 +++++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 1049 insertions(+), 1 deletions(-)
 create mode 100644 include/qemu/type.h
 create mode 100644 qom/Makefile
 create mode 100644 qom/Qconfig
 create mode 100644 qom/type.c

diff --git a/Makefile.qom b/Makefile.qom
index 1b06970..34f1f91 100644
--- a/Makefile.qom
+++ b/Makefile.qom
@@ -9,3 +9,7 @@ config-qom.mak: $(SRC_PATH)/Qconfig $(QCONFIGS)
 
 include $(SRC_PATH)/qapi/Makefile
 common-obj-y += $(addprefix qapi/,$(qapi-obj-y))
+
+include $(SRC_PATH)/qom/Makefile
+common-obj-y += $(addprefix qom/,$(qom-obj-y))
+
diff --git a/Qconfig b/Qconfig
index cdf8f6c..889dfa6 100644
--- a/Qconfig
+++ b/Qconfig
@@ -1 +1,2 @@
 source qapi/Qconfig
+source qom/Qconfig
diff --git a/configure b/configure
index 600da9b..93e5e97 100755
--- a/configure
+++ b/configure
@@ -3516,7 +3516,7 @@ DIRS="$DIRS pc-bios/spapr-rtas"
 DIRS="$DIRS roms/seabios roms/vgabios"
 DIRS="$DIRS fsdev ui"
 DIRS="$DIRS qapi"
-DIRS="$DIRS qga"
+DIRS="$DIRS qga qom"
 FILES="Makefile tests/Makefile"
 FILES="$FILES tests/cris/Makefile tests/cris/.gdbinit"
 FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps"
diff --git a/include/qemu/type.h b/include/qemu/type.h
new file mode 100644
index 0000000..170f485
--- /dev/null
+++ b/include/qemu/type.h
@@ -0,0 +1,513 @@
+/*
+ * QEMU Object Model
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_TYPE_H
+#define QEMU_TYPE_H
+
+#include "qemu-common.h"
+#include <glib.h>
+
+#define MAX_ID (32 + 1)
+
+typedef uint64_t Type;
+
+typedef struct TypeClass TypeClass;
+typedef struct TypeInstance TypeInstance;
+typedef struct TypeInfo TypeInfo;
+
+typedef struct InterfaceClass InterfaceClass;
+typedef struct Interface Interface;
+typedef struct InterfaceInfo InterfaceInfo;
+
+/**
+ * @TypeClass:
+ *
+ * The base for all classes.  The only thing that @TypeClass contains is an
+ * integer type handle.
+ */
+struct TypeClass
+{
+    /**
+     * @type the handle of the type for a class
+     */
+    Type type;
+};
+
+/**
+ * @TypeInstance:
+ *
+ * The base for all objects.  The first member of this object is a pointer to
+ * a @TypeClass.  Since C guarantees that the first member of a structure
+ * always begins at byte 0 of that structure, as long as any sub-object places
+ * its parent as the first member, we can cast directly to a @TypeInstance.
+ *
+ * As a result, @TypeInstance contains a reference to the objects type as its
+ * first member.  This allows identification of the real type of the object at
+ * run time.
+ *
+ * @TypeInstance also contains a list of @Interfaces that this object
+ * implements.
+ */
+struct TypeInstance
+{
+    /**
+     * @class the type of the instantiated object.
+     */
+    TypeClass *class;
+
+    /**
+     * @id the name of the object
+     */
+    char id[MAX_ID];
+
+    /**
+     * @interfaces a list of @Interface objects implemented by this object
+     */
+    GSList *interfaces;
+};
+
+/**
+ * @TypeInfo:
+ *
+ */
+struct TypeInfo
+{
+    /**
+     * @name the name of the type
+     */
+    const char *name;
+
+    /**
+     * @parent the name of the parent type
+     */
+    const char *parent;
+
+    /**
+     * Instance Initialization
+     *
+     * This functions manage the instance construction and destruction of a
+     * type.
+     */
+
+    /**
+     * @instance_size the size of the object (derivative of @TypeInstance).  If
+     * @instance_size is 0, then the size of the object will be the size of the
+     * parent object.
+     */
+    size_t instance_size;
+
+    /**
+     * @instance_init
+     *
+     * This function is called to initialize an object.  The parent class will
+     * have already been initialized so the type is only responsible for
+     * initializing its own members.
+     */
+    void (*instance_init)(TypeInstance *obj);
+
+    /**
+     * @instance_finalize
+     *
+     * This function is called during object destruction.  This is called before
+     * the parent @instance_finalize function has been called.  An object should
+     * only free the members that are unique to its type in this function.
+     */
+    void (*instance_finalize)(TypeInstance *obj);
+
+    /**
+     * @abstract
+     *
+     * If this field is true, then the class is considered abstract and cannot
+     * be directly instantiated.
+     */
+    bool abstract;
+
+    /**
+     * Class Initialization
+     *
+     * Before an object is initialized, the class for the object must be
+     * initialized.  There is only one class object for all instance objects
+     * that is created lazily.
+     *
+     * Classes are initialized by first initializing any parent classes (if
+     * necessary).  After the parent class object has initialized, it will be
+     * copied into the current class object and any additional storage in the
+     * class object is zero filled.
+     *
+     * The effect of this is that classes automatically inherit any virtual
+     * function pointers that the parent class has already initialized.  All
+     * other fields will be zero filled.
+     *
+     * After this initial copy, @base_init is invoked.  This is meant to handle
+     * the case where a class may have a dynamic field that was copied via
+     * a shallow copy but needs to be deep copied.  @base_init is called for
+     * each parent class but not for the class being instantiated.
+     *
+     * Once all of the parent classes have been initialized and their @base_init
+     * functions have been called, @class_init is called to let the class being
+     * instantiated provide default initialize for it's virtual functions.
+     */
+
+    /**
+     * @class_size the size of the class object (derivative of @TypeClass) for
+     * this object.  If @class_size is 0, then the size of the class will be
+     * assumed to be the size of the parent class.  This allows a type to avoid
+     * implementing an explicit class type if they are not adding additional
+     * virtual functions.
+     */
+    size_t class_size;
+
+    /**
+     * @base_init
+     *
+     * This function is called after memcpy()'ing the base class into the new
+     * class to reinitialize any members that require deep copy.
+     */
+    void (*base_init)(TypeClass *klass);
+
+    /**
+     * @base_finalize
+     *
+     * This function is called during a class's destruction and is meant to
+     * allow any dynamic parameters allocated by @base_init to be released.
+     */
+    void (*base_finalize)(TypeClass *klass);
+
+    /**
+     * @class_init
+     *
+     * This function is called after all parent class initialization has occured
+     * to allow a class to set its default virtual method pointers.  This is
+     * also the function to use to override virtual methods from a parent class.
+     */
+    void (*class_init)(TypeClass *klass);
+
+    /**
+     * @class_finalize
+     *
+     * This function is called during class destruction and is meant to release
+     * and dynamic parameters allocated by @class_init.
+     */
+    void (*class_finalize)(TypeClass *klass);
+
+    /**
+     * Interfaces
+     *
+     * Interfaces allow a limited form of multiple inheritance.  Instances are
+     * similar to normal types except for the fact that are only defined by
+     * their classes and never carry any state.  You can cast an object to one
+     * of its @Interface types and vice versa.
+     */
+
+    /**
+     * @interfaces the list of interfaces associated with this type.  This
+     * should point to a static array that's terminated with a zero filled
+     * element.
+     */
+    InterfaceInfo *interfaces;
+};
+
+/**
+ * @TYPE_INSTANCE
+ *
+ * Converts an object to a @TypeInstance.  Since all objects are @TypeInstances,
+ * this function will always succeed.
+ */
+#define TYPE_INSTANCE(obj) \
+    ((TypeInstance *)(obj))
+
+/**
+ * @TYPE_CHECK
+ *
+ * A type safe version of @type_dynamic_cast_assert.  Typically each class will
+ * define a macro based on this type to perform type safe dynamic_casts to
+ * this object type.
+ *
+ * If an invalid object is passed to this function, a run time assert will be
+ * generated.
+ */
+#define TYPE_CHECK(type, obj, name) \
+    ((type *)type_dynamic_cast_assert((TypeInstance *)(obj), (name)))
+
+/**
+ * @TYPE_CLASS_CHECK
+ *
+ * A type safe version of @type_check_class.  This macro is typically wrapped
+ * by each type to perform type safe casts of a class to a specific class type.
+ */
+#define TYPE_CLASS_CHECK(class, obj, name) \
+    ((class *)type_check_class((TypeClass *)(obj), (name)))
+
+/**
+ * @TYPE_GET_CLASS
+ *
+ * This function will return a specific class for a given object.  Its generally
+ * used by each type to provide a type safe macro to get a specific class type
+ * from an object.
+ */
+#define TYPE_GET_CLASS(class, obj, name) \
+    TYPE_CLASS_CHECK(class, type_get_class(TYPE_INSTANCE(obj)), name)
+
+/**
+ * @Interface:
+ *
+ * The base for all Interfaces.  This is a subclass of TypeInstance.  Subclasses
+ * of @Interface should never have an instance that contains anything other than
+ * a single @Interface member.
+ */ 
+struct Interface
+{
+    /**
+     * @parent base class
+     */
+    TypeInstance parent;
+
+    /* private */
+
+    /**
+     * @obj a pointer to the object that implements this interface.  This is
+     * used to allow casting from an interface to the base object.
+     */
+    TypeInstance *obj;
+};
+
+/**
+ * @InterfaceClass:
+ *
+ * The class for all interfaces.  Subclasses of this class should only add
+ * virtual methods.
+ */
+struct InterfaceClass
+{
+    /**
+     * @parent_class the base class
+     */
+    TypeClass parent_class;
+};
+
+/**
+ * @InterfaceInfo:
+ *
+ * The information associated with an interface.
+ */
+struct InterfaceInfo
+{
+    /**
+     * @type the name of the interface
+     */
+    const char *type;
+
+    /**
+     * @interface_initfn is called during class initialization and is used to
+     * initialize an interface associated with a class.  This function should
+     * initialize any default virtual functions for a class and/or override
+     * virtual functions in a parent class.
+     */
+    void (*interface_initfn)(TypeClass *class);
+};
+
+#define TYPE_INTERFACE "interface"
+#define INTERFACE(obj) TYPE_CHECK(Interface, obj, TYPE_INTERFACE)
+
+/**
+ * @type_new:
+ *
+ * This function will initialize a new object using heap allocated memory.  This
+ * function should be paired with @type_delete to free the resources associated
+ * with the object.
+ *
+ * @typename: The name of the type of the object to instantiate
+ *
+ * @id:       The id of the object.  This must be unique.
+ *
+ * Returns:   The newly allocated and instantiated object.
+ *
+ */
+TypeInstance *type_new(const char *typename, const char *id);
+
+/**
+ * @type_delete:
+ *
+ * Finalize an object and then free the memory associated with it.  This should
+ * be paired with @type_new to free the resources associated with an object.
+ *
+ * @obj:  The object to free.
+ *
+ */
+void type_delete(TypeInstance *obj);
+
+/**
+ * @type_initialize:
+ *
+ * This function will initialize an object.  The memory for the object should
+ * have already been allocated.
+ *
+ * @obj:      A pointer to the memory to be used for the object.
+ *
+ * @typename: The name of the type of the object to instantiate
+ *
+ * @id:       The id of the object.  This must be unique.
+ *
+ */
+void type_initialize(void *obj, const char *typename, const char *id);
+
+/**
+ * @type_finalize:
+ *
+ * This function destroys and object without freeing the memory associated with
+ * it.
+ *
+ * @obj:
+ *
+ */
+void type_finalize(void *obj);
+
+/**
+ * @type_dynamic_cast:
+ *
+ * This function will determine if @obj is-a @typename.  @obj can refer to an
+ * object or an interface associated with an object.
+ *
+ * @obj:       The object to cast.
+ *
+ * @typename:  The @typename
+ *
+ * Returns:
+ *
+ */
+TypeInstance *type_dynamic_cast(TypeInstance *obj, const char *typename);
+
+/**
+ * @type_dynamic_cast_assert:
+ *
+ * @obj:
+ *
+ * @typename:
+ *
+ * Returns:
+ *
+ */
+TypeInstance *type_dynamic_cast_assert(TypeInstance *obj, const char *typename);
+
+/**
+ * @type_is_type:
+ *
+ * @obj:
+ *
+ * @typename:
+ *
+ * Returns:
+ *
+ */
+bool type_is_type(TypeInstance *obj, const char *typename);
+
+/**
+ * @type_get_class:
+ *
+ * @obj:
+ *
+ * Returns:
+ *
+ */
+TypeClass *type_get_class(TypeInstance *obj);
+
+/**
+ * @type_get_id:
+ *
+ * @obj:
+ *
+ * Returns:
+ *
+ */
+const char *type_get_id(TypeInstance *obj);
+
+/**
+ * @type_get_type:
+ *
+ * @obj:
+ *
+ * Returns:
+ */
+const char *type_get_type(TypeInstance *obj);
+
+/**
+ * @type_get_super:
+ *
+ * @obj:
+ *
+ * Returns:
+ */
+TypeClass *type_get_super(TypeInstance *obj);
+
+/**/
+
+/**
+ * @type_register_static:
+ *
+ * @info:
+ *
+ * Returns:
+ */
+Type type_register_static(const TypeInfo *info);
+
+/**
+ * @type_find_by_id:
+ *
+ * @id:
+ *
+ * Returns:
+ *
+ */
+TypeInstance *type_find_by_id(const char *id);
+
+/**
+ * @type_check_class:
+ *
+ * @obj:
+ *
+ * @typename:
+ *
+ * Returns:
+ */
+TypeClass *type_check_class(TypeClass *obj, const char *typename);
+
+/**
+ * @type_get_by_name:
+ *
+ * @name:
+ *
+ * Returns:
+ */
+Type type_get_by_name(const char *name);
+
+/**
+ * @type_get_name:
+ *
+ * @type:
+ *
+ * Returns:
+ */
+const char *type_get_name(Type type);
+
+/**
+ * @type_foreach:
+ *
+ * @enumfn:
+ *
+ * @opaque:
+ *
+ */
+void type_foreach(void (*enumfn)(TypeInstance *obj, void *opaque),
+                  void *opaque);
+
+#endif
diff --git a/qom/Makefile b/qom/Makefile
new file mode 100644
index 0000000..838054f
--- /dev/null
+++ b/qom/Makefile
@@ -0,0 +1 @@
+qom-obj-$(CONFIG_QOM) += type.o
diff --git a/qom/Qconfig b/qom/Qconfig
new file mode 100644
index 0000000..16660b4
--- /dev/null
+++ b/qom/Qconfig
@@ -0,0 +1,6 @@
+config QOM
+       bool "QEMU Object Model Support"
+       default y
+       depends on QAPI
+       help
+         This is the core object model used by QEMU.  If in doubt, say y here.
diff --git a/qom/type.c b/qom/type.c
new file mode 100644
index 0000000..28e8114
--- /dev/null
+++ b/qom/type.c
@@ -0,0 +1,523 @@
+/*
+ * QEMU Object Model
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/type.h"
+#include <glib.h>
+
+#define MAX_INTERFACES 32
+
+typedef struct InterfaceImpl
+{
+    const char *parent;
+    void (*interface_initfn)(TypeClass *class);
+    Type type;
+} InterfaceImpl;
+
+typedef struct TypeImpl
+{
+    const char *name;
+    Type type;
+
+    size_t class_size;
+
+    size_t instance_size;
+
+    void (*base_init)(TypeClass *klass);
+    void (*base_finalize)(TypeClass *klass);
+
+    void (*class_init)(TypeClass *klass);
+    void (*class_finalize)(TypeClass *klass);
+
+    void (*instance_init)(TypeInstance *obj);
+    void (*instance_finalize)(TypeInstance *obj);
+
+    bool abstract;
+
+    const char *parent;
+
+    TypeClass *class;
+
+    int num_interfaces;
+    InterfaceImpl interfaces[MAX_INTERFACES];
+} TypeImpl;
+
+static int num_types = 1;
+static TypeImpl type_table[128];
+
+Type type_register_static(const TypeInfo *info)
+{
+    Type type = num_types++;
+    TypeImpl *ti;
+
+    ti = &type_table[type];
+
+    assert(info->name != NULL);
+
+    ti->name = info->name;
+    ti->parent = info->parent;
+    ti->type = type;
+
+    ti->class_size = info->class_size;
+    ti->instance_size = info->instance_size;
+
+    ti->base_init = info->base_init;
+    ti->base_finalize = info->base_finalize;
+
+    ti->class_init = info->class_init;
+    ti->class_finalize = info->class_finalize;
+
+    ti->instance_init = info->instance_init;
+    ti->instance_finalize = info->instance_finalize;
+
+    ti->abstract = info->abstract;
+
+    if (info->interfaces) {
+        int i;
+
+        for (i = 0; info->interfaces[i].type; i++) {
+            ti->interfaces[i].parent = info->interfaces[i].type;
+            ti->interfaces[i].interface_initfn = info->interfaces[i].interface_initfn;
+            ti->num_interfaces++;
+        }
+    }
+
+    return type;
+}
+
+static Type type_register_anonymous(const TypeInfo *info)
+{
+    Type type = num_types++;
+    TypeImpl *ti;
+    char buffer[32];
+    static int count;
+
+    ti = &type_table[type];
+
+    snprintf(buffer, sizeof(buffer), "<anonymous-%d>", count++);
+    ti->name = qemu_strdup(buffer);
+    ti->parent = qemu_strdup(info->parent);
+    ti->type = type;
+
+    ti->class_size = info->class_size;
+    ti->instance_size = info->instance_size;
+
+    ti->base_init = info->base_init;
+    ti->base_finalize = info->base_finalize;
+
+    ti->class_init = info->class_init;
+    ti->class_finalize = info->class_finalize;
+
+    ti->instance_init = info->instance_init;
+    ti->instance_finalize = info->instance_finalize;
+
+    if (info->interfaces) {
+        int i;
+
+        for (i = 0; info->interfaces[i].type; i++) {
+            ti->interfaces[i].parent = info->interfaces[i].type;
+            ti->interfaces[i].interface_initfn = info->interfaces[i].interface_initfn;
+            ti->num_interfaces++;
+        }
+    }
+
+    return type;
+}
+
+static TypeImpl *type_get_instance(Type type)
+{
+    assert(type != 0);
+    assert(type < num_types);
+
+    return &type_table[type];
+}
+
+Type type_get_by_name(const char *name)
+{
+    int i;
+
+    if (name == NULL) {
+        return 0;
+    }
+
+    for (i = 1; i < num_types; i++) {
+        if (strcmp(name, type_table[i].name) == 0) {
+            return i;
+        }
+    }
+
+    return 0;
+}
+
+static void type_class_base_init(TypeImpl *base_ti, const char *typename)
+{
+    TypeImpl *ti;
+
+    if (!typename) {
+        return;
+    }
+
+    ti = type_get_instance(type_get_by_name(typename));
+
+    type_class_base_init(base_ti, ti->parent);
+
+    if (ti->base_init) {
+        ti->base_init(base_ti->class);
+    }
+}
+
+static size_t type_class_get_size(TypeImpl *ti)
+{
+    if (ti->class_size) {
+        return ti->class_size;
+    }
+
+    if (ti->parent) {
+        return type_class_get_size(type_get_instance(type_get_by_name(ti->parent)));
+    }
+
+    return sizeof(TypeClass);
+}
+
+static void type_class_interface_init(TypeImpl *ti, InterfaceImpl *iface)
+{
+    TypeInfo info = {
+        .instance_size = sizeof(Interface),
+        .parent = iface->parent,
+        .class_size = sizeof(InterfaceClass),
+        .class_init = iface->interface_initfn,
+        .abstract = true,
+    };
+
+    iface->type = type_register_anonymous(&info);
+}
+
+static void type_class_init(TypeImpl *ti)
+{
+    size_t class_size = sizeof(TypeClass);
+    int i;
+
+    if (ti->class) {
+        return;
+    }
+
+    ti->class_size = type_class_get_size(ti);
+
+    ti->class = qemu_malloc(ti->class_size);
+    ti->class->type = ti->type;
+
+    if (ti->parent) {
+        TypeImpl *ti_parent;
+
+        ti_parent = type_get_instance(type_get_by_name(ti->parent));
+
+        type_class_init(ti_parent);
+
+        class_size = ti_parent->class_size;
+        assert(ti_parent->class_size <= ti->class_size);
+
+        memcpy((void *)ti->class + sizeof(TypeClass),
+               (void *)ti_parent->class + sizeof(TypeClass),
+               ti_parent->class_size - sizeof(TypeClass));
+    }
+
+    memset((void *)ti->class + class_size, 0, ti->class_size - class_size);
+
+    type_class_base_init(ti, ti->parent);
+
+    for (i = 0; i < ti->num_interfaces; i++) {
+        type_class_interface_init(ti, &ti->interfaces[i]);
+    }
+
+    if (ti->class_init) {
+        ti->class_init(ti->class);
+    }
+}
+
+static void type_instance_interface_init(TypeInstance *obj, InterfaceImpl *iface)
+{
+    TypeImpl *ti = type_get_instance(iface->type);
+    Interface *iface_obj;
+    static int count;
+    char buffer[32];
+
+    snprintf(buffer, sizeof(buffer), "__anonymous_%d", count++);
+    iface_obj = INTERFACE(type_new(ti->name, buffer));
+    iface_obj->obj = obj;
+
+    obj->interfaces = g_slist_prepend(obj->interfaces, iface_obj);
+}
+
+static void type_instance_init(TypeInstance *obj, const char *typename)
+{
+    TypeImpl *ti = type_get_instance(type_get_by_name(typename));
+    int i;
+
+    if (ti->parent) {
+        type_instance_init(obj, ti->parent);
+    }
+
+    for (i = 0; i < ti->num_interfaces; i++) {
+        type_instance_interface_init(obj, &ti->interfaces[i]);
+    }
+
+    if (ti->instance_init) {
+        ti->instance_init(obj);
+    }
+}
+
+static GHashTable *global_object_table;
+
+void type_initialize(void *data, const char *typename, const char *id)
+{
+    TypeImpl *ti = type_get_instance(type_get_by_name(typename));
+    TypeInstance *obj = data;
+
+    g_assert(ti->instance_size >= sizeof(TypeClass));
+
+    type_class_init(ti);
+
+    g_assert(ti->abstract == false);
+
+    memset(obj, 0, ti->instance_size);
+
+    obj->class = ti->class;
+    snprintf(obj->id, sizeof(obj->id), "%s", id);
+
+    if (global_object_table == NULL) {
+        global_object_table = g_hash_table_new(g_str_hash, g_str_equal);
+    }
+
+    g_hash_table_insert(global_object_table, obj->id, obj);
+
+    type_instance_init(obj, typename);
+}
+
+static void type_instance_finalize(TypeInstance *obj, const char *typename)
+{
+    TypeImpl *ti = type_get_instance(type_get_by_name(typename));
+
+    if (ti->instance_finalize) {
+        ti->instance_finalize(obj);
+    }
+
+    while (obj->interfaces) {
+        Interface *iface_obj = obj->interfaces->data;
+        obj->interfaces = g_slist_delete_link(obj->interfaces, obj->interfaces);
+        type_delete(TYPE_INSTANCE(iface_obj));
+    }
+
+    if (ti->parent) {
+        type_instance_init(obj, ti->parent);
+    }
+}
+
+void type_finalize(void *data)
+{
+    TypeInstance *obj = data;
+    TypeImpl *ti = type_get_instance(obj->class->type);
+
+    g_hash_table_remove(global_object_table, obj->id);
+
+    type_instance_finalize(obj, ti->name);
+}
+
+const char *type_get_name(Type type)
+{
+    TypeImpl *ti = type_get_instance(type);
+    return ti->name;
+}
+
+TypeInstance *type_new(const char *typename, const char *id)
+{
+    TypeImpl *ti = type_get_instance(type_get_by_name(typename));
+    TypeInstance *obj;
+
+    obj = qemu_malloc(ti->instance_size);
+    type_initialize(obj, typename, id);
+
+    return obj;
+}
+
+void type_delete(TypeInstance *obj)
+{
+    type_finalize(obj);
+    qemu_free(obj);
+}
+
+TypeInstance *type_find_by_id(const char *id)
+{
+    gpointer data;
+
+    if (global_object_table == NULL) {
+        return NULL;
+    }
+
+    data = g_hash_table_lookup(global_object_table, id);
+
+    if (!data) {
+        return NULL;
+    }
+
+    return TYPE_INSTANCE(data);
+}
+
+bool type_is_type(TypeInstance *obj, const char *typename)
+{
+    Type target_type = type_get_by_name(typename);
+    Type type = obj->class->type;
+    GSList *i;
+
+    /* Check if typename is a direct ancestor of type */
+    while (type) {
+        TypeImpl *ti = type_get_instance(type);
+
+        if (ti->type == target_type) {
+            return true;
+        }
+
+        type = type_get_by_name(ti->parent);
+    }
+
+    /* Check if obj has an interface of typename */
+    for (i = obj->interfaces; i; i = i->next) {
+        Interface *iface = i->data;
+
+        if (type_is_type(TYPE_INSTANCE(iface), typename)) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+TypeInstance *type_dynamic_cast(TypeInstance *obj, const char *typename)
+{
+    GSList *i;
+
+    /* Check if typename is a direct ancestor */
+    if (type_is_type(obj, typename)) {
+        return obj;
+    }
+
+    /* Check if obj has an interface of typename */
+    for (i = obj->interfaces; i; i = i->next) {
+        Interface *iface = i->data;
+
+        if (type_is_type(TYPE_INSTANCE(iface), typename)) {
+            return TYPE_INSTANCE(iface);
+        }
+    }
+
+    /* Check if obj is an interface and it's containing object is a direct ancestor of typename */
+    if (type_is_type(obj, TYPE_INTERFACE)) {
+        Interface *iface = INTERFACE(obj);
+
+        if (type_is_type(iface->obj, typename)) {
+            return iface->obj;
+        }
+    }
+
+    return NULL;
+}
+
+
+static void register_interface(void)
+{
+    static TypeInfo interface_info = {
+        .name = TYPE_INTERFACE,
+        .instance_size = sizeof(Interface),
+        .abstract = true,
+    };
+
+    type_register_static(&interface_info);
+}
+
+device_init(register_interface);
+
+TypeInstance *type_dynamic_cast_assert(TypeInstance *obj, const char *typename)
+{
+    TypeInstance *inst;
+
+    inst = type_dynamic_cast(obj, typename);
+
+    if (!inst) {
+        fprintf(stderr, "Object %p is not an instance of type %s\n", obj, typename);
+        abort();
+    }
+
+    return inst;
+}
+
+TypeClass *type_check_class(TypeClass *class, const char *typename)
+{
+    Type target_type = type_get_by_name(typename);
+    Type type = class->type;
+
+    while (type) {
+        TypeImpl *ti = type_get_instance(type);
+
+        if (ti->type == target_type) {
+            return class;
+        }
+
+        type = type_get_by_name(ti->parent);
+    }
+
+    fprintf(stderr, "Object %p is not an instance of type %d\n", class, (int)type);
+    abort();
+
+    return NULL;
+}
+
+const char *type_get_id(TypeInstance *obj)
+{
+    return obj->id;
+}
+
+const char *type_get_type(TypeInstance *obj)
+{
+    return type_get_name(obj->class->type);
+}
+
+TypeClass *type_get_class(TypeInstance *obj)
+{
+    return obj->class;
+}
+
+typedef struct TypeForeachData
+{
+    void (*enumfn)(TypeInstance *obj, void *opaque);
+    void *opaque;
+} TypeForeachData;
+
+static void type_foreach_tramp(gpointer key, gpointer value, gpointer opaque)
+{
+    TypeForeachData *data = opaque;
+    data->enumfn(TYPE_INSTANCE(value), data->opaque);
+}
+
+void type_foreach(void (*enumfn)(TypeInstance *obj, void *opaque), void *opaque)
+{
+    TypeForeachData data = {
+        .enumfn = enumfn,
+        .opaque = opaque,
+    };
+
+    g_hash_table_foreach(global_object_table, type_foreach_tramp, &data);
+}
+
+TypeClass *type_get_super(TypeInstance *obj)
+{
+    return type_get_instance(type_get_by_name(type_get_instance(obj->class->type)->parent))->class;
+}
+
-- 
1.7.4.1

[Qemu-devel] [PATCH 04/21] qom: add Plug class

[Anthony Liguori]

The Plug class is the base class for all objects.  Plugs implement properties
using the existing Visitor framework.  This allows for very complex properties
to be implemented.

Plugs also implement life cycle management by containing a realized state.  An
object isn't fully constructed until its realized.  By setting realized to
false, an object can also be reset.

Plug properties can be set to be read-only or read-write.  A read-write property
can be locked.  While a read-write property is locked, it cannot be written to.

The expectation is that some properties will be locked at realize time.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 include/qemu/plug-proptypes.h |   14 +++
 include/qemu/plug.h           |   73 +++++++++++++
 qerror.c                      |    4 +
 qerror.h                      |    3 +
 qom/Makefile                  |    2 +-
 qom/plug-proptypes.c          |   52 ++++++++++
 qom/plug.c                    |  225 +++++++++++++++++++++++++++++++++++++++++
 qom/string-visitor.c          |   41 ++++++++
 qom/string-visitor.h          |   21 ++++
 9 files changed, 434 insertions(+), 1 deletions(-)
 create mode 100644 include/qemu/plug-proptypes.h
 create mode 100644 include/qemu/plug.h
 create mode 100644 qom/plug-proptypes.c
 create mode 100644 qom/plug.c
 create mode 100644 qom/string-visitor.c
 create mode 100644 qom/string-visitor.h

diff --git a/include/qemu/plug-proptypes.h b/include/qemu/plug-proptypes.h
new file mode 100644
index 0000000..2c7be0e
--- /dev/null
+++ b/include/qemu/plug-proptypes.h
@@ -0,0 +1,14 @@
+#ifndef PLUG_PROPTYPES_H
+#define PLUG_PROPTYPES_H
+
+#include "plug.h"
+
+typedef bool (PlugPropertyGetterBool)(Plug *plug);
+typedef void (PlugPropertySetterBool)(Plug *plug, bool value);
+
+void plug_add_property_bool(Plug *plug, const char *name,
+                            PlugPropertyGetterBool *getter,
+                            PlugPropertySetterBool *setter,
+                            int flags);
+
+#endif
diff --git a/include/qemu/plug.h b/include/qemu/plug.h
new file mode 100644
index 0000000..f6948a0
--- /dev/null
+++ b/include/qemu/plug.h
@@ -0,0 +1,73 @@
+#ifndef PLUG_H
+#define PLUG_H
+
+#include "qemu/type.h"
+#include "error.h"
+#include "qapi/qapi-visit-core.h"
+
+typedef struct Plug Plug;
+typedef struct PlugProperty PlugProperty;
+
+typedef enum PlugPropertyFlags
+{
+    PROP_F_NONE = 0,
+    PROP_F_READ = 1,
+    PROP_F_WRITE = 2,
+    PROP_F_READWRITE = (PROP_F_READ | PROP_F_WRITE),
+    PROP_F_LOCKED = 4,
+} PlugPropertyFlags;
+
+struct Plug
+{
+    TypeInstance parent;
+
+    /* private */
+    bool realized;
+    PlugProperty *first_prop;
+};
+
+typedef struct PlugClass {
+    TypeClass parent_class;
+
+    /* protected */
+    void (*realize)(Plug *plug);
+    void (*unrealize)(Plug *plug);
+} PlugClass;
+
+#define TYPE_PLUG "plug"
+#define PLUG(obj) TYPE_CHECK(Plug, obj, TYPE_PLUG)
+#define PLUG_GET_CLASS(obj) TYPE_GET_CLASS(PlugClass, obj, TYPE_PLUG)
+#define PLUG_CLASS(obj) TYPE_CLASS_CHECK(PlugClass, obj, TYPE_PLUG)
+
+typedef void (PlugPropertyAccessor)(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp);
+typedef void (PlugPropertyFinalize)(Plug *plug, const char *name, void *opaque);
+typedef void (PropertyEnumerator)(Plug *plug, const char *name, const char *typename, int flags, void *opaque);
+
+void plug_set_property(Plug *plug, const char *name, Visitor *v, Error **errp);
+
+void plug_get_property(Plug *plug, const char *name, Visitor *v, Error **errp);
+
+void plug_add_property_full(Plug *plug, const char *name,
+                            PlugPropertyAccessor *getter,
+                            PlugPropertyAccessor *setter,
+                            PlugPropertyFinalize *fini,
+                            void *opaque,
+                            const char *typename, int flags);
+
+void plug_foreach_property(Plug *plug, PropertyEnumerator *enumfn, void *opaque);
+
+void plug_lock_property(Plug *plug, const char *name);
+void plug_unlock_property(Plug *plug, const char *name);
+
+void plug_lock_all_properties(Plug *plug);
+void plug_unlock_all_properties(Plug *plug);
+
+void plug_set_realized(Plug *plug, bool realized);
+bool plug_get_realized(Plug *plug);
+
+void plug_realize_all(Plug *plug);
+void plug_unrealize_all(Plug *plug);
+
+#include "qemu/plug-proptypes.h"
+
+#endif
diff --git a/qerror.c b/qerror.c
index 69c1bc9..dbe119c 100644
--- a/qerror.c
+++ b/qerror.c
@@ -170,6 +170,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Property '%(device).%(property)' not found",
     },
     {
+        .error_fmt = QERR_PROPERTY_READ_ONLY,
+        .desc      = "Property '%(device).%(property)' is read only",
+    },
+    {
         .error_fmt = QERR_PROPERTY_VALUE_BAD,
         .desc      = "Property '%(device).%(property)' doesn't take value '%(value)'",
     },
diff --git a/qerror.h b/qerror.h
index 8058456..eaabe6c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -145,6 +145,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_PROPERTY_NOT_FOUND \
     "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
 
+#define QERR_PROPERTY_READ_ONLY \
+    "{ 'class': 'PropertyReadOnly', 'data': { 'device': %s, 'property': %s } }"
+
 #define QERR_PROPERTY_VALUE_BAD \
     "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
 
diff --git a/qom/Makefile b/qom/Makefile
index 838054f..8a438d5 100644
--- a/qom/Makefile
+++ b/qom/Makefile
@@ -1 +1 @@
-qom-obj-$(CONFIG_QOM) += type.o
+qom-obj-$(CONFIG_QOM) += type.o string-visitor.o plug.o plug-proptypes.o
diff --git a/qom/plug-proptypes.c b/qom/plug-proptypes.c
new file mode 100644
index 0000000..e9152a7
--- /dev/null
+++ b/qom/plug-proptypes.c
@@ -0,0 +1,52 @@
+/** FIXME: move to generated code **/
+
+#include "qemu/plug-proptypes.h"
+
+typedef struct FunctionPointer
+{
+    void (*getter)(void);
+    void (*setter)(void);
+} FunctionPointer;
+
+static void plug_get_property__bool(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)
+{
+    FunctionPointer *fp = opaque;
+    PlugPropertyGetterBool *getter = (PlugPropertyGetterBool *)fp->getter;
+    bool value;
+
+    value = getter(plug);
+    visit_type_bool(v, &value, name, errp);
+}
+
+static void plug_set_property__bool(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)
+{
+    FunctionPointer *fp = opaque;
+    PlugPropertySetterBool *setter = (PlugPropertySetterBool *)fp->setter;
+    bool value = false;
+    Error *local_err = NULL;
+
+    visit_type_bool(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    setter(plug, value);
+}
+
+void plug_add_property_bool(Plug *plug, const char *name,
+                            PlugPropertyGetterBool *getter,
+                            PlugPropertySetterBool *setter,
+                            int flags)
+{
+    FunctionPointer *fp = qemu_mallocz(sizeof(*fp));
+
+    fp->getter = (void (*)(void))getter;
+    fp->setter = (void (*)(void))setter;
+
+    plug_add_property_full(plug, name,
+                           plug_get_property__bool,
+                           plug_set_property__bool,
+                           plug_del_property__fp,
+                           fp, "bool", flags);
+}
diff --git a/qom/plug.c b/qom/plug.c
new file mode 100644
index 0000000..4a5f2fb
--- /dev/null
+++ b/qom/plug.c
@@ -0,0 +1,225 @@
+#include "qemu/plug.h"
+#include "qerror.h"
+#include "qom/string-visitor.h"
+
+#define MAX_NAME (128 + 1)
+#define MAX_TYPENAME (32 + 1)
+
+struct PlugProperty
+{
+    char name[MAX_NAME];
+    char typename[MAX_TYPENAME];
+    
+    PlugPropertyAccessor *getter;
+    PlugPropertyAccessor *setter;
+    PlugPropertyFinalize *finalize;
+    void *opaque;
+
+    int flags;
+
+    PlugProperty *next;
+};
+
+void plug_add_property_full(Plug *plug, const char *name,
+                            PlugPropertyAccessor *getter,
+                            PlugPropertyAccessor *setter,
+                            PlugPropertyFinalize *fini,
+                            void *opaque, const char *typename, int flags)
+{
+    PlugProperty *prop = qemu_mallocz(sizeof(*prop));
+
+    snprintf(prop->name, sizeof(prop->name), "%s", name);
+    snprintf(prop->typename, sizeof(prop->typename), "%s", typename);
+
+    prop->getter = getter;
+    prop->setter = setter;
+    prop->finalize = fini;
+    prop->opaque = opaque;
+
+    prop->flags = flags;
+
+    prop->next = plug->first_prop;
+    plug->first_prop = prop;
+}
+
+static PlugProperty *plug_find_property(Plug *plug, const char *name)
+{
+    PlugProperty *prop;
+
+    for (prop = plug->first_prop; prop; prop = prop->next) {
+        if (strcmp(prop->name, name) == 0) {
+            return prop;
+        }
+    }
+
+    return NULL;
+}
+
+void plug_set_property(Plug *plug, const char *name, Visitor *v, Error **errp)
+{
+    PlugProperty *prop = plug_find_property(plug, name);
+
+    if (!prop) {
+        error_set(errp, QERR_PROPERTY_NOT_FOUND, type_get_id(TYPE_INSTANCE(plug)), name);
+        return;
+    }
+
+    if (!(prop->flags & PROP_F_WRITE) || (prop->flags & PROP_F_LOCKED)) {
+        error_set(errp, QERR_PROPERTY_READ_ONLY, type_get_id(TYPE_INSTANCE(plug)), name);
+        return;
+    }
+
+    prop->setter(plug, name, v, prop->opaque, errp);
+}
+
+void plug_get_property(Plug *plug, const char *name, Visitor *v, Error **errp)
+{
+    PlugProperty *prop = plug_find_property(plug, name);
+
+    if (!prop) {
+        error_set(errp, QERR_PROPERTY_NOT_FOUND, type_get_id(TYPE_INSTANCE(plug)), name);
+        printf("property not found\n");
+        return;
+    }
+
+    if (!(prop->flags & PROP_F_READ)) {
+        error_set(errp, QERR_PROPERTY_READ_ONLY, type_get_id(TYPE_INSTANCE(plug)), name);
+        printf("property read only\n");
+        return;
+    }
+
+    prop->getter(plug, name, v, prop->opaque, errp);
+}
+
+void plug_lock_property(Plug *plug, const char *name)
+{
+    PlugProperty *prop = plug_find_property(plug, name);
+
+    assert(prop != NULL);
+
+    prop->flags |= PROP_F_LOCKED;
+}
+
+void plug_unlock_property(Plug *plug, const char *name)
+{
+    PlugProperty *prop = plug_find_property(plug, name);
+
+    assert(prop != NULL);
+
+    prop->flags &= ~PROP_F_LOCKED;
+}
+
+void plug_lock_all_properties(Plug *plug)
+{
+    PlugProperty *prop;
+
+    for (prop = plug->first_prop; prop; prop = prop->next) {
+        prop->flags |= PROP_F_LOCKED;
+    }
+}
+
+void plug_unlock_all_properties(Plug *plug)
+{
+    PlugProperty *prop;
+
+    for (prop = plug->first_prop; prop; prop = prop->next) {
+        prop->flags &= ~PROP_F_LOCKED;
+    }
+}
+
+void plug_foreach_property(Plug *plug, PropertyEnumerator *enumfn, void *opaque)
+{
+    PlugProperty *prop;
+
+    for (prop = plug->first_prop; prop; prop = prop->next) {
+        enumfn(plug, prop->name, prop->typename, prop->flags, opaque);
+    }
+}
+
+void plug_set_realized(Plug *plug, bool realized)
+{
+    PlugClass *class = PLUG_GET_CLASS(plug);
+    bool old_value = plug->realized;
+
+    plug->realized = realized;
+
+    if (plug->realized && !old_value) {
+        if (class->realize) {
+            class->realize(plug);
+        }
+    } else if (!plug->realized && old_value) {
+        if (class->unrealize) {
+            class->unrealize(plug);
+        }
+    }
+}
+
+bool plug_get_realized(Plug *plug)
+{
+    return plug->realized;
+}
+
+void plug_realize_all(Plug *plug)
+{
+    /* This doesn't loop infinitely because the callbacks are only called when
+     * the state changes. */
+    plug_set_realized(plug, true);
+    plug_lock_all_properties(plug);
+}
+
+void plug_unrealize_all(Plug *plug)
+{
+    /* This doesn't loop infinitely because the callbacks are only called when
+     * the state changes. */
+    plug_set_realized(plug, false);
+    plug_unlock_all_properties(plug);
+}
+
+static void plug_class_initfn(TypeClass *base_class)
+{
+    PlugClass *class = PLUG_CLASS(base_class);
+
+    class->realize = plug_realize_all;
+    class->unrealize = plug_unrealize_all;
+}
+
+static void plug_initfn(TypeInstance *inst)
+{
+    Plug *obj = PLUG(inst);
+
+    plug_add_property_bool(obj, "realized",
+                           plug_get_realized,
+                           plug_set_realized,
+                           PROP_F_READWRITE);
+}
+
+static void plug_finifn(TypeInstance *inst)
+{
+    Plug *plug = PLUG(inst);
+
+    while (plug->first_prop) {
+        PlugProperty *p = plug->first_prop;
+
+        plug->first_prop = plug->first_prop->next;
+        if (p->finalize) {
+            p->finalize(plug, p->name, p->opaque);
+        }
+        qemu_free(p);
+    }
+}
+
+static const TypeInfo plug_type_info = {
+    .name = TYPE_PLUG,
+    .instance_size = sizeof(Plug),
+    .class_size = sizeof(PlugClass),
+    .instance_init = plug_initfn,
+    .instance_finalize = plug_finifn,
+    .class_init = plug_class_initfn,
+};
+
+static void register_devices(void)
+{
+    type_register_static(&plug_type_info);
+}
+
+device_init(register_devices);
diff --git a/qom/string-visitor.c b/qom/string-visitor.c
new file mode 100644
index 0000000..55297e2
--- /dev/null
+++ b/qom/string-visitor.c
@@ -0,0 +1,41 @@
+#include "string-visitor.h"
+
+static void string_input_visitor_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
+{
+    StringInputVisitor *sv = container_of(v, StringInputVisitor, parent);
+    *obj = atoi(sv->value);
+}
+
+static void string_input_visitor_str(Visitor *v, char **obj, const char *name, Error **errp)
+{
+    StringInputVisitor *sv = container_of(v, StringInputVisitor, parent);
+    *obj = qemu_strdup(sv->value);
+}
+
+void string_input_visitor_init(StringInputVisitor *sv, const char *value)
+{
+    memset(sv, 0, sizeof(*sv));
+    sv->parent.type_int = string_input_visitor_int;
+    sv->parent.type_str = string_input_visitor_str;
+    sv->value = value;
+}
+
+static void string_output_visitor_int(Visitor *v, int64_t *obj, const char *name, Error **errp)
+{
+    StringOutputVisitor *sv = container_of(v, StringOutputVisitor, parent);
+    snprintf(sv->value, sizeof(sv->value), "%" PRId64, *obj);
+}
+
+static void string_output_visitor_str(Visitor *v, char **obj, const char *name, Error **errp)
+{
+    StringOutputVisitor *sv = container_of(v, StringOutputVisitor, parent);
+
+    snprintf(sv->value, sizeof(sv->value), "%s", *obj);
+}
+
+void string_output_visitor_init(StringOutputVisitor *sv)
+{
+    memset(sv, 0, sizeof(*sv));
+    sv->parent.type_int = string_output_visitor_int;
+    sv->parent.type_str = string_output_visitor_str;
+}
diff --git a/qom/string-visitor.h b/qom/string-visitor.h
new file mode 100644
index 0000000..ddee3e5
--- /dev/null
+++ b/qom/string-visitor.h
@@ -0,0 +1,21 @@
+#ifndef STRING_VISITOR_H
+#define STRING_VISITOR_H
+
+#include "qapi/qapi-visit-core.h"
+
+typedef struct StringInputVisitor
+{
+    Visitor parent;
+    const char *value;
+} StringInputVisitor;
+
+typedef struct StringOutputVisitor
+{
+    Visitor parent;
+    char value[256];
+} StringOutputVisitor;
+
+void string_input_visitor_init(StringInputVisitor *sv, const char *value);
+void string_output_visitor_init(StringOutputVisitor *sv);
+
+#endif
-- 
1.7.4.1

[Qemu-devel] [PATCH 05/21] plug: add Plug property type

[Anthony Liguori]

Plug properties types allow composition within the object model.  A plug is an
object that is directly created from the current object, usually using the same
memory as the current object.

Realize state is propagated to any plugs assocatied with the object.  Lifecycle
is also propagated such that when a plug is added, its automatically initialized
and the plug is finalized when the parent object is finalized.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 include/qemu/plug.h |    4 ++
 qom/plug.c          |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 0 deletions(-)

diff --git a/include/qemu/plug.h b/include/qemu/plug.h
index f6948a0..fdefa04 100644
--- a/include/qemu/plug.h
+++ b/include/qemu/plug.h
@@ -68,6 +68,10 @@ bool plug_get_realized(Plug *plug);
 void plug_realize_all(Plug *plug);
 void plug_unrealize_all(Plug *plug);
 
+void plug_add_property_plug(Plug *plug, Plug *value, const char *typename, const char *name, ...);
+
+Plug *plug_get_property_plug(Plug *plug, Error **errp, const char *name, ...);
+
 #include "qemu/plug-proptypes.h"
 
 #endif
diff --git a/qom/plug.c b/qom/plug.c
index 4a5f2fb..725cac2 100644
--- a/qom/plug.c
+++ b/qom/plug.c
@@ -159,12 +159,115 @@ bool plug_get_realized(Plug *plug)
     return plug->realized;
 }
 
+static char *plug_get_property_str(Plug *plug, const char *name, Error **errp)
+{
+    StringOutputVisitor sov;
+
+    string_output_visitor_init(&sov);
+    plug_get_property(plug, name, &sov.parent, errp);
+
+    return qemu_strdup(sov.value);
+}
+
+static void plug_propagate_realized(Plug *plug, const char *name,
+                                    const char *typename, int flags,
+                                    void *opaque)
+{
+    if (strstart(typename, "plug<", NULL)) {
+        char *child_name;
+        Plug *child_plug;
+
+        child_name = plug_get_property_str(plug, name, NULL);
+        child_plug = PLUG(type_find_by_id(child_name));
+
+        plug_set_realized(child_plug, plug_get_realized(plug));
+
+        qemu_free(child_name);
+    }
+}
+
+typedef struct PlugData
+{
+    const char *typename;
+    Plug *value;
+} PlugData;
+
+static void plug_get_property__plug(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)
+{
+    PlugData *data = opaque;
+    char *value;
+
+    value = (char *)TYPE_INSTANCE(data->value)->id;
+    visit_type_str(v, &value, name, errp);
+}
+
+static void plug_del_property__plug(Plug *plug, const char *name, void *opaque)
+{
+    PlugData *data = opaque;
+
+    type_finalize(data->value);
+    qemu_free(data);
+}
+
+void plug_add_property_plug(Plug *plug, Plug *value, const char *typename,
+                            const char *name, ...)
+{
+    PlugData *data = qemu_mallocz(sizeof(*data));
+    char fullid[MAX_NAME];
+    char fulltype[MAX_TYPENAME];
+    size_t off;
+    va_list ap;
+
+    data->typename = typename;
+    data->value = value;
+
+    snprintf(fulltype, sizeof(fulltype), "plug<%s>", typename);
+
+    va_start(ap, name);
+    off = snprintf(fullid, sizeof(fullid), "%s::",
+                   type_get_id(TYPE_INSTANCE(plug)));
+    vsnprintf(&fullid[off], sizeof(fullid) - off, name, ap);
+    va_end(ap);
+
+    type_initialize(plug, typename, fullid);
+
+    plug_add_property_full(plug, name,
+                           plug_get_property__plug,
+                           NULL,
+                           plug_del_property__plug,
+                           data, fulltype, PROP_F_READ);
+}
+
+Plug *plug_get_property_plug(Plug *plug, Error **errp, const char *name, ...)
+{
+    char fullname[MAX_NAME];
+    char *plugname;
+    Plug *value;
+    va_list ap;
+
+    va_start(ap, name);
+    vsnprintf(fullname, sizeof(fullname), name, ap);
+    va_end(ap);
+
+    plugname = plug_get_property_str(plug, fullname, errp);
+    if (error_is_set(errp)) {
+        return NULL;
+    }
+
+    value = PLUG(type_find_by_id(plugname));
+
+    qemu_free(plugname);
+
+    return value;
+}
+
 void plug_realize_all(Plug *plug)
 {
     /* This doesn't loop infinitely because the callbacks are only called when
      * the state changes. */
     plug_set_realized(plug, true);
     plug_lock_all_properties(plug);
+    plug_foreach_property(plug, plug_propagate_realized, NULL);
 }
 
 void plug_unrealize_all(Plug *plug)
@@ -173,6 +276,7 @@ void plug_unrealize_all(Plug *plug)
      * the state changes. */
     plug_set_realized(plug, false);
     plug_unlock_all_properties(plug);
+    plug_foreach_property(plug, plug_propagate_realized, NULL);
 }
 
 static void plug_class_initfn(TypeClass *base_class)
-- 
1.7.4.1

[Qemu-devel] [PATCH 06/21] plug: add socket property type

[Anthony Liguori]

Sockets form back links in the object graph.  From the object's perspective,
a socket just looks like a pointer to an object.  The socket property type
provides a generic mechanism to set those pointers to other objects while
providing strong type checking.

It's also useful to lock sockets, particularly after realize.  This allows for
an object to enforce that a socket is programmed prior to realize and then not
modified afterwards.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 include/qemu/plug.h |    2 +
 qom/plug.c          |   64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/include/qemu/plug.h b/include/qemu/plug.h
index fdefa04..a7ca985 100644
--- a/include/qemu/plug.h
+++ b/include/qemu/plug.h
@@ -72,6 +72,8 @@ void plug_add_property_plug(Plug *plug, Plug *value, const char *typename, const
 
 Plug *plug_get_property_plug(Plug *plug, Error **errp, const char *name, ...);
 
+void plug_add_property_socket(Plug *plug, const char *name, Plug **value, const char *typename);
+
 #include "qemu/plug-proptypes.h"
 
 #endif
diff --git a/qom/plug.c b/qom/plug.c
index 725cac2..a9d8154 100644
--- a/qom/plug.c
+++ b/qom/plug.c
@@ -261,6 +261,70 @@ Plug *plug_get_property_plug(Plug *plug, Error **errp, const char *name, ...)
     return value;
 }
 
+typedef struct SocketData
+{
+    const char *typename;
+    Plug **value;
+} SocketData;
+
+static void plug_get_property__socket(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)
+{
+    SocketData *data = opaque;
+    const char *value = "";
+
+    if (*data->value) {
+        value = TYPE_INSTANCE(*data->value)->id;
+    }
+
+    visit_type_str(v, (char **)&value, name, errp);
+}
+
+static void plug_set_property__socket(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)
+{
+    char *value = NULL;
+    Error *local_err = NULL;
+    SocketData *data = opaque;
+    TypeInstance *obj;
+
+    visit_type_str(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    obj = type_find_by_id(value);
+    assert(obj != NULL);
+
+    *data->value = PLUG(type_dynamic_cast_assert(obj, data->typename));
+
+    qemu_free(value);
+}
+
+static void plug_del_property__socket(Plug *plug, const char *name, void *opaque)
+{
+    SocketData *data = opaque;
+
+    qemu_free(data);
+}
+
+void plug_add_property_socket(Plug *plug, const char *name, Plug **value, const char *typename)
+{
+    SocketData *data = qemu_mallocz(sizeof(*data));
+    char fulltype[33];
+
+    data->typename = typename;
+    data->value = value;
+
+    snprintf(fulltype, sizeof(fulltype), "socket<%s>", typename);
+
+    plug_add_property_full(plug, name,
+                           plug_get_property__socket,
+                           plug_set_property__socket,
+                           plug_del_property__socket,
+                           data,
+                           fulltype, PROP_F_READWRITE);
+}
+
 void plug_realize_all(Plug *plug)
 {
     /* This doesn't loop infinitely because the callbacks are only called when
-- 
1.7.4.1

[Qemu-devel] [PATCH 07/21] plug: add generated property types

[Anthony Liguori]

Right now this uses the preprocessor and is vomit inducing.  I think a python
generator is in the near future.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 include/qemu/plug-proptypes.h |   29 +++++++++-
 include/qemu/plug.h           |    4 +-
 qom/plug-proptypes.c          |  119 ++++++++++++++++++++++++++++++++++++++++-
 qom/plug.c                    |   10 ++--
 4 files changed, 151 insertions(+), 11 deletions(-)

diff --git a/include/qemu/plug-proptypes.h b/include/qemu/plug-proptypes.h
index 2c7be0e..cf3e06e 100644
--- a/include/qemu/plug-proptypes.h
+++ b/include/qemu/plug-proptypes.h
@@ -3,8 +3,33 @@
 
 #include "plug.h"
 
-typedef bool (PlugPropertyGetterBool)(Plug *plug);
-typedef void (PlugPropertySetterBool)(Plug *plug, bool value);
+#define CONCAT_I(a, b) a ## b
+#define CONCAT(a, b) CONCAT_I(a, b)
+
+#define GEN_PROP(ctype, typename, ctypename)                   \
+typedef ctype (CONCAT(PlugPropertyGetter, typename))(Plug *plug, Error **errp);  \
+typedef void (CONCAT(PlugPropertySetter, typename))(Plug *plug, ctype value, Error **errp); \
+ \
+void CONCAT(plug_add_property_, ctypename)(Plug *plug, const char *name, \
+                                           CONCAT(PlugPropertyGetter, typename) *getter, \
+                                           CONCAT(PlugPropertySetter, typename) *setter, \
+                                           int flags)
+
+GEN_PROP(int8_t, Int8, int8);
+GEN_PROP(int16_t, Int16, int16);
+GEN_PROP(int32_t, Int32, int32);
+GEN_PROP(int64_t, Int64, int64);
+GEN_PROP(uint8_t, UInt8, uint8);
+GEN_PROP(uint16_t, UInt16, uint16);
+GEN_PROP(uint32_t, UInt32, uint32);
+GEN_PROP(uint64_t, UInt64, uint64);
+GEN_PROP(int64_t, Int, int);
+GEN_PROP(const char *, Str, str);
+
+#undef GEN_PROP
+
+typedef bool (PlugPropertyGetterBool)(Plug *plug, Error **errp);
+typedef void (PlugPropertySetterBool)(Plug *plug, bool value, Error **errp);
 
 void plug_add_property_bool(Plug *plug, const char *name,
                             PlugPropertyGetterBool *getter,
diff --git a/include/qemu/plug.h b/include/qemu/plug.h
index a7ca985..03b63a2 100644
--- a/include/qemu/plug.h
+++ b/include/qemu/plug.h
@@ -62,8 +62,8 @@ void plug_unlock_property(Plug *plug, const char *name);
 void plug_lock_all_properties(Plug *plug);
 void plug_unlock_all_properties(Plug *plug);
 
-void plug_set_realized(Plug *plug, bool realized);
-bool plug_get_realized(Plug *plug);
+void plug_set_realized(Plug *plug, bool realized, Error **errp);
+bool plug_get_realized(Plug *plug, Error **errp);
 
 void plug_realize_all(Plug *plug);
 void plug_unrealize_all(Plug *plug);
diff --git a/qom/plug-proptypes.c b/qom/plug-proptypes.c
index e9152a7..b25093f 100644
--- a/qom/plug-proptypes.c
+++ b/qom/plug-proptypes.c
@@ -8,13 +8,78 @@ typedef struct FunctionPointer
     void (*setter)(void);
 } FunctionPointer;
 
+#define STRIFY_I(a) # a
+#define STRIFY(a) STRIFY_I(a)
+
+static void plug_del_property__fp(Plug *plug, const char *name, void *opaque)
+{
+    FunctionPointer *fp = opaque;
+    qemu_free(fp);
+}
+
+#define GEN_PROP(ctype, typename, ctypename) \
+static void CONCAT(plug_get_property__, ctypename)(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp) \
+{ \
+    FunctionPointer *fp = opaque; \
+    CONCAT(PlugPropertyGetter, typename) *getter = (CONCAT(PlugPropertyGetter, typename) *)fp->getter; \
+    int64_t value; \
+ \
+    value = getter(plug, errp);                 \
+    visit_type_int(v, &value, name, errp); \
+} \
+ \
+static void CONCAT(plug_set_property__, ctypename)(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)  \
+{ \
+    FunctionPointer *fp = opaque; \
+    CONCAT(PlugPropertySetter, typename) *setter = (CONCAT(PlugPropertySetter, typename) *)fp->setter; \
+    int64_t value = 0; \
+    Error *local_err = NULL; \
+ \
+    visit_type_int(v, &value, name, &local_err); \
+    if (local_err) { \
+        error_propagate(errp, local_err); \
+        return; \
+    } \
+ \
+    setter(plug, value, errp);                       \
+} \
+ \
+void CONCAT(plug_add_property_, ctypename)(Plug *plug, const char *name, \
+                                           CONCAT(PlugPropertyGetter, typename) *getter, \
+                                           CONCAT(PlugPropertySetter, typename) *setter, \
+                                           int flags) \
+{ \
+    FunctionPointer *fp = qemu_mallocz(sizeof(*fp)); \
+ \
+    fp->getter = (void (*)(void))getter; \
+    fp->setter = (void (*)(void))setter; \
+ \
+    plug_add_property_full(plug, name, \
+                           CONCAT(plug_get_property__, ctypename), \
+                           CONCAT(plug_set_property__, ctypename), \
+                           plug_del_property__fp, \
+                           fp, STRIFY(ctypename), flags); \
+}
+
+GEN_PROP(int8_t, Int8, int8);
+GEN_PROP(int16_t, Int16, int16);
+GEN_PROP(int32_t, Int32, int32);
+GEN_PROP(int64_t, Int64, int64);
+GEN_PROP(uint8_t, UInt8, uint8);
+GEN_PROP(uint16_t, UInt16, uint16);
+GEN_PROP(uint32_t, UInt32, uint32);
+GEN_PROP(uint64_t, UInt64, uint64);
+GEN_PROP(int64_t, Int, int);
+
+#undef GEN_PROP
+
 static void plug_get_property__bool(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)
 {
     FunctionPointer *fp = opaque;
     PlugPropertyGetterBool *getter = (PlugPropertyGetterBool *)fp->getter;
     bool value;
 
-    value = getter(plug);
+    value = getter(plug, errp);
     visit_type_bool(v, &value, name, errp);
 }
 
@@ -31,7 +96,7 @@ static void plug_set_property__bool(Plug *plug, const char *name, Visitor *v, vo
         return;
     }
 
-    setter(plug, value);
+    setter(plug, value, errp);
 }
 
 void plug_add_property_bool(Plug *plug, const char *name,
@@ -50,3 +115,53 @@ void plug_add_property_bool(Plug *plug, const char *name,
                            plug_del_property__fp,
                            fp, "bool", flags);
 }
+
+/** str **/
+
+static void plug_get_property__str(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)
+{
+    FunctionPointer *fp = opaque;
+    PlugPropertyGetterStr *getter = (PlugPropertyGetterStr *)fp->getter;
+    char *value;
+
+    value = (char *)getter(plug, errp);
+    if (value == NULL) {
+        value = (char *)"";
+    }
+    visit_type_str(v, &value, name, errp);
+}
+
+static void plug_set_property__str(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)
+{
+    FunctionPointer *fp = opaque;
+    PlugPropertySetterStr *setter = (PlugPropertySetterStr *)fp->setter;
+    char *value = false;
+    Error *local_err = NULL;
+
+    visit_type_str(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    setter(plug, value, errp);
+
+    qemu_free(value);
+}
+
+void plug_add_property_str(Plug *plug, const char *name,
+                           PlugPropertyGetterStr *getter,
+                           PlugPropertySetterStr *setter,
+                           int flags)
+{
+    FunctionPointer *fp = qemu_mallocz(sizeof(*fp));
+
+    fp->getter = (void (*)(void))getter;
+    fp->setter = (void (*)(void))setter;
+
+    plug_add_property_full(plug, name,
+                           plug_get_property__str,
+                           plug_set_property__str,
+                           plug_del_property__fp,
+                           fp, "str", flags);
+}
diff --git a/qom/plug.c b/qom/plug.c
index a9d8154..7f21c6b 100644
--- a/qom/plug.c
+++ b/qom/plug.c
@@ -136,7 +136,7 @@ void plug_foreach_property(Plug *plug, PropertyEnumerator *enumfn, void *opaque)
     }
 }
 
-void plug_set_realized(Plug *plug, bool realized)
+void plug_set_realized(Plug *plug, bool realized, Error **errp)
 {
     PlugClass *class = PLUG_GET_CLASS(plug);
     bool old_value = plug->realized;
@@ -154,7 +154,7 @@ void plug_set_realized(Plug *plug, bool realized)
     }
 }
 
-bool plug_get_realized(Plug *plug)
+bool plug_get_realized(Plug *plug, Error **errp)
 {
     return plug->realized;
 }
@@ -180,7 +180,7 @@ static void plug_propagate_realized(Plug *plug, const char *name,
         child_name = plug_get_property_str(plug, name, NULL);
         child_plug = PLUG(type_find_by_id(child_name));
 
-        plug_set_realized(child_plug, plug_get_realized(plug));
+        plug_set_realized(child_plug, plug_get_realized(plug, NULL), NULL);
 
         qemu_free(child_name);
     }
@@ -329,7 +329,7 @@ void plug_realize_all(Plug *plug)
 {
     /* This doesn't loop infinitely because the callbacks are only called when
      * the state changes. */
-    plug_set_realized(plug, true);
+    plug_set_realized(plug, true, NULL);
     plug_lock_all_properties(plug);
     plug_foreach_property(plug, plug_propagate_realized, NULL);
 }
@@ -338,7 +338,7 @@ void plug_unrealize_all(Plug *plug)
 {
     /* This doesn't loop infinitely because the callbacks are only called when
      * the state changes. */
-    plug_set_realized(plug, false);
+    plug_set_realized(plug, false, NULL);
     plug_unlock_all_properties(plug);
     plug_foreach_property(plug, plug_propagate_realized, NULL);
 }
-- 
1.7.4.1

[Qemu-devel] [PATCH 08/21] qom: add plug_create QMP command

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 monitor.c       |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx |   27 +++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 718935b..8406b5b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -62,6 +62,12 @@
 #endif
 #include "ui/qemu-spice.h"
 
+#include "qemu/plug.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
+
+#include <glib.h>
+
 //#define DEBUG
 //#define DEBUG_COMPLETION
 
@@ -1020,6 +1026,49 @@ static int do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
+static GSList *global_plug_list;
+
+static int do_plug_create(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    const char *type = qdict_get_str(qdict, "type");
+    QmpInputVisitor *qiv;
+    Visitor *v;
+    const QDictEntry *e;
+    TypeInstance *inst;
+    Plug *plug;
+
+    inst = type_new(type, id);
+    if (inst == NULL) {
+        return -1;
+    }
+
+    plug = PLUG(inst);
+
+    global_plug_list = g_slist_prepend(global_plug_list, plug);
+
+    qiv = qmp_input_visitor_new((QObject *)QOBJECT(qdict));
+    v = qmp_input_get_visitor(qiv);
+
+    for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
+        Error *local_err = NULL;
+
+        if (strcmp(e->key, "id") == 0 || strcmp(e->key, "type") == 0) {
+            continue;
+        }
+
+        plug_set_property(plug, e->key, v, &local_err);
+        if (local_err) {
+            error_free(local_err);
+            return -1;
+        }
+    }
+
+    qmp_input_visitor_cleanup(qiv);
+
+    return 0;
+}
+
 #ifdef CONFIG_VNC
 static int change_vnc_password(const char *password)
 {
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 54e313c..c86d396 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -85,6 +85,33 @@ Example:
 EQMP
 
     {
+        .name       = "plug_create",
+        .args_type  = "device:O",
+        .params     = "type=value,id=value[,prop=value][,...]",
+        .help       = "create a plug",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_plug_create,
+    },
+
+SQMP
+plug_create
+-----------
+
+Create a plug.
+
+Arguments:
+
+- type: typename of object (json-string)
+- id: name of object (json-string)
+- object properties
+
+Notes:
+
+ (1) id MUST not contain the sequence "::"
+
+EQMP
+
+    {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
-- 
1.7.4.1

[Qemu-devel] [PATCH 09/21] qom: add plug_list QMP command

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 monitor.c       |   25 +++++++++++++++++++++++++
 qmp-commands.hx |    9 +++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 8406b5b..f17eb3a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1069,6 +1069,31 @@ static int do_plug_create(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
+static int do_plug_list(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    QList *qlist = qlist_new();
+    const char *type = qdict_get_try_str(qdict, "type");
+    GSList *i;
+
+    for (i = global_plug_list; i; i = i->next) {
+        TypeInstance *obj = i->data;
+        QDict *item = qdict_new();
+
+        if (type && !type_is_type(obj, type)) {
+            continue;
+        }
+
+        qdict_put(item, "id", qstring_from_str(type_get_id(obj)));
+        qdict_put(item, "type", qstring_from_str(type_get_type(obj)));
+
+        qlist_append(qlist, item);
+    }
+
+    *ret_data = QOBJECT(qlist);
+
+    return 0;
+}
+
 #ifdef CONFIG_VNC
 static int change_vnc_password(const char *password)
 {
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c86d396..16f3f4d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -112,6 +112,15 @@ Notes:
 EQMP
 
     {
+        .name       = "plug_list",
+        .args_type  = "type:s?",
+        .params     = "",
+        .help       = "list plugs",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_plug_list,
+    },
+
+    {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
-- 
1.7.4.1

[Qemu-devel] [PATCH 10/21] qom: add plug_get QMP command

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 monitor.c       |   35 +++++++++++++++++++++++++++++++++++
 qmp-commands.hx |    9 +++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index f17eb3a..1937e9f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1094,6 +1094,41 @@ static int do_plug_list(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
+static int do_plug_get(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    const char *name = qdict_get_str(qdict, "name");
+    Error *local_err = NULL;
+    QmpOutputVisitor *qov;
+    TypeInstance *inst;
+    Plug *plug;
+    Visitor *v;
+
+    qov = qmp_output_visitor_new();
+    v = qmp_output_get_visitor(qov);
+
+    inst = type_find_by_id(id);
+    if (inst == NULL) {
+        return -1;
+    }
+
+    plug = PLUG(inst);
+
+    plug_get_property(plug, name, v, &local_err);
+    if (local_err) {
+        error_free(local_err);
+        qmp_output_visitor_cleanup(qov);
+
+        return -1;
+    }
+
+    *ret_data = qmp_output_get_qobject(qov);
+
+    qmp_output_visitor_cleanup(qov);
+
+    return 0;
+}
+
 #ifdef CONFIG_VNC
 static int change_vnc_password(const char *password)
 {
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 16f3f4d..822d422 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -121,6 +121,15 @@ EQMP
     },
 
     {
+        .name       = "plug_get",
+        .args_type  = "options:O",
+        .params     = "",
+        .help       = "get plug property",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_plug_get,
+    },
+
+    {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
-- 
1.7.4.1

[Qemu-devel] [PATCH 11/21] qom: add plug_set QMP command

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 monitor.c       |   33 +++++++++++++++++++++++++++++++++
 qmp-commands.hx |    9 +++++++++
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 1937e9f..47011dd 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1129,6 +1129,39 @@ static int do_plug_get(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
+static int do_plug_set(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    const char *name = qdict_get_str(qdict, "name");
+    QObject *value = qdict_get(qdict, "value");
+    Error *local_err = NULL;
+    QmpInputVisitor *qiv;
+    TypeInstance *inst;
+    Plug *plug;
+    Visitor *v;
+
+    qiv = qmp_input_visitor_new(value);
+    v = qmp_input_get_visitor(qiv);
+
+    inst = type_find_by_id(id);
+    if (inst == NULL) {
+        return -1;
+    }
+
+    plug = PLUG(inst);
+
+    plug_set_property(plug, name, v, &local_err);
+    if (local_err) {
+        error_free(local_err);
+        qmp_input_visitor_cleanup(qiv);
+        return -1;
+    }
+
+    qmp_input_visitor_cleanup(qiv);
+
+    return 0;
+}
+
 #ifdef CONFIG_VNC
 static int change_vnc_password(const char *password)
 {
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 822d422..1d7e87b 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -130,6 +130,15 @@ EQMP
     },
 
     {
+        .name       = "plug_set",
+        .args_type  = "options:O",
+        .params     = "",
+        .help       = "set plug property",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_plug_set,
+    },
+
+    {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
-- 
1.7.4.1

[Qemu-devel] [PATCH 12/21] qom: add plug_list_props QMP command

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 monitor.c       |   35 +++++++++++++++++++++++++++++++++++
 qmp-commands.hx |    9 +++++++++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 47011dd..e53808a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1162,6 +1162,41 @@ static int do_plug_set(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
+static void plug_list_props_tramp(Plug *plug, const char *name, const char *typename, int flags, void *opaque)
+{
+    QList *qlist = opaque;
+    QDict *item = qdict_new();
+
+    qdict_put(item, "name", qstring_from_str(name));
+    qdict_put(item, "type", qstring_from_str(typename));
+    qdict_put(item, "readable", qbool_from_int(!!(flags & PROP_F_READ)));
+    qdict_put(item, "writeable", qbool_from_int(!!(flags & PROP_F_WRITE)));
+    qdict_put(item, "locked", qbool_from_int(!!(flags & PROP_F_LOCKED)));
+
+    qlist_append(qlist, item);
+}
+
+static int do_plug_list_props(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    QList *qlist = qlist_new();
+    TypeInstance *inst;
+    Plug *plug;
+
+    inst = type_find_by_id(id);
+    if (inst == NULL) {
+        return -1;
+    }
+
+    plug = PLUG(inst);
+
+    plug_foreach_property(plug, plug_list_props_tramp, qlist);
+
+    *ret_data = QOBJECT(qlist);
+
+    return 0;
+}
+
 #ifdef CONFIG_VNC
 static int change_vnc_password(const char *password)
 {
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1d7e87b..6f1091b 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -139,6 +139,15 @@ EQMP
     },
 
     {
+        .name       = "plug_list_props",
+        .args_type  = "id:s",
+        .params     = "",
+        .help       = "list plug properties",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_plug_list_props,
+    },
+
+    {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
-- 
1.7.4.1

[Qemu-devel] [PATCH 13/21] qom: add plug_destroy command

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 monitor.c       |   20 ++++++++++++++++++++
 qmp-commands.hx |    9 +++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index e53808a..549251a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1069,6 +1069,26 @@ static int do_plug_create(Monitor *mon, const QDict *qdict, QObject **ret_data)
     return 0;
 }
 
+static int do_plug_destroy(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    TypeInstance *inst;
+    Plug *plug;
+
+    inst = type_find_by_id(id);
+    if (inst == NULL) {
+        return -1;
+    }
+
+    plug = PLUG(inst);
+
+    global_plug_list = g_slist_remove(global_plug_list, plug);
+
+    type_delete(inst);
+
+    return 0;
+}
+
 static int do_plug_list(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     QList *qlist = qlist_new();
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6f1091b..8203371 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -112,6 +112,15 @@ Notes:
 EQMP
 
     {
+        .name       = "plug_destroy",
+        .args_type  = "type:s",
+        .params     = "type=value",
+        .help       = "destroy a plug",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_plug_destroy,
+    },
+
+    {
         .name       = "plug_list",
         .args_type  = "type:s?",
         .params     = "",
-- 
1.7.4.1

[Qemu-devel] [PATCH 14/21] qom: add example qsh command

[Anthony Liguori]

This lets you explore the QEMU Object Model with a filesystem like interface.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 scripts/qsh |  178 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 178 insertions(+), 0 deletions(-)
 create mode 100755 scripts/qsh

diff --git a/scripts/qsh b/scripts/qsh
new file mode 100755
index 0000000..b4b5e94
--- /dev/null
+++ b/scripts/qsh
@@ -0,0 +1,178 @@
+#!/usr/bin/python
+
+import json, socket
+import sys
+
+class QmpException(Exception):
+    def __init__(self, cls, desc):
+        Exception.__init__(self, desc)
+
+class QmpClient(object):
+    def __init__(self, host, port):
+        s = socket.socket(socket.AF_INET)
+        s.connect((host, port))
+
+        self.s = s
+        self.buffer = ''
+        self.readline()
+
+    def readline(self):
+        while not self.buffer.find('\n') != -1:
+            self.buffer += self.s.recv(1024)
+
+        line, self.buffer = self.buffer.split('\n', 1)
+        return line
+
+    def _command(self, _command_name, **args):
+        req = { "execute": _command_name, "arguments": args }
+        self.s.sendall(json.dumps(req))
+        
+        rsp = json.loads(self.readline())
+        if rsp.has_key('error'):
+            raise QmpException(rsp['error']['class'], rsp['error']['desc'])
+        return rsp['return']
+
+    def qmp_capabilities(self):
+        return self._command('qmp_capabilities')
+
+    def plug_create(self, typename, name, **args):
+        self._command('plug_create', id=name, type=typename, **args)
+
+    def plug_list(self, typename=None):
+        if typename == None:
+            return self._command('plug_list')
+        else:
+            return self._command('plug_list', type=typename)
+
+    def plug_get(self, name, propname):
+        return self._command('plug_get', id=name, name=propname)
+
+    def plug_set(self, name, propname, value):
+        self._command('plug_set', id=name, name=propname, value=value)
+
+    def plug_list_props(self, name):
+        return self._command('plug_list_props', id=name)
+
+    def quit(self):
+        self._command('quit')
+
+from ConfigParser import SafeConfigParser
+import re
+
+class GitConfigParser(SafeConfigParser):
+    SECTCRE = re.compile(r'\[(?P<header>[^\"]+"[^"]*")\]')
+
+
+def resolve_property(qmp, value):
+    value = value[1:]
+
+    components = value.split('/')
+
+    name = components[0]
+    components = components[1:]
+
+    prop = components[-1]
+    for component in components[:-1]:
+        if component.startswith('@'):
+            component = component[1:]
+        name = qmp.plug_get(name, component)
+
+    if prop.startswith('@'):
+        prop = prop[1:]
+
+    return name, prop
+
+def parse_property(srv, value):
+    if value == 'True':
+        value = True
+    elif value == 'False':
+        value = False
+    elif value[0] in '0123456789-':
+        value = int(value)
+    elif value[0] == '/':
+        name, prop = resolve_property(srv, value)
+        value = qmp.plug_get(name, prop)
+    elif value[0] == '"':
+        value = value[1:-1]
+    return value
+
+qmp = QmpClient('localhost', 8080)
+qmp.qmp_capabilities()
+
+command = sys.argv[1]
+
+if command == 'import':    
+    cp = GitConfigParser()
+    cp.read([sys.argv[2]])
+
+    for section in cp.sections():
+        kind, name = section.split(' ', 1)
+        name = name[1:-1]
+        qmp.plug_create(kind, name)
+
+    for section in cp.sections():
+        kind, name = section.split(' ', 1)
+        name = name[1:-1]
+
+        for key, value in cp.items(section):
+            value = parse_property(qmp, value)
+            qmp.plug_set(name, key, value)
+elif command == 'create':
+    if len(sys.argv) != 4:
+        sys.exit(1)
+    qmp.plug_create(sys.argv[2], sys.argv[3])
+elif command == 'exit':
+    qmp.quit()
+elif command == 'set':
+    if len(sys.argv) != 4:
+        sys.exit(1)
+
+    name, prop = resolve_property(qmp, sys.argv[2])
+
+    value = parse_property(qmp, sys.argv[3])
+    qmp.plug_set(name, prop, value)
+elif command == 'get':
+    if len(sys.argv) != 3:
+        sys.exit(1)
+
+    name, prop = resolve_property(qmp, sys.argv[2])
+    print qmp.plug_get(name, prop)
+elif command == 'plug_list':
+    if len(sys.argv) != 3:
+        sys.exit(1)
+
+    items = qmp.plug_list(sys.argv[2])
+    for i in items:
+        print i
+elif command == 'ls':
+    if len(sys.argv) != 3:
+        sys.exit(1)
+
+    path = sys.argv[2]
+
+    while path.endswith('/'):
+        path = path[:-1]
+
+    if path == '':
+        path = '/'
+
+    if path == '/':
+        items = qmp.plug_list()
+        for item in items:
+            print '%s/' % item['id']
+    else:
+        if path[1:].find('/') == -1:
+            items = qmp.plug_list_props(path[1:])
+        else:
+            name, prop = resolve_property(qmp, path)
+            name = qmp.plug_get(name, prop)
+            items = qmp.plug_list_props(name)
+
+        for item in items:
+            if item['type'].startswith('plug<'):
+                print '%s/' % item['name']
+            elif item['type'].startswith('socket<'):
+                print '@%s/' % item['name']
+            else:
+                print '%s' % item['name']
+
-- 
1.7.4.1

[Qemu-devel] [PATCH 15/21] qom: add Device class

[Anthony Liguori]

Device is meant to replace DeviceState as the root class for the device model.
This is included here merely as a RFC.  Device adds a couple of useful features.

1) Default hard reset.  Device will literally call finalize on the object and
   then reinitialize it in place.  This means that most devices don't have to
   worry about implementing reset logic.

   Reset preserves the current state of properties which makes it equivalent to
   taking the properties of the current device and then initializing a new
   object using those properties.

2) Full object serialization as a property.  The 'state' property is special in
   that it will invoke a visit method that's intended to be overridden by a
   subclass.  The visit method will visit the full object.

   This exists to enable live migration.  Notice that there is no mention of
   compatibility, versioning, subsections, etc.  The idea behind supporting
   robust migration is that the device model is only responsible for generating
   the state of the current device model, the structure of the device model, and
   the current set of properties.

   The expectation is that another layer will perform transformations of the
   resulting tree to preserve migration compatibility.  Tree transformation is
   a powerful mechanism and even with a totally different device model, this
   should enable us to support migration compatibility even to the current
   VMState based migration code.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.qom          |    3 +
 Qconfig               |    1 +
 configure             |    2 +-
 devices/Makefile      |    1 +
 devices/Qconfig       |    6 ++
 devices/device.c      |  125 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/device.h |   28 +++++++++++
 7 files changed, 165 insertions(+), 1 deletions(-)
 create mode 100644 devices/Makefile
 create mode 100644 devices/Qconfig
 create mode 100644 devices/device.c
 create mode 100644 include/qemu/device.h

diff --git a/Makefile.qom b/Makefile.qom
index 34f1f91..c694cbb 100644
--- a/Makefile.qom
+++ b/Makefile.qom
@@ -13,3 +13,6 @@ common-obj-y += $(addprefix qapi/,$(qapi-obj-y))
 include $(SRC_PATH)/qom/Makefile
 common-obj-y += $(addprefix qom/,$(qom-obj-y))
 
+include $(SRC_PATH)/devices/Makefile
+common-obj-y += $(addprefix devices/,$(devices-obj-y))
+
diff --git a/Qconfig b/Qconfig
index 889dfa6..03f2a87 100644
--- a/Qconfig
+++ b/Qconfig
@@ -1,2 +1,3 @@
 source qapi/Qconfig
 source qom/Qconfig
+source devices/Qconfig
diff --git a/configure b/configure
index 93e5e97..6ec1020 100755
--- a/configure
+++ b/configure
@@ -3516,7 +3516,7 @@ DIRS="$DIRS pc-bios/spapr-rtas"
 DIRS="$DIRS roms/seabios roms/vgabios"
 DIRS="$DIRS fsdev ui"
 DIRS="$DIRS qapi"
-DIRS="$DIRS qga qom"
+DIRS="$DIRS qga qom devices"
 FILES="Makefile tests/Makefile"
 FILES="$FILES tests/cris/Makefile tests/cris/.gdbinit"
 FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps"
diff --git a/devices/Makefile b/devices/Makefile
new file mode 100644
index 0000000..fbb0b82
--- /dev/null
+++ b/devices/Makefile
@@ -0,0 +1 @@
+devices-obj-$(CONFIG_DEVICE) := device.o
diff --git a/devices/Qconfig b/devices/Qconfig
new file mode 100644
index 0000000..6a06417
--- /dev/null
+++ b/devices/Qconfig
@@ -0,0 +1,6 @@
+config DEVICE
+       bool "QOM based device model"
+       default y
+       depends on QOM && QAPI_QMP
+       help
+         Device model
diff --git a/devices/device.c b/devices/device.c
new file mode 100644
index 0000000..5d289fc
--- /dev/null
+++ b/devices/device.c
@@ -0,0 +1,125 @@
+#include "qemu/device.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp-input-visitor.h"
+
+static void device_state_accessor(Plug *plug, const char *name, Visitor *v, void *opaque, Error **errp)
+{
+    DeviceClass *class = DEVICE_GET_CLASS(DEVICE(plug));
+    return class->visit(DEVICE(plug), v, name, errp);
+}
+
+static void device_initfn(TypeInstance *obj)
+{
+    Device *device = DEVICE(obj);
+
+    plug_add_property_full(PLUG(device), "state",
+                           device_state_accessor,
+                           device_state_accessor,
+                           NULL,
+                           NULL,
+                           type_get_type(obj),
+                           PROP_F_READWRITE);
+}
+
+void device_visit(Device *device, Visitor *v, const char *name, Error **errp)
+{
+    visit_start_struct(v, (void **)&device, "Device", name, 0, errp);
+    visit_end_struct(v, errp);
+}
+
+static void device_visit_properties(Device *device, bool is_input, const char *name, Visitor *v, Error **errp);
+
+typedef struct DeviceVisitPropertyData
+{
+    Visitor *v;
+    Error **errp;
+    bool is_input;
+} DeviceVisitPropertyData;
+
+static void device_visit_property(Plug *plug, const char *name, const char *typename, int flags, void *opaque)
+{
+    DeviceVisitPropertyData *data = opaque;
+
+    if (strcmp(name, "state") == 0 || strcmp(name, "realized") == 0) {
+        return;
+    }
+
+    if (strstart(typename, "plug<", NULL)) {
+        Device *value = DEVICE(plug_get_property_plug(plug, NULL, name));
+        device_visit_properties(value, data->is_input, name, data->v, data->errp);
+    } else if (data->is_input) {
+        if ((flags & PROP_F_READ) && (flags & PROP_F_WRITE)) {
+            plug_set_property(plug, name, data->v, data->errp);
+        }
+    } else {
+        if (flags & PROP_F_READ) {
+            plug_get_property(plug, name, data->v, data->errp);
+        }
+    }
+}
+
+static void device_visit_properties(Device *device, bool is_input, const char *name, Visitor *v, Error **errp)
+{
+    DeviceVisitPropertyData data = {
+        .v = v,
+        .errp = errp,
+        .is_input = is_input,
+    };
+
+    visit_start_struct(v, (void **)&device, "Device", name, sizeof(Device), errp);
+    plug_foreach_property(PLUG(device), device_visit_property, &data);
+    visit_end_struct(v, errp);
+}
+
+static void device_unrealize(Plug *plug)
+{
+    Device *device = DEVICE(plug);
+    const char *typename;
+    char id[MAX_ID];
+    QmpOutputVisitor *qov;
+    QmpInputVisitor *qiv;
+    Error *local_err = NULL; // FIXME
+
+    snprintf(id, sizeof(id), "%s", type_get_id(TYPE_INSTANCE(device)));
+    typename = type_get_type(TYPE_INSTANCE(device));
+
+    qov = qmp_output_visitor_new();
+
+    device_visit_properties(device, false, id, qmp_output_get_visitor(qov), &local_err);
+
+    type_finalize(device);
+    type_initialize(device, typename, id);
+
+    qiv = qmp_input_visitor_new(qmp_output_get_qobject(qov));
+
+    device_visit_properties(device, true, id, qmp_input_get_visitor(qiv), &local_err);
+
+    qmp_input_visitor_cleanup(qiv);
+    qmp_output_visitor_cleanup(qov);
+}
+
+static void device_class_initfn(TypeClass *type_class)
+{
+    PlugClass *plug_class = PLUG_CLASS(type_class);
+    DeviceClass *class = DEVICE_CLASS(type_class);
+
+    plug_class->unrealize = device_unrealize;
+    class->visit = device_visit;
+}
+
+static const TypeInfo device_type_info = {
+    .name = TYPE_DEVICE,
+    .parent = TYPE_PLUG,
+    .instance_size = sizeof(Device),
+    .class_size = sizeof(DeviceClass),
+    .class_init = device_class_initfn,
+    .instance_init = device_initfn,
+    .abstract = true,
+};
+
+static void register_devices(void)
+{
+    type_register_static(&device_type_info);
+}
+
+device_init(register_devices);
diff --git a/include/qemu/device.h b/include/qemu/device.h
new file mode 100644
index 0000000..8e15232
--- /dev/null
+++ b/include/qemu/device.h
@@ -0,0 +1,28 @@
+#ifndef DEVICE_H
+#define DEVICE_H
+
+#include "qemu/plug.h"
+
+typedef struct Device
+{
+    Plug parent;
+} Device;
+
+typedef void (DeviceVisitor)(Device *device, Visitor *v, const char *name, Error **errp);
+
+typedef struct DeviceClass
+{
+    PlugClass parent_class;
+
+    /* public */
+    DeviceVisitor *visit;
+} DeviceClass;
+
+#define TYPE_DEVICE "device"
+#define DEVICE(obj) TYPE_CHECK(Device, obj, TYPE_DEVICE)
+#define DEVICE_CLASS(class) TYPE_CLASS_CHECK(DeviceClass, class, TYPE_DEVICE)
+#define DEVICE_GET_CLASS(obj) TYPE_GET_CLASS(DeviceClass, obj, TYPE_DEVICE)
+
+void device_visit(Device *device, Visitor *v, const char *name, Error **errp);
+
+#endif
-- 
1.7.4.1

Re: [Qemu-devel] [PATCH 15/21] qom: add Device class

[Peter Maydell]

On 25 July 2011 02:44, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Device is meant to replace DeviceState as the root class for the device model.
> This is included here merely as a RFC.  Device adds a couple of useful features.
>
> 1) Default hard reset.  Device will literally call finalize on the object and
>   then reinitialize it in place.  This means that most devices don't have to
>   worry about implementing reset logic.

I like having a reset implemented as "just reinstantiate everything", but
this only really covers one of the different reset flavours ("simulation
reset", ie "give me a system in the same state as if I'd just started
qemu from scratch"). I think devices are still going to need to implement
"simulated reset", which is what they do when the core causes a simulated
reset signal to go active (and which ought really to be implemented by
having an incoming gpio signal 'reset'). The two are not always identical,
and you don't necessarily want to reset the whole of the model at once...
(And then some devices have another level of 'soft reset' which you get
by writing to one of its registers.)

...all of which isn't particularly relevant to the object/device model,
but I just wanted to say "reset isn't quite that simple" :-)

-- PMM

Re: [Qemu-devel] [PATCH 15/21] qom: add Device class

[Anthony Liguori]

On 07/27/2011 10:10 AM, Peter Maydell wrote:
> On 25 July 2011 02:44, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> Device is meant to replace DeviceState as the root class for the device model.
>> This is included here merely as a RFC.  Device adds a couple of useful features.
>>
>> 1) Default hard reset.  Device will literally call finalize on the object and
>>    then reinitialize it in place.  This means that most devices don't have to
>>    worry about implementing reset logic.
>
> I like having a reset implemented as "just reinstantiate everything", but
> this only really covers one of the different reset flavours ("simulation
> reset", ie "give me a system in the same state as if I'd just started
> qemu from scratch"). I think devices are still going to need to implement
> "simulated reset", which is what they do when the core causes a simulated
> reset signal to go active (and which ought really to be implemented by
> having an incoming gpio signal 'reset').

reset is implemented as an edge of the realized property which is 
basically a pin.

I view realized as the Vcc pin.  In fact, I named it as such originally 
but thought that was too obscure :-)

realize() is when Vcc goes high, unrealize/reset is when Vcc goes low. 
In terms of the very base Device class, there really is only one Vcc 
pin.  For other types of devices, there might be multiple types of pins 
that have reset semantics.

> The two are not always identical,
> and you don't necessarily want to reset the whole of the model at once...
> (And then some devices have another level of 'soft reset' which you get
> by writing to one of its registers.)
>
> ...all of which isn't particularly relevant to the object/device model,
> but I just wanted to say "reset isn't quite that simple" :-)

Am very well aware, and tried to accommodate this.  It's not modelled as 
a Pin because a Pin is a Device and it turns out that the notion of 
realized is also useful for non-Device objects.

Regards,

Anthony Liguori

> -- PMM
>

[Qemu-devel] [PATCH 16/21] qom-devices: add a Pin class

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 devices/Makefile   |    2 +-
 devices/pin.c      |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/pin.h |   28 +++++++++++++++++++++
 3 files changed, 98 insertions(+), 1 deletions(-)
 create mode 100644 devices/pin.c
 create mode 100644 include/qemu/pin.h

diff --git a/devices/Makefile b/devices/Makefile
index fbb0b82..fa52689 100644
--- a/devices/Makefile
+++ b/devices/Makefile
@@ -1 +1 @@
-devices-obj-$(CONFIG_DEVICE) := device.o
+devices-obj-$(CONFIG_DEVICE) := device.o pin.o
diff --git a/devices/pin.c b/devices/pin.c
new file mode 100644
index 0000000..27812d8
--- /dev/null
+++ b/devices/pin.c
@@ -0,0 +1,69 @@
+#include "qemu/pin.h"
+
+void pin_initialize(Pin *pin, const char *id)
+{
+    type_initialize(pin, TYPE_PIN, id);
+}
+
+void pin_finalize(Pin *pin)
+{
+    type_finalize(pin);
+}
+
+void pin_visit(Pin *obj, Visitor *v, const char *name, Error **errp)
+{
+    visit_start_struct(v, (void **)&obj, "Pin", name, sizeof(Pin), errp);
+    device_visit(DEVICE(obj), v, "super", errp);
+    visit_type_bool(v, &obj->level, "level", errp);
+    visit_end_struct(v, errp);
+}
+
+void pin_set_level(Pin *pin, bool value, Error **errp)
+{
+    bool old_level = pin->level;
+
+    pin->level = value;
+
+    if (old_level != value) {
+        notifier_list_notify(&pin->level_changed, NULL);
+    }
+}
+
+bool pin_get_level(Pin *pin, Error **errp)
+{
+    return pin->level;
+}
+
+static void pin_initfn(TypeInstance *inst)
+{
+    Pin *obj = PIN(inst);
+
+    notifier_list_init(&obj->level_changed);
+
+    plug_add_property_bool(PLUG(obj), "level",
+                           (PlugPropertyGetterBool *)pin_get_level, 
+                           (PlugPropertySetterBool *)pin_set_level,
+                           PROP_F_READWRITE);
+}
+
+static void pin_class_initfn(TypeClass *class)
+{
+    DeviceClass *device_class = DEVICE_CLASS(class);
+
+    device_class->visit = (DeviceVisitor *)pin_visit;
+}
+
+static const TypeInfo pin_type_info = {
+    .name = TYPE_PIN,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(Pin),
+    .instance_init = pin_initfn,
+    .class_init = pin_class_initfn,
+};
+
+static void register_devices(void)
+{
+    type_register_static(&pin_type_info);
+}
+
+device_init(register_devices);
diff --git a/include/qemu/pin.h b/include/qemu/pin.h
new file mode 100644
index 0000000..e8f2582
--- /dev/null
+++ b/include/qemu/pin.h
@@ -0,0 +1,28 @@
+#ifndef PIN_H
+#define PIN_H
+
+#include "qemu/device.h"
+#include "notify.h"
+
+typedef struct Pin
+{
+    Device parent;
+
+    /* private */
+    bool level;
+
+    /* FIXME */
+    NotifierList level_changed;
+} Pin;
+
+#define TYPE_PIN "pin"
+#define PIN(obj) TYPE_CHECK(Pin, obj, TYPE_PIN)
+
+void pin_initialize(Pin *pin, const char *id);
+void pin_finalize(Pin *pin);
+void pin_visit(Pin *pin, Visitor *v, const char *name, Error **errp);
+
+void pin_set_level(Pin *pin, bool value, Error **errp);
+bool pin_get_level(Pin *pin, Error **errp);
+
+#endif
-- 
1.7.4.1

[Qemu-devel] [PATCH 17/21] qom: add CharDriver class

[Anthony Liguori]

The CharDriver type replaces the CharDriverState in QEMU today.  Here's how
everything matches up:

 1) qemu_chr_open() no longer exists.  This function used to act as a factory
    and used parsing of the filename to determine which type to create.  A newer
    version using QemuOpts was introduced, qemu_chr_open_opts() which eliminates
    the filename in favor of typed key value pairs.

    Now, plug_create() can be used to create CharDrivers using an explicit
    type that subclasses CharDriver.

 2) query-chardev is deprecated.  This is replaced by
    plug_list(type=char-driver) and plug_list_props().

 3) We can now dynamically add and remove new CharDrivers

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile.qom          |    3 +
 Qconfig               |    1 +
 chrdrv/Makefile       |    1 +
 chrdrv/Qconfig        |    5 +
 chrdrv/chrdrv.c       |  229 ++++++++++++++++++++++++++++++++++++++++
 configure             |    2 +-
 include/qemu/chrdrv.h |  281 +++++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 521 insertions(+), 1 deletions(-)
 create mode 100644 chrdrv/Makefile
 create mode 100644 chrdrv/Qconfig
 create mode 100644 chrdrv/chrdrv.c
 create mode 100644 include/qemu/chrdrv.h

diff --git a/Makefile.qom b/Makefile.qom
index c694cbb..02a5ca5 100644
--- a/Makefile.qom
+++ b/Makefile.qom
@@ -16,3 +16,6 @@ common-obj-y += $(addprefix qom/,$(qom-obj-y))
 include $(SRC_PATH)/devices/Makefile
 common-obj-y += $(addprefix devices/,$(devices-obj-y))
 
+include $(SRC_PATH)/chrdrv/Makefile
+common-obj-y += $(addprefix chrdrv/,$(chrdrv-obj-y))
+
diff --git a/Qconfig b/Qconfig
index 03f2a87..57c1c7d 100644
--- a/Qconfig
+++ b/Qconfig
@@ -1,3 +1,4 @@
 source qapi/Qconfig
 source qom/Qconfig
 source devices/Qconfig
+source chrdrv/Qconfig
diff --git a/chrdrv/Makefile b/chrdrv/Makefile
new file mode 100644
index 0000000..43a51e7
--- /dev/null
+++ b/chrdrv/Makefile
@@ -0,0 +1 @@
+chrdrv-obj-$(CONFIG_CHRDRV) := chrdrv.o
diff --git a/chrdrv/Qconfig b/chrdrv/Qconfig
new file mode 100644
index 0000000..845c205
--- /dev/null
+++ b/chrdrv/Qconfig
@@ -0,0 +1,5 @@
+config CHRDRV
+       bool "QEMU Character Drivers"
+       default y
+       help
+         Character layer
diff --git a/chrdrv/chrdrv.c b/chrdrv/chrdrv.c
new file mode 100644
index 0000000..a9b8dc2
--- /dev/null
+++ b/chrdrv/chrdrv.c
@@ -0,0 +1,229 @@
+#include "qemu/chrdrv.h"
+
+int char_driver_write(CharDriver *s, const uint8_t *buf, int len)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(s);
+
+    return cdc->write(s, buf, len);
+}
+
+int char_driver_ioctl(CharDriver *s, int cmd, void *arg)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(s);
+
+    return cdc->ioctl(s, cmd, arg);
+}
+
+int char_driver_get_msgfd(CharDriver *s)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(s);
+
+    return cdc->get_msgfd(s);
+}
+
+void char_driver_send_event(CharDriver *s, int event)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(s);
+
+    cdc->send_event(s, event);
+}
+
+void char_driver_accept_input(CharDriver *s)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(s);
+
+    cdc->accept_input(s);
+}
+
+void char_driver_set_echo(CharDriver *s, bool echo)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(s);
+
+    cdc->set_echo(s, echo);
+}
+
+void char_driver_guest_open(CharDriver *s)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(s);
+
+    cdc->guest_open(s);
+}
+
+void char_driver_guest_close(CharDriver *s)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(s);
+
+    cdc->guest_close(s);
+}
+
+static int char_driver_def_write(CharDriver *s, const uint8_t *buf, int len)
+{
+    return -ENOTSUP;
+}
+
+static void char_driver_def_update_read_handler(CharDriver *s)
+{
+}
+
+static int char_driver_def_ioctl(CharDriver *s, int cmd, void *arg)
+{
+    return -ENOTSUP;
+}
+
+static int char_driver_def_get_msgfd(CharDriver *s)
+{
+    return -1;
+}
+
+static void char_driver_def_send_event(CharDriver *chr, int event)
+{
+}
+
+static void char_driver_def_close(CharDriver *chr)
+{
+    char_driver_send_event(chr, CHR_EVENT_CLOSED);
+}
+
+static void char_driver_def_accept_input(CharDriver *chr)
+{
+}
+
+static void char_driver_def_set_echo(CharDriver *chr, bool echo)
+{
+}
+
+static void char_driver_def_guest_open(CharDriver *chr)
+{
+}
+
+static void char_driver_def_guest_close(CharDriver *chr)
+{
+}
+
+static void char_driver_def_open(CharDriver *chr, Error **errp)
+{
+}
+
+static void char_driver_realize(Plug *plug)
+{
+    CharDriver *chr = CHAR_DRIVER(plug);
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(chr);
+
+    cdc->open(chr, NULL);
+}
+
+int char_driver_can_read(CharDriver *chr)
+{
+    if (!chr->chr_can_read) {
+        return 1024;
+    }
+
+    return chr->chr_can_read(chr->handler_opaque);
+}
+
+void char_driver_read(CharDriver *chr, uint8_t *buf, int len)
+{
+    if (!chr->chr_read) {
+        return;
+    }
+
+    chr->chr_read(chr->handler_opaque, buf, len);
+}
+
+void char_driver_event(CharDriver *chr, int event)
+{
+    /* Keep track if the char device is open */
+    switch (event) {
+    case CHR_EVENT_OPENED:
+        chr->opened = 1;
+        break;
+    case CHR_EVENT_CLOSED:
+        chr->opened = 0;
+        break;
+    }
+
+    if (!chr->chr_event) {
+        return;
+    }
+
+    chr->chr_event(chr->handler_opaque, event);
+}
+
+static void char_driver_class_init(TypeClass *class)
+{
+    PlugClass *pc = PLUG_CLASS(class);
+    CharDriverClass *cdc = CHAR_DRIVER_CLASS(class);
+
+    pc->realize = char_driver_realize;
+    cdc->write = char_driver_def_write;
+    cdc->ioctl = char_driver_def_ioctl;
+    cdc->get_msgfd = char_driver_def_get_msgfd;
+    cdc->send_event = char_driver_def_send_event;
+    cdc->close = char_driver_def_close;
+    cdc->accept_input = char_driver_def_accept_input;
+    cdc->set_echo = char_driver_def_set_echo;
+    cdc->guest_open = char_driver_def_guest_open;
+    cdc->guest_close = char_driver_def_guest_close;
+    cdc->open = char_driver_def_open;
+    cdc->update_read_handler = char_driver_def_update_read_handler;
+
+}
+
+static void char_driver_generic_open(CharDriver *s)
+{
+    char_driver_event(s, CHR_EVENT_OPENED);
+}
+
+void char_driver_add_handlers(CharDriver *s,
+                              IOCanReadHandler *fd_can_read,
+                              IOReadHandler *fd_read,
+                              IOEventHandler *fd_event,
+                              void *opaque)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(s);
+
+    if (!opaque && !fd_can_read && !fd_read && !fd_event) {
+        /* chr driver being released. */
+        ++s->avail_connections;
+    }
+    s->chr_can_read = fd_can_read;
+    s->chr_read = fd_read;
+    s->chr_event = fd_event;
+    s->handler_opaque = opaque;
+    if (cdc->update_read_handler) {
+        cdc->update_read_handler(s);
+    }
+
+    /* We're connecting to an already opened device, so let's make sure we
+       also get the open event */
+    if (s->opened) {
+        char_driver_generic_open(s);
+    }
+}
+
+static void char_driver_fini(TypeInstance *inst)
+{
+    CharDriver *chr = CHAR_DRIVER(inst);
+    CharDriverClass *cdc = CHAR_DRIVER_GET_CLASS(chr);
+
+    if (cdc->close) {
+        cdc->close(chr);
+    }
+}
+
+static TypeInfo chrdrv_type_info = {
+    .name = TYPE_CHAR_DRIVER,
+    .parent = TYPE_PLUG,
+    .instance_size = sizeof(CharDriver),
+    .instance_finalize = char_driver_fini,
+    .class_size = sizeof(CharDriverClass),
+    .class_init = char_driver_class_init,
+    .abstract = true,
+};
+
+static void register_backends(void)
+{
+    type_register_static(&chrdrv_type_info);
+}
+
+device_init(register_backends);
diff --git a/configure b/configure
index 6ec1020..63c62b0 100755
--- a/configure
+++ b/configure
@@ -3516,7 +3516,7 @@ DIRS="$DIRS pc-bios/spapr-rtas"
 DIRS="$DIRS roms/seabios roms/vgabios"
 DIRS="$DIRS fsdev ui"
 DIRS="$DIRS qapi"
-DIRS="$DIRS qga qom devices"
+DIRS="$DIRS qga qom devices chrdrv"
 FILES="Makefile tests/Makefile"
 FILES="$FILES tests/cris/Makefile tests/cris/.gdbinit"
 FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps"
diff --git a/include/qemu/chrdrv.h b/include/qemu/chrdrv.h
new file mode 100644
index 0000000..075b589
--- /dev/null
+++ b/include/qemu/chrdrv.h
@@ -0,0 +1,281 @@
+#ifndef QEMU_CHAR_DRIVER
+#define QEMU_CHAR_DRIVER
+
+#include "qemu/plug.h"
+
+/* Temporarily for a couple of enum */
+#include "qemu-char.h"
+
+/**
+ * @CharDriver
+ *
+ * A streaming data connection, typically used to transfer data from a @Device
+ * to some process on the host.
+ *
+ * A @CharDriver subclass implements the driver.  Typically, this is the host
+ * routine and may include a TCP, stdio, or fd transport.  The @Device that is
+ * interacting with the driver is the client.  This is typically a device that
+ * looks like a serial port including UARTs, virtio-serial, etc.
+ *
+ * @CharDriver also supports by direction messaging.  These messages carry no
+ * data though.
+ */
+typedef struct CharDriver
+{
+    Plug parent;
+
+    /* Public */
+
+    /**
+     * @avail_connection used by qdev to keep track of how "connections" are
+     * available in the @CharDriver verses how many qdev are using.  This is
+     * meant to ensure that the same @CharDriver isn't connected to multiple
+     * sockets at the same time.
+     *
+     * It's not well thought through though as there are other things that can
+     * use a @CharDriver and the attempt at supporting mux drivers results in
+     * this value just being ignored for mux (although not predictably).
+     *
+     * Sufficed to say, this needs to go away.
+     */
+    int avail_connections;
+
+    /**
+     * @opened despite what the name implies, this doesn't correspond to whether
+     * the @CharDriver is opened.  @CharDriver doesn't have a notion of opened,
+     * instead, @opened tracks whether the CHR_EVENT_OPEN event has been
+     * generated.  The CHR_EVENT_CLOSED event will clear the @opened flag.
+     *
+     * The primary purpose of this flag is to ensure the CHR_EVENT_OPEN event is
+     * not generated twice in a row.
+     */
+    int opened;
+
+    /* Private */
+
+    /**
+     * @chr_can_read when a client has added its handlers to a @CharDriver, this
+     * contains the can read callback for the client.
+     */
+    IOCanReadHandler *chr_can_read;
+
+    /**
+     * @chr_read when a client has added its handlers to a @CharDriver, this
+     * contains the read callback for the client.
+     */
+    IOReadHandler *chr_read;
+
+    /**
+     * @chr_event when a client has added its handlers to a @CharDriver, this
+     * contains the event callback for the client.
+     */
+    IOEventHandler *chr_event;
+
+    /**
+     * @handler_opaque when a client has added its handlers to a @CharDriver,
+     * this contains the opaque associated with the callbacks for the client.
+     */
+    void *handler_opaque;
+} CharDriver;
+
+typedef struct CharDriverClass
+{
+    PlugClass parent_class;
+
+    /* Public */
+    int (*write)(CharDriver *s, const uint8_t *buf, int len);
+    int (*ioctl)(CharDriver *s, int cmd, void *arg);
+    int (*get_msgfd)(CharDriver *s);
+    void (*send_event)(CharDriver *chr, int event);
+    void (*accept_input)(CharDriver *chr);
+    void (*set_echo)(CharDriver *chr, bool echo);
+    void (*guest_open)(CharDriver *chr);
+    void (*guest_close)(CharDriver *chr);
+
+    /* Protected */
+    /**
+     * @open:
+     *
+     * This is called during realize to initialize the object.  At this point,
+     * all of the properties should have been set.
+     */
+    void (*open)(CharDriver *s, Error **errp);
+
+    /**
+     * @update_read_handler:
+     *
+     * This is called after @char_driver_add_handlers is called to allow sub-
+     * classes to re-register their callbacks if necessary.
+     */
+    void (*update_read_handler)(CharDriver *s);
+
+    /**
+     * @close:
+     *
+     * Called during the finalize path.  The default behavior sends a
+     * CHR_EVENT_CLOSED.  It's generally better to use the classes destructor to
+     * implement driver specific cleanup.
+     */
+    void (*close)(CharDriver *chr);
+} CharDriverClass;
+
+#define TYPE_CHAR_DRIVER "char-driver"
+#define CHAR_DRIVER(obj) TYPE_CHECK(CharDriver, obj, TYPE_CHAR_DRIVER)
+#define CHAR_DRIVER_CLASS(class) \
+    TYPE_CLASS_CHECK(CharDriverClass, class, TYPE_CHAR_DRIVER)
+#define CHAR_DRIVER_GET_CLASS(obj) \
+    TYPE_GET_CLASS(CharDriverClass, obj, TYPE_CHAR_DRIVER)
+
+/**
+ * @char_driver_write:
+ *
+ * Write data to a @CharDriver
+ *
+ * @buf      The data to write
+ * @len      The size of the data in buf
+ *
+ * Returns:  The number of bytes written to the device
+ *
+ * Notes:    Each backend deals with flow control on its own.  Depending on the
+ *           driver, this function may block execution or silently drop data.
+ *
+ *           Dropping data may also occur if the backend uses a connection
+ *           oriented transport and the transport is disconnected.
+ *
+ *           The caller receives no indication that data was dropped.  This
+ *           function may return a partial write result.
+ */
+int char_driver_write(CharDriver *s, const uint8_t *buf, int len);
+
+/**
+ * @char_driver_ioctl:
+ *
+ * Performs a device specific ioctl.
+ *
+ * @cmd      an ioctl, see CHR_IOCTL_*
+ * @arg      values depends on @cmd
+ *
+ * Returns:  The result of this depends on the @cmd.  If @cmd is not supported
+ *           by this device, -ENOTSUP.
+ */
+int char_driver_ioctl(CharDriver *s, int cmd, void *arg);
+
+/**
+ * @char_driver_get_msgfd:
+ *
+ * If the driver has received a file descriptor through its transport, this
+ * function will return the file descriptor.
+ *
+ * Returns:  The file descriptor or -1 if transport doesn't support file
+ *           descriptor passing or doesn't currently have a file descriptor.
+ */
+int char_driver_get_msgfd(CharDriver *s);
+
+/**
+ * @char_driver_send_event:
+ *
+ * Raise an event to a driver.
+ *
+ * @event  see CHR_EVENT_*
+ *
+ * Notes:  There is no way to determine if the driver successfully handled the
+ *         event.
+ */
+void char_driver_send_event(CharDriver *chr, int event);
+
+/**
+ * @char_driver_accept_input:
+ *
+ * I honestly can't tell what this function is meant to do.
+ */
+void char_driver_accept_input(CharDriver *chr);
+
+/**
+ * @char_driver_set_echo:
+ *
+ * Requests to override the backends use of echo.  This is really only
+ * applicable to the stdio backend but is a generic interface today.
+ *
+ * @echo true to enable echo
+ */
+void char_driver_set_echo(CharDriver *chr, bool echo);
+
+/**
+ * @char_driver_guest_open:
+ *
+ * If the client has a notion of a connection, this is invoked when the
+ * connection is created.
+ *
+ * Note:  There is no way to determine if a client has the notion of a
+ *        connection.  A driver cannot rely on this function ever being called.
+ */
+void char_driver_guest_open(CharDriver *chr);
+
+/**
+ * @char_driver_guest_close:
+ *
+ * If the client has a notion of a connection, this is invoked when the
+ * connection is closed.
+ *
+ * Note:  There is no way to determine if a client has the notion of a
+ *        connection.  A driver cannot rely on this function ever being called.
+ */
+void char_driver_guest_close(CharDriver *chr);
+
+/**
+ * @char_driver_can_read:
+ *
+ * Returns:  The maximum number of bytes the client connected to the driver can
+ *           receive at the moment.
+ */
+int char_driver_can_read(CharDriver *chr);
+
+/**
+ * @char_driver_read:
+ *
+ * This function transfers the contents of buf to the client.
+ *
+ * @buf  the buffer to write to the client
+ * @len  the number of bytes to write
+ *
+ * Notes:  This function should only be invoked after receiving a non-zero
+ *         value from @char_driver_can_read.  @len may be larger than the return
+ *         value but the results are undefined.  It may result in the entire
+ *         message being dropped or the message being truncated.
+ */
+void char_driver_read(CharDriver *chr, uint8_t *buf, int len);
+
+/**
+ * @char_driver_event:
+ *
+ * Sends an event to the client of a driver.
+ *
+ * @event  the CHR_EVENT_* to send to the client
+ */
+void char_driver_event(CharDriver *chr, int event);
+
+/**
+ * @char_driver_add_handlers:
+ *
+ * Connect a client to a @CharDriver.  A connected client may send data to the
+ * driver by using the @char_driver_write function.  Data is received from the
+ * driver to the client using the callbacks registered in this function.
+ *
+ * @fd_can_read  This callback returns the maximum amount of data the client is
+ *               prepared to receive from the driver.
+ *
+ * @fd_read      This callback is used by the driver to pass data to the client.
+ *
+ * @fd_event     This callback is used to send events from the driver to the
+ *               client.
+ *
+ * @opaque       An opaque value that is passed with the registered callbacks to
+ *               form a closure.
+ */
+void char_driver_add_handlers(CharDriver *s,
+                              IOCanReadHandler *fd_can_read,
+                              IOReadHandler *fd_read,
+                              IOEventHandler *fd_event,
+                              void *opaque);
+
+#endif
-- 
1.7.4.1

[Qemu-devel] [PATCH 18/21] qom-chrdrv: add memory character driver

[Anthony Liguori]

This is used primarly by QMP.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 chrdrv/Makefile       |    1 +
 chrdrv/Qconfig        |    7 +++++
 chrdrv/memchr.c       |   70 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/memchr.h |   46 ++++++++++++++++++++++++++++++++
 4 files changed, 124 insertions(+), 0 deletions(-)
 create mode 100644 chrdrv/memchr.c
 create mode 100644 include/qemu/memchr.h

diff --git a/chrdrv/Makefile b/chrdrv/Makefile
index 43a51e7..6cfdd27 100644
--- a/chrdrv/Makefile
+++ b/chrdrv/Makefile
@@ -1 +1,2 @@
 chrdrv-obj-$(CONFIG_CHRDRV) := chrdrv.o
+chrdrv-obj-$(CONFIG_CHRDRV_MEM) += memchr.o
diff --git a/chrdrv/Qconfig b/chrdrv/Qconfig
index 845c205..f3f939e 100644
--- a/chrdrv/Qconfig
+++ b/chrdrv/Qconfig
@@ -3,3 +3,10 @@ config CHRDRV
        default y
        help
          Character layer
+
+config CHRDRV_MEM
+       bool "Memory character device"
+       default y
+       depends on CHRDRV
+       help
+         Character device that stores all written data to a memory buffer.
diff --git a/chrdrv/memchr.c b/chrdrv/memchr.c
new file mode 100644
index 0000000..3046d2e
--- /dev/null
+++ b/chrdrv/memchr.c
@@ -0,0 +1,70 @@
+#include "qemu/memchr.h"
+
+void memory_char_driver_initialize(MemoryCharDriver *d, const char *id)
+{
+    type_initialize(d, TYPE_MEMORY_CHAR_DRIVER, id);
+}
+
+void memory_char_driver_finalize(MemoryCharDriver *d)
+{
+    type_finalize(d);
+}
+
+static int memory_char_driver_write(CharDriver *chr, const uint8_t *buf,
+                                    int len)
+{
+    MemoryCharDriver *d = MEMORY_CHAR_DRIVER(chr);
+
+    /* TODO: the QString implementation has the same code, we should
+     * introduce a generic way to do this in cutils.c */
+    if (d->outbuf_capacity < d->outbuf_size + len) {
+        /* grow outbuf */
+        d->outbuf_capacity += len;
+        d->outbuf_capacity *= 2;
+        d->outbuf = qemu_realloc(d->outbuf, d->outbuf_capacity);
+    }
+
+    memcpy(d->outbuf + d->outbuf_size, buf, len);
+    d->outbuf_size += len;
+
+    return len;
+}
+
+size_t memory_char_driver_get_osize(MemoryCharDriver *d)
+{
+    return d->outbuf_size;
+}
+
+QString *memory_char_driver_get_qs(MemoryCharDriver *d)
+{
+    return qstring_from_substr((char *) d->outbuf, 0, d->outbuf_size - 1);
+}
+
+static void memory_char_driver_fini(TypeInstance *inst)
+{
+    MemoryCharDriver *d = MEMORY_CHAR_DRIVER(inst);
+
+    qemu_free(d->outbuf);
+}
+
+static void memory_char_driver_class_init(TypeClass *class)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_CLASS(class);
+
+    cdc->write = memory_char_driver_write;
+}
+
+static TypeInfo memory_char_driver_type_info = {
+    .name = TYPE_MEMORY_CHAR_DRIVER,
+    .parent = TYPE_CHAR_DRIVER,
+    .instance_size = sizeof(MemoryCharDriver),
+    .class_init = memory_char_driver_class_init,
+    .instance_finalize = memory_char_driver_fini,
+};
+
+static void register_backends(void)
+{
+    type_register_static(&memory_char_driver_type_info);
+}
+
+device_init(register_backends);
diff --git a/include/qemu/memchr.h b/include/qemu/memchr.h
new file mode 100644
index 0000000..0744684
--- /dev/null
+++ b/include/qemu/memchr.h
@@ -0,0 +1,46 @@
+#ifndef CHAR_DRIVER_MEM_H
+#define CHAR_DRIVER_MEM_H
+
+#include "qemu/chrdrv.h"
+
+/**
+ * @MemoryCharDriver:
+ *
+ * A @CharDriver that stores all written data to memory.  This is useful for
+ * interfacing directly with objects that expect to work with a @CharDriver.
+ *
+ * The accumulated data can be obtained as a QString.
+ */
+typedef struct MemoryCharDriver
+{
+    CharDriver parent;
+
+    /* Private */
+    size_t outbuf_size;
+    size_t outbuf_capacity;
+    uint8_t *outbuf;
+} MemoryCharDriver;
+
+#define TYPE_MEMORY_CHAR_DRIVER "memory-char-driver"
+#define MEMORY_CHAR_DRIVER(obj) \
+    TYPE_CHECK(MemoryCharDriver, obj, TYPE_MEMORY_CHAR_DRIVER)
+
+void memory_char_driver_initialize(MemoryCharDriver *d, const char *id);
+void memory_char_driver_finalize(MemoryCharDriver *d);
+
+/**
+ * @memory_char_driver_get_osize:
+ *
+ * Returns:  The size of the output buffer.
+ */
+size_t memory_char_driver_get_osize(MemoryCharDriver *d);
+
+/**
+ * @memory_char_driver_get_qs:
+ *
+ * Returns:  A QString representing the output buffer.  The reference ownership
+ *           is transferred to the caller.
+ */
+QString *memory_char_driver_get_qs(MemoryCharDriver *d);
+
+#endif
-- 
1.7.4.1

[Qemu-devel] [PATCH 19/21] qom-chrdrv: add Socket base class

[Anthony Liguori]

This introduces two classed, SocketCharDriver and SocketServer.
SocketCharDriver implements the generic logic for all socket based transports.
SocketServer implements the necessary functionality for server-based socket
transports.

Unlike the previous socket CharDriverState, there is no magic overloading based
on passed options.  You have to explicitly create types of socket CharDrivers
for TCP or Unix domain sockets.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 chrdrv/Makefile          |    1 +
 chrdrv/Qconfig           |    7 ++
 chrdrv/socketchr.c       |  250 ++++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/socketchr.h |  109 ++++++++++++++++++++
 4 files changed, 367 insertions(+), 0 deletions(-)
 create mode 100644 chrdrv/socketchr.c
 create mode 100644 include/qemu/socketchr.h

diff --git a/chrdrv/Makefile b/chrdrv/Makefile
index 6cfdd27..84bff48 100644
--- a/chrdrv/Makefile
+++ b/chrdrv/Makefile
@@ -1,2 +1,3 @@
 chrdrv-obj-$(CONFIG_CHRDRV) := chrdrv.o
 chrdrv-obj-$(CONFIG_CHRDRV_MEM) += memchr.o
+chrdrv-obj-$(CONFIG_CHRDRV_SOCKET) += socketchr.o
diff --git a/chrdrv/Qconfig b/chrdrv/Qconfig
index f3f939e..417c063 100644
--- a/chrdrv/Qconfig
+++ b/chrdrv/Qconfig
@@ -10,3 +10,10 @@ config CHRDRV_MEM
        depends on CHRDRV
        help
          Character device that stores all written data to a memory buffer.
+
+config CHRDRV_SOCKET
+       bool "Socket character driver"
+       default y
+       depends on CHRDRV
+       help
+         Character driver that implements a socket based transport.
diff --git a/chrdrv/socketchr.c b/chrdrv/socketchr.c
new file mode 100644
index 0000000..941792c
--- /dev/null
+++ b/chrdrv/socketchr.c
@@ -0,0 +1,250 @@
+#include "qemu/socketchr.h"
+#include "qemu_socket.h"
+
+static int socket_chr_write(CharDriver *chr, const uint8_t *buf, int len)
+{
+    SocketCharDriver *s = SOCKET_CHAR_DRIVER(chr);
+
+    if (!s->connected) {
+        return len;
+    }
+
+    return send_all(s->fd, buf, len);
+}
+    
+static int socket_chr_get_msgfd(CharDriver *chr)
+{
+    SocketCharDriver *s = SOCKET_CHAR_DRIVER(chr);
+    int fd = s->msgfd;
+    s->msgfd = -1;
+    return fd;
+}
+
+#ifndef _WIN32
+static void unix_process_msgfd(CharDriver *chr, struct msghdr *msg)
+{
+    SocketCharDriver *s = SOCKET_CHAR_DRIVER(chr);
+    struct cmsghdr *cmsg;
+
+    for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+        int fd;
+
+        if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) ||
+            cmsg->cmsg_level != SOL_SOCKET ||
+            cmsg->cmsg_type != SCM_RIGHTS) {
+            continue;
+        }
+
+        fd = *((int *)CMSG_DATA(cmsg));
+        if (fd < 0) {
+            continue;
+        }
+
+        if (s->msgfd != -1) {
+            close(s->msgfd);
+        }
+        s->msgfd = fd;
+    }
+}
+
+static ssize_t socket_chr_recv(CharDriver *chr, char *buf, size_t len)
+{
+    SocketCharDriver *s = SOCKET_CHAR_DRIVER(chr);
+    struct msghdr msg = { NULL, };
+    struct iovec iov[1];
+    union {
+        struct cmsghdr cmsg;
+        char control[CMSG_SPACE(sizeof(int))];
+    } msg_control;
+    ssize_t ret;
+
+    iov[0].iov_base = buf;
+    iov[0].iov_len = len;
+
+    msg.msg_iov = iov;
+    msg.msg_iovlen = 1;
+    msg.msg_control = &msg_control;
+    msg.msg_controllen = sizeof(msg_control);
+
+    ret = recvmsg(s->fd, &msg, 0);
+    if (ret > 0) {
+        unix_process_msgfd(chr, &msg);
+    }
+
+    return ret;
+}
+#else
+static ssize_t socket_chr_recv(CharDriver *chr, char *buf, size_t len)
+{
+    SocketCharDriver *s = SOCKET_CHAR_DRIVER(chr);
+    return recv(s->fd, buf, len, 0);
+}
+#endif
+
+#define READ_BUF_LEN 1024
+
+static void socket_chr_try_read(void *opaque)
+{
+    SocketCharDriver *s = opaque;
+    uint8_t buf[READ_BUF_LEN];
+    int len, size;
+    int max_size;
+
+    /* Hopefully no drivers rely on can_read anymore.. */
+    max_size = char_driver_can_read(CHAR_DRIVER(s));
+    if (max_size <= 0) {
+        return;
+    }
+
+    len = sizeof(buf);
+    if (len > max_size) {
+        len = max_size;
+    }
+    size = socket_chr_recv(CHAR_DRIVER(s), (void *)buf, len);
+    if (size == 0) {
+        /* connection closed */
+        s->connected = 0;
+        qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
+        closesocket(s->fd);
+        s->fd = -1;
+        char_driver_event(CHAR_DRIVER(s), CHR_EVENT_CLOSED);
+        // fixme call something that socketserver can hook
+    } else if (size > 0) {
+        char_driver_read(CHAR_DRIVER(s), buf, size);
+    }
+}
+
+void socket_chr_connect(SocketCharDriver *s)
+{
+    s->connected = 1;
+    qemu_set_fd_handler(s->fd, socket_chr_try_read, NULL, s);
+// FIXME    qemu_chr_generic_open(chr);
+}
+
+static void socket_chr_close(CharDriver *chr)
+{
+    SocketCharDriver *s = SOCKET_CHAR_DRIVER(chr);
+
+    if (s->fd >= 0) {
+        qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
+        closesocket(s->fd);
+    }
+    char_driver_event(chr, CHR_EVENT_CLOSED);
+}
+
+static void socket_chr_init(TypeInstance *inst)
+{
+    SocketCharDriver *s = SOCKET_CHAR_DRIVER(inst);
+
+    s->fd = -1;
+}
+
+static void socket_chr_class_init(TypeClass *class)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_CLASS(class);
+
+    cdc->write = socket_chr_write;
+    cdc->close = socket_chr_close;
+    cdc->get_msgfd = socket_chr_get_msgfd;
+}
+
+static TypeInfo socket_chr_info = {
+    .name = TYPE_SOCKET_CHAR_DRIVER,
+    .parent = TYPE_CHAR_DRIVER,
+    .instance_size = sizeof(SocketCharDriver),
+    .class_init = socket_chr_class_init,
+    .instance_init = socket_chr_init,
+    .abstract = true,
+};
+
+/* Socket Server */
+
+static void socket_server_accept(void *opaque)
+{
+    SocketServer *s = SOCKET_SERVER(opaque);
+    SocketCharDriver *scd = SOCKET_CHAR_DRIVER(s);
+    SocketServerClass *ssc = SOCKET_SERVER_GET_CLASS(s);
+    int fd;
+
+    fd = ssc->accept(s);
+    socket_set_nonblock(fd);
+    scd->fd = fd;
+    qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
+    socket_chr_connect(scd);
+}
+
+static void socket_server_open(CharDriver *chr, Error **errp)
+{
+    SocketServer *s = SOCKET_SERVER(chr);
+
+    socket_server_rebind(s);
+}
+
+bool socket_server_get_wait(SocketServer *s, Error **errp)
+{
+    return s->wait;
+}
+
+void socket_server_set_wait(SocketServer *s, bool value, Error **errp)
+{
+    s->wait = value;
+}
+
+void socket_server_rebind(SocketServer *s)
+{
+    SocketServerClass *ssc = SOCKET_SERVER_GET_CLASS(s);
+
+    /* Don't immediately rebind if the backend isn't realized */
+    if (!plug_get_realized(PLUG(s), NULL)) {
+        return;
+    }
+
+    if (s->listen_fd != -1) {
+        qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
+        close(s->listen_fd);
+    }
+
+    s->listen_fd = ssc->make_listen_socket(s);
+    if (!s->wait) {
+        socket_set_nonblock(s->listen_fd);
+        qemu_set_fd_handler(s->listen_fd, socket_server_accept, NULL, s);
+    } else {
+        socket_server_accept(s);
+    }
+}
+
+static void socket_server_init(TypeInstance *inst)
+{
+    SocketServer *s = SOCKET_SERVER(inst);
+
+    s->listen_fd = -1;
+
+    plug_add_property_bool(PLUG(s), "wait",
+                           (PlugPropertyGetterBool *)socket_server_get_wait,
+                           (PlugPropertySetterBool *)socket_server_set_wait,
+                           PROP_F_READWRITE);
+}
+
+static void socket_server_class_init(TypeClass *class)
+{
+    CharDriverClass *cdc = CHAR_DRIVER_CLASS(class);
+    cdc->open = socket_server_open;
+}
+
+static TypeInfo socket_server_info = {
+    .name = TYPE_SOCKET_SERVER,
+    .parent = TYPE_SOCKET_CHAR_DRIVER,
+    .instance_size = sizeof(SocketServer),
+    .class_size = sizeof(SocketServerClass),
+    .class_init = socket_server_class_init,
+    .instance_init = socket_server_init,
+    .abstract = true,
+};
+
+static void register_backends(void)
+{
+    type_register_static(&socket_chr_info);
+    type_register_static(&socket_server_info);
+}
+
+device_init(register_backends);
diff --git a/include/qemu/socketchr.h b/include/qemu/socketchr.h
new file mode 100644
index 0000000..2beb888
--- /dev/null
+++ b/include/qemu/socketchr.h
@@ -0,0 +1,109 @@
+#ifndef CHR_SOCKET_H
+#define CHR_SOCKET_H
+
+#include "qemu/chrdrv.h"
+
+/**
+ * @SocketCharDriver:
+ *
+ * Base class for any @CharDriver that uses a socket as a transport.
+ */
+typedef struct SocketCharDriver
+{
+    CharDriver parent;
+
+    /* Private */
+
+    /**
+     * @connected whether the transport is connected
+     */
+    int connected;
+
+    /**
+     * @fd the data socket fd
+     */
+    int fd;
+
+    /**
+     * @msgfd the received file descriptor from the transport
+     */
+    int msgfd;
+} SocketCharDriver;
+
+typedef struct SocketCharDriverClass
+{
+    CharDriverClass parent_class;
+} SocketCharDriverClass;
+
+#define TYPE_SOCKET_CHAR_DRIVER "socket-char-driver"
+#define SOCKET_CHAR_DRIVER(obj) \
+    TYPE_CHECK(SocketCharDriver, obj, TYPE_SOCKET_CHAR_DRIVER)
+
+void socket_chr_connect(SocketCharDriver *s);
+
+/**
+ * @SocketServer:
+ *
+ * The base class for any socket based transport that accepts connections.
+ */
+typedef struct SocketServer
+{
+    SocketCharDriver parent;
+
+    /* protected */
+
+    /**
+     * @wait if true, then immediate block until a client connects
+     */
+    bool wait;
+
+    /**
+     * @listen_fd the fd of the socket to accept connections on
+     */
+    int listen_fd;
+} SocketServer;
+
+typedef struct SocketServerClass
+{
+    SocketCharDriverClass parent_class;
+
+    /* Protected */
+
+    /**
+     * @make_listen_socket
+     *
+     * This function should create a socket and bind it such that its
+     * suitable for listening.
+     */
+    int (*make_listen_socket)(SocketServer *obj);
+
+    /**
+     * @accept
+     *
+     * This function should accept a connection on @listen_fd and return
+     * a socket representing that connection.
+     */
+    int (*accept)(SocketServer *obj);
+} SocketServerClass;
+
+#define TYPE_SOCKET_SERVER "socket-server"
+#define SOCKET_SERVER(obj) \
+    TYPE_CHECK(SocketServer, obj, TYPE_SOCKET_SERVER)
+#define SOCKET_SERVER_CLASS(class) \
+    TYPE_CLASS_CHECK(SocketServerClass, class, TYPE_SOCKET_SERVER)
+#define SOCKET_SERVER_GET_CLASS(obj) \
+    TYPE_GET_CLASS(SocketServerClass, obj, TYPE_SOCKET_SERVER)
+
+bool socket_server_get_wait(SocketServer *s, Error **errp);
+void socket_server_set_wait(SocketServer *s, bool value, Error **errp);
+
+/**
+ * @socket_server_rebind:
+ *
+ * Rebinds the server by closing and reopening the listen_fd.  This is only
+ * meant to be used by sub classes implementations.
+ */
+void socket_server_rebind(SocketServer *s);
+
+#endif
+
-- 
1.7.4.1

[Qemu-devel] [PATCH 20/21] qom-chrdrv: add TCPServer class

[Anthony Liguori]

This is roughly equivalent to -chardev socket,path=PATH,port=PORT,server=on

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 chrdrv/Makefile       |    1 +
 chrdrv/Qconfig        |    7 ++
 chrdrv/tcpsrv.c       |  203 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/tcpsrv.h |   40 ++++++++++
 4 files changed, 251 insertions(+), 0 deletions(-)
 create mode 100644 chrdrv/tcpsrv.c
 create mode 100644 include/qemu/tcpsrv.h

diff --git a/chrdrv/Makefile b/chrdrv/Makefile
index 84bff48..50a3d50 100644
--- a/chrdrv/Makefile
+++ b/chrdrv/Makefile
@@ -1,3 +1,4 @@
 chrdrv-obj-$(CONFIG_CHRDRV) := chrdrv.o
 chrdrv-obj-$(CONFIG_CHRDRV_MEM) += memchr.o
 chrdrv-obj-$(CONFIG_CHRDRV_SOCKET) += socketchr.o
+chrdrv-obj-$(CONFIG_CHRDRV_TCP_SERVER) += tcpsrv.o
diff --git a/chrdrv/Qconfig b/chrdrv/Qconfig
index 417c063..ea22b50 100644
--- a/chrdrv/Qconfig
+++ b/chrdrv/Qconfig
@@ -17,3 +17,10 @@ config CHRDRV_SOCKET
        depends on CHRDRV
        help
          Character driver that implements a socket based transport.
+
+config CHRDRV_TCP_SERVER
+       bool "TCP server character driver"
+       default y
+       depends on CHRDRV_SOCKET
+       help
+         Character driver that implements a TCP server transport.
diff --git a/chrdrv/tcpsrv.c b/chrdrv/tcpsrv.c
new file mode 100644
index 0000000..bab94f0
--- /dev/null
+++ b/chrdrv/tcpsrv.c
@@ -0,0 +1,203 @@
+#include "qemu/tcpsrv.h"
+
+void tcp_server_initialize(TcpServer *obj, const char *id)
+{
+    type_initialize(obj, TYPE_TCP_SERVER, id);
+}
+
+void tcp_server_finalize(TcpServer *obj)
+{
+    type_finalize(obj);
+}
+
+const char *tcp_server_get_host(TcpServer *obj, Error **errp)
+{
+    return obj->host;
+}
+
+void tcp_server_set_host(TcpServer *obj, const char *value, Error **errp)
+{
+    qemu_free(obj->host);
+    obj->host = qemu_strdup(value);
+
+    socket_server_rebind(SOCKET_SERVER(obj));
+}
+
+const char *tcp_server_get_peername(TcpServer *obj, Error **errp)
+{
+    /* FIXME */
+    return "w00t!";
+}
+
+const char *tcp_server_get_port(TcpServer *obj, Error **errp)
+{
+    return obj->port;
+}
+
+void tcp_server_set_port(TcpServer *obj, const char *value, Error **errp)
+{
+    qemu_free(obj->port);
+    obj->port = qemu_strdup(value);
+
+    socket_server_rebind(SOCKET_SERVER(obj));
+}
+
+bool tcp_server_get_ipv4(TcpServer *obj, Error **errp)
+{
+    return obj->ipv4;
+}
+
+void tcp_server_set_ipv4(TcpServer *obj, bool value, Error **errp)
+{
+    obj->ipv4 = value;
+    socket_server_rebind(SOCKET_SERVER(obj));
+}
+
+bool tcp_server_get_ipv6(TcpServer *obj, Error **errp)
+{
+    return obj->ipv6;
+}
+
+void tcp_server_set_ipv6(TcpServer *obj, bool value, Error **errp)
+{
+    obj->ipv6 = value;
+    socket_server_rebind(SOCKET_SERVER(obj));
+}
+
+static int tcp_server_make_listen_socket(SocketServer *ss)
+{
+    TcpServer *s = TCP_SERVER(ss);
+    struct addrinfo ai, *res, *e;
+    int ret;
+    int fd = -1;
+
+    memset(&ai, 0, sizeof(ai));
+    ai.ai_flags = AI_PASSIVE | AI_ADDRCONFIG;
+    ai.ai_family = PF_UNSPEC;
+    ai.ai_socktype = SOCK_STREAM;
+
+    if (s->ipv4) {
+        ai.ai_family = PF_INET;
+    }
+
+    if (s->ipv6) {
+        ai.ai_family = PF_INET6;
+    }
+
+    ret = getaddrinfo(s->host, s->port, &ai, &res);
+    if (ret != 0) {
+        return -ret;
+    }
+
+    for (e = res; e != NULL; e = e->ai_next) {
+        char uaddr[INET6_ADDRSTRLEN + 1];
+        char uport[32 + 1];
+        int on = 1;
+        int off = 0;
+
+        getnameinfo((struct sockaddr *)e->ai_addr, e->ai_addrlen,
+                    uaddr, INET6_ADDRSTRLEN,
+                    uport, sizeof(uport) - 1,
+                    NI_NUMERICHOST | NI_NUMERICSERV);
+
+        fd = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
+        if (fd == -1) {
+            continue;
+        }
+
+        setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
+#ifdef IPV6_V6ONLY
+        if (e->ai_family == PF_INET6) {
+            /* listen on both ipv4 and ipv6 */
+            setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &off, sizeof(off));
+        }
+#endif
+
+        ret = bind(fd, e->ai_addr, e->ai_addrlen);
+        if (ret == 0) {
+            break;
+        }
+
+        closesocket(fd);
+        fd = -1;
+    }
+
+    listen(fd, 1);
+
+    return fd;
+}
+
+static int tcp_server_accept(SocketServer *ss)
+{
+    TcpServer *s = TCP_SERVER(ss);
+    int fd;
+
+    do {
+        socklen_t addrlen = sizeof(s->peer);
+        fd = qemu_accept(ss->listen_fd, (struct sockaddr *)&s->peer, &addrlen);
+    } while (fd == -1 && errno == EINTR);
+
+    return fd;
+}
+
+static void tcp_server_init(TypeInstance *inst)
+{
+    TcpServer *s = TCP_SERVER(inst);
+
+    plug_add_property_str(PLUG(s), "host",
+                          (PlugPropertyGetterStr *)tcp_server_get_host,
+                          (PlugPropertySetterStr *)tcp_server_set_host,
+                          PROP_F_READWRITE);
+
+    plug_add_property_str(PLUG(s), "port",
+                          (PlugPropertyGetterStr *)tcp_server_get_port,
+                          (PlugPropertySetterStr *)tcp_server_set_port,
+                          PROP_F_READWRITE);
+
+    plug_add_property_bool(PLUG(s), "ipv4",
+                           (PlugPropertyGetterBool *)tcp_server_get_ipv4,
+                           (PlugPropertySetterBool *)tcp_server_set_ipv4,
+                           PROP_F_READWRITE);
+
+    plug_add_property_bool(PLUG(s), "ipv6",
+                           (PlugPropertyGetterBool *)tcp_server_get_ipv6,
+                           (PlugPropertySetterBool *)tcp_server_set_ipv6,
+                           PROP_F_READWRITE);
+
+    plug_add_property_str(PLUG(s), "peername",
+                          (PlugPropertyGetterStr *)tcp_server_get_peername,
+                          NULL,
+                          PROP_F_READ);
+}
+
+static void tcp_server_fini(TypeInstance *inst)
+{
+    TcpServer *s = TCP_SERVER(inst);
+
+    qemu_free(s->port);
+    qemu_free(s->host);
+}
+
+static void tcp_server_class_init(TypeClass *class)
+{
+    SocketServerClass *ssc = SOCKET_SERVER_CLASS(class);
+
+    ssc->accept = tcp_server_accept;
+    ssc->make_listen_socket = tcp_server_make_listen_socket;
+}
+
+static TypeInfo tcp_server_type_info = {
+    .name = TYPE_TCP_SERVER,
+    .parent = TYPE_SOCKET_SERVER,
+    .instance_size = sizeof(TcpServer),
+    .instance_init = tcp_server_init,
+    .instance_finalize = tcp_server_fini,
+    .class_init = tcp_server_class_init,
+};
+
+static void register_backends(void)
+{
+    type_register_static(&tcp_server_type_info);
+}
+
+device_init(register_backends);
diff --git a/include/qemu/tcpsrv.h b/include/qemu/tcpsrv.h
new file mode 100644
index 0000000..c398483
--- /dev/null
+++ b/include/qemu/tcpsrv.h
@@ -0,0 +1,40 @@
+#ifndef CHAR_DRIVER_TCP_H
+#define CHAR_DRIVER_TCP_H
+
+#include "qemu/socketchr.h"
+#include "qemu_socket.h"
+
+typedef struct TcpServer
+{
+    SocketServer parent;
+
+    struct sockaddr_in peer;
+
+    char *host;
+    char *port;
+
+    bool ipv4;
+    bool ipv6;
+} TcpServer;
+
+#define TYPE_TCP_SERVER "tcp-server"
+#define TCP_SERVER(obj) TYPE_CHECK(TcpServer, obj, TYPE_TCP_SERVER)
+
+void tcp_server_initialize(TcpServer *obj, const char *id);
+void tcp_server_finalize(TcpServer *obj);
+
+const char *tcp_server_get_host(TcpServer *obj, Error **errp);
+void tcp_server_set_host(TcpServer *obj, const char *value, Error **errp);
+
+const char *tcp_server_get_port(TcpServer *obj, Error **errp);
+void tcp_server_set_port(TcpServer *obj, const char *value, Error **errp);
+
+bool tcp_server_get_ipv4(TcpServer *obj, Error **errp);
+void tcp_server_set_ipv4(TcpServer *obj, bool value, Error **errp);
+
+bool tcp_server_get_ipv6(TcpServer *obj, Error **errp);
+void tcp_server_set_ipv6(TcpServer *obj, bool value, Error **errp);
+
+const char *tcp_server_get_peername(TcpServer *obj, Error **errp);
+
+#endif
-- 
1.7.4.1

[Qemu-devel] [PATCH 21/21] qom-chrdrv: add UnixServer

[Anthony Liguori]

This is roughly equivalent to -chardev socket,path=PATH,server=on

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 chrdrv/Makefile        |    1 +
 chrdrv/Qconfig         |    7 +++
 chrdrv/unixsrv.c       |  104 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/unixsrv.h |   24 +++++++++++
 4 files changed, 136 insertions(+), 0 deletions(-)
 create mode 100644 chrdrv/unixsrv.c
 create mode 100644 include/qemu/unixsrv.h

diff --git a/chrdrv/Makefile b/chrdrv/Makefile
index 50a3d50..43e516f 100644
--- a/chrdrv/Makefile
+++ b/chrdrv/Makefile
@@ -2,3 +2,4 @@ chrdrv-obj-$(CONFIG_CHRDRV) := chrdrv.o
 chrdrv-obj-$(CONFIG_CHRDRV_MEM) += memchr.o
 chrdrv-obj-$(CONFIG_CHRDRV_SOCKET) += socketchr.o
 chrdrv-obj-$(CONFIG_CHRDRV_TCP_SERVER) += tcpsrv.o
+chrdrv-obj-$(CONFIG_CHRDRV_UNIX_SERVER) += unixsrv.o
diff --git a/chrdrv/Qconfig b/chrdrv/Qconfig
index ea22b50..ae2c20e 100644
--- a/chrdrv/Qconfig
+++ b/chrdrv/Qconfig
@@ -24,3 +24,10 @@ config CHRDRV_TCP_SERVER
        depends on CHRDRV_SOCKET
        help
          Character driver that implements a TCP server transport.
+
+config CHRDRV_UNIX_SERVER
+       bool "UNIX server character driver"
+       default y
+       depends on CHRDRV_SOCKET && UNIX
+       help
+         Character driver that implements a domain socket server transport.
diff --git a/chrdrv/unixsrv.c b/chrdrv/unixsrv.c
new file mode 100644
index 0000000..c4a0621
--- /dev/null
+++ b/chrdrv/unixsrv.c
@@ -0,0 +1,104 @@
+#include "qemu/unixsrv.h"
+
+void unix_server_initialize(UnixServer *obj, const char *id)
+{
+    type_initialize(obj, TYPE_UNIX_SERVER, id);
+}
+
+void unix_server_finalize(UnixServer *obj)
+{
+    type_finalize(obj);
+}
+
+const char *unix_server_get_path(UnixServer *obj, Error **errp)
+{
+    return obj->path;
+}
+
+void unix_server_set_path(UnixServer *obj, const char *path, Error **errp)
+{
+    qemu_free(obj->path);
+    obj->path = qemu_strdup(path);
+
+    socket_server_rebind(SOCKET_SERVER(obj));
+}
+
+static int unix_server_make_listen_socket(SocketServer *ss)
+{
+    UnixServer *s = UNIX_SERVER(ss);
+    struct sockaddr_un addr;
+    int ret;
+    int fd = -1;
+
+    fd = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
+    if (fd == -1) {
+        return -1;
+    }
+
+    addr.sun_family = AF_UNIX;
+    snprintf(addr.sun_path, sizeof(addr.sun_path), "%s", s->path);
+
+    ret = bind(fd, (struct sockaddr *)&addr, sizeof(addr));
+    if (ret == -1) {
+        closesocket(fd);
+        return -1;
+    }
+
+    listen(fd, 1);
+
+    return fd;
+}
+
+static int unix_server_accept(SocketServer *ss)
+{
+    int fd;
+
+    do {
+        struct sockaddr_un addr;
+        socklen_t addrlen = sizeof(addr);
+        fd = qemu_accept(ss->listen_fd, (struct sockaddr *)&addr, &addrlen);
+    } while (fd == -1 && errno == EINTR);
+
+    return fd;
+}
+
+static void unix_server_init(TypeInstance *inst)
+{
+    UnixServer *s = UNIX_SERVER(inst);
+
+    plug_add_property_str(PLUG(s), "path",
+                          (PlugPropertyGetterStr *)unix_server_get_path,
+                          (PlugPropertySetterStr *)unix_server_set_path,
+                          PROP_F_READWRITE);
+}
+
+static void unix_server_fini(TypeInstance *inst)
+{
+    UnixServer *s = UNIX_SERVER(inst);
+
+    qemu_free(s->path);
+}
+
+static void unix_server_class_init(TypeClass *class)
+{
+    SocketServerClass *ssc = SOCKET_SERVER_CLASS(class);
+
+    ssc->accept = unix_server_accept;
+    ssc->make_listen_socket = unix_server_make_listen_socket;
+}
+
+static TypeInfo unix_server_type_info = {
+    .name = TYPE_UNIX_SERVER,
+    .parent = TYPE_SOCKET_SERVER,
+    .instance_size = sizeof(UnixServer),
+    .instance_init = unix_server_init,
+    .instance_finalize = unix_server_fini,
+    .class_init = unix_server_class_init,
+};
+
+static void register_backends(void)
+{
+    type_register_static(&unix_server_type_info);
+}
+
+device_init(register_backends);
diff --git a/include/qemu/unixsrv.h b/include/qemu/unixsrv.h
new file mode 100644
index 0000000..71182bc
--- /dev/null
+++ b/include/qemu/unixsrv.h
@@ -0,0 +1,24 @@
+#ifndef CHAR_DRIVER_UNIX_H
+#define CHAR_DRIVER_UNIX_H
+
+#include "qemu/socketchr.h"
+#include "qemu_socket.h"
+
+typedef struct UnixServer
+{
+    SocketServer parent;
+
+    /* private */
+    char *path;
+} UnixServer;
+
+#define TYPE_UNIX_SERVER "unix-server"
+#define UNIX_SERVER(obj) TYPE_CHECK(UnixServer, obj, TYPE_UNIX_SERVER)
+
+void unix_server_initialize(UnixServer *obj, const char *id);
+void unix_server_finalize(UnixServer *obj);
+
+const char *unix_server_get_path(UnixServer *obj, Error **errp);
+void unix_server_set_path(UnixServer *obj, const char *value, Error **errp);
+
+#endif
-- 
1.7.4.1

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Kevin Wolf]

Am 25.07.2011 03:44, schrieb Anthony Liguori:
> Hi,
> 
> This series is the rough beginnings of the QEMU Object Model.  This is basically
> qdev generalized on steroids.
> 
> This series includes the core infrastructure, a strawman Device type, and
> the beginnings of the conversion of CharDriverState.  This is in a rougher
> state than I would like it to be but I wanted to get the concepts on the list
> as soon as possible.
> 
> My plan is to drop the Device parts from future versions of the series and just
> focus on backends to start with.
> 
> Please note that this series has an awful lot of ramifications.  Most of our
> current command line options would become deprecated, the build system will
> change significantly, and a lot of our QMP functions will become deprecated.
> 
> It seems like a lot of change, but hopefully this series illustrates how we
> can do it very incrementally with value being added at each stage of the
> conversion.

I haven't looked in much detail at it yet, but it has still the same
problem I was talking about last week: Patches 17-21 don't actually
convert existing code, but they add new code. This means that we can't
review only the changes, but have to review the whole code. It also
makes conflicts with patches modifying the old version hard to even notice.


On another note, I'm not so sure if your renaming is really helpful. It
doesn't matter that much with qemu-char because someone thought having
the function pointers in CharDriverState was a good idea, but if you're
consistent, the rename would go like this in the block layer:

BlockDriverState -> BlockDriver
BlockDriver -> BlockDriverClass

IMHO, that's not very helpful, but just going to create confusion. We
could probably discuss other parts of the terminology, too, but let's
save the bikeshedding for later.

Kevin

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/25/2011 06:21 AM, Kevin Wolf wrote:
> Am 25.07.2011 03:44, schrieb Anthony Liguori:
>> Hi,
>>
>> This series is the rough beginnings of the QEMU Object Model.  This is basically
>> qdev generalized on steroids.
>>
>> This series includes the core infrastructure, a strawman Device type, and
>> the beginnings of the conversion of CharDriverState.  This is in a rougher
>> state than I would like it to be but I wanted to get the concepts on the list
>> as soon as possible.
>>
>> My plan is to drop the Device parts from future versions of the series and just
>> focus on backends to start with.
>>
>> Please note that this series has an awful lot of ramifications.  Most of our
>> current command line options would become deprecated, the build system will
>> change significantly, and a lot of our QMP functions will become deprecated.
>>
>> It seems like a lot of change, but hopefully this series illustrates how we
>> can do it very incrementally with value being added at each stage of the
>> conversion.
>
> I haven't looked in much detail at it yet, but it has still the same
> problem I was talking about last week: Patches 17-21 don't actually
> convert existing code, but they add new code.

Actually, it's mostly existing code.  In terms of incremental 
conversion, the most straight forward way is to adds the new version 
side-by-side with the old version and then remove the old version.

Converting in device in place requires some gymnastics.  If you think 
it's absolutely critical, I could try to do it but I'm not sure I agree 
it's the thing to do.

> This means that we can't
> review only the changes, but have to review the whole code. It also
> makes conflicts with patches modifying the old version hard to even notice.
>
>
> On another note, I'm not so sure if your renaming is really helpful. It
> doesn't matter that much with qemu-char because someone thought having
> the function pointers in CharDriverState was a good idea, but if you're
> consistent, the rename would go like this in the block layer:
>
> BlockDriverState ->  BlockDriver
> BlockDriver ->  BlockDriverClass

I think we do need to introduce consistent naming conventions.

If those conventions are FooState and Foo, that could be okay, but the 
code base today is absolutely not consistent on it.

I think Foo and FooClass is better because Foo is the most common usage 
of the type and it's less characters to type.

Regards,

Anthony Liguori

>
> IMHO, that's not very helpful, but just going to create confusion. We
> could probably discuss other parts of the terminology, too, but let's
> save the bikeshedding for later.
>
> Kevin
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Kevin Wolf]

Am 25.07.2011 14:45, schrieb Anthony Liguori:
> On 07/25/2011 06:21 AM, Kevin Wolf wrote:
>> Am 25.07.2011 03:44, schrieb Anthony Liguori:
>>> Hi,
>>>
>>> This series is the rough beginnings of the QEMU Object Model.  This is basically
>>> qdev generalized on steroids.
>>>
>>> This series includes the core infrastructure, a strawman Device type, and
>>> the beginnings of the conversion of CharDriverState.  This is in a rougher
>>> state than I would like it to be but I wanted to get the concepts on the list
>>> as soon as possible.
>>>
>>> My plan is to drop the Device parts from future versions of the series and just
>>> focus on backends to start with.
>>>
>>> Please note that this series has an awful lot of ramifications.  Most of our
>>> current command line options would become deprecated, the build system will
>>> change significantly, and a lot of our QMP functions will become deprecated.
>>>
>>> It seems like a lot of change, but hopefully this series illustrates how we
>>> can do it very incrementally with value being added at each stage of the
>>> conversion.
>>
>> I haven't looked in much detail at it yet, but it has still the same
>> problem I was talking about last week: Patches 17-21 don't actually
>> convert existing code, but they add new code.
> 
> Actually, it's mostly existing code.  In terms of incremental 
> conversion, the most straight forward way is to adds the new version 
> side-by-side with the old version and then remove the old version.
> 
> Converting in device in place requires some gymnastics.  If you think 
> it's absolutely critical, I could try to do it but I'm not sure I agree 
> it's the thing to do.

Okay, if it isn't possible with reasonable effort, I guess we'll have to
bite the bullet and give it a very careful manual review immediately
before the merge.

By the way, I see that you create new directories. You probably have an
idea of what the directory structure will look like after the whole
conversion is completed. Can you share this idea with us?

>> This means that we can't
>> review only the changes, but have to review the whole code. It also
>> makes conflicts with patches modifying the old version hard to even notice.
>>
>>
>> On another note, I'm not so sure if your renaming is really helpful. It
>> doesn't matter that much with qemu-char because someone thought having
>> the function pointers in CharDriverState was a good idea, but if you're
>> consistent, the rename would go like this in the block layer:
>>
>> BlockDriverState ->  BlockDriver
>> BlockDriver ->  BlockDriverClass
> 
> I think we do need to introduce consistent naming conventions.
> 
> If those conventions are FooState and Foo, that could be okay, but the 
> code base today is absolutely not consistent on it.
> 
> I think Foo and FooClass is better because Foo is the most common usage 
> of the type and it's less characters to type.

Maybe. But then, CharDriver is a really bad names for an instance. There
is only one driver, which made it a good name for the class until now.
Maybe CharBackend and CharBackendClass (or CharBackendDriver) would be a
more sensible replacement that follows your pattern.

Kevin

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/25/2011 08:08 AM, Kevin Wolf wrote:
> Am 25.07.2011 14:45, schrieb Anthony Liguori:
> Okay, if it isn't possible with reasonable effort, I guess we'll have to
> bite the bullet and give it a very careful manual review immediately
> before the merge.
>
> By the way, I see that you create new directories. You probably have an
> idea of what the directory structure will look like after the whole
> conversion is completed. Can you share this idea with us?

Just the logic extension of what we have already:

block/
chrdrv/
net/
qom/
qapi/
devices/
   \  pc/
    | pci/
    | scsi/
    | etc.

>> I think Foo and FooClass is better because Foo is the most common usage
>> of the type and it's less characters to type.
>
> Maybe. But then, CharDriver is a really bad names for an instance. There
> is only one driver, which made it a good name for the class until now.
> Maybe CharBackend and CharBackendClass (or CharBackendDriver) would be a
> more sensible replacement that follows your pattern.

Good suggestion.  I've been thinking that there's like to be a need for 
a generic Backend base class too so that would work well from a naming 
convention perspective.

Regards,

Anthony Liguori

>
> Kevin
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/25/2011 03:44 AM, Anthony Liguori wrote:
> Hi,
>
> This series is the rough beginnings of the QEMU Object Model.  This is basically
> qdev generalized on steroids.
>
> This series includes the core infrastructure, a strawman Device type, and
> the beginnings of the conversion of CharDriverState.  This is in a rougher
> state than I would like it to be but I wanted to get the concepts on the list
> as soon as possible.
>
> My plan is to drop the Device parts from future versions of the series and just
> focus on backends to start with.

I am not sure of how the Device and Char/Block/NetworkDriver bits relate 
to each other.  It seems to me that they are two very different families 
of objects, and we should treat them differently.

For host devices (Char/Block/Network) we have a relatively simple 
interface but we need a richer way to create and destroy the objects at 
runtime.  We have it already for block and network, we don't for char, 
but it is done ad hoc for each hierarchy.  We do not have a deep 
hierarchy, in fact there is no hierarchy at all (only one abstract class 
per subsystem).

Honestly, it is not too bad.  It may not be the cleanest code, but is it 
worth the kind of ramifications that your patch has?  Of course, the 
answer could be "yes" if the same model can be applied to devices. 
Devices do have the same introspection needs more or less, and I like 
your ideas there.  However, the requirements are very different in terms 
of composition.

Also because there is no hierarchy, composition in host devices can be 
done very easily.  A decorator for char/block devices, such as a "tee" 
device, can treat the wrapped object(s) the same independent of the 
actual class.  A simple vtable works very well.  GObject would also do 
well, unifying the introspection at the cost of significantly more 
boilerplate.

     (Of course, we could rewrite QEMU in Vala).

For *guest* devices, however, this is not the case.  The PCI host needs 
to know that the device on the other end is a PCI device.  Even the 
simplest bus, for example I2C, has methods defined on its children.

So, the most important point, it seems to me, is to support composition 
effectively.  There need not be any hierarchy at all---not even the 
two-level hierarchy we have now, everything can inherit from 
DeviceState---but the combination between devices must be:

1) in a flexible manner, so that it can express complex topologies (as 
long as "plugs" and "sockets" have the same shape of course);

2) in an easily introspectable manner, so that migration and QMP and so 
on work very well in the presence of said topologies;

3) in a precise manner, so that the two devices being connected can 
interact effectively;


The current qdev fails completely at (1).  (2) is very limited if you 
consider qdev only; qdev+VMState is at least decent, though nowhere near 
the potential of QOM.

However, qdev gets (3) right by making interaction between the parent 
and the child go through a bus object that knows the details of both. 
In particular, bus objects and their vtable structures are very 
effective in qdev because they provide flexibility and avoid impedence 
mismatch, with a very limited amount of boilerplate.

By contrast, the plug/socket model in QOM achieves (1) and (2), but in 
my opinion it fails at (3).  The model for example is not effective in 
specifying the properties of the socket, which are roughly are the bus 
properties and the fields added by DeviceState abstract subclasses in 
the current qdev; it is not clear where they would live in QOM.  Perhaps 
in the parent device or in an intermediate object between the parent and 
the child; if the latter, it looks like a lot of pointer chasing and a 
conversion nightmare.


Based on my earlier conversations with Peter, I thought a bit on how to 
do more incremental changes to qdev in order to overcome the 
inflexibility.  Here is a rough overview of the steps:


1) make properties more flexible.  In particular, we *absolutely* need 
array properties, either static or dynamic.  The current special casing 
of GPIO pins is required exactly because we do not have arrays of 
properties.

Also, since arrays occur a lot in the device state, array properties 
would also be required to be able to introspect on run-time properties 
(which are now part of VMState only).  However, I am not going to 
consider run-time introspection much more.  It is "a simple matter of 
programming" if your general introspection design is done right.


2) add more kinds of first-class (user specifiable) properties.  GPIO 
pins, for example, which are already part of qdev but cannot be 
specified by the user on the command line.  Possibly memory regions too. 
  Anything that can be used to configure a device (with respect to the 
main bus or to its parent) should be a property.


3) at this point, you can get rid of the specialties of SysBus devices. 
  IRQs are an array of GPIO properties, same for memory regions.  SysBus 
devices can use a naked DeviceState structure.

At this point, many embedded devices could be entirely described in 
terms of configuration files, except for legacy options (-serial kind) 
and for the few devices that attach directly to the CPU.  This without 
the need to define artificial buses that do not exist in real hardware.


4) add support for compound properties.  A compound properties is such 
that you define a property named (coincidence...) "bus" and you get 
automatically "bus.parent", "bus.addr", "bus.romfile", "bus.rombar" etc.


5) convert buses to compound properties.  Rather than inheriting from 
PCIDevice, a PCI device would inherit straight from DeviceState and 
include a PCIDevice struct that defines the backlink from a device to 
its parent.  Note that since we're using C, this is not a big change 
from what we're doing now!  (Inheritance by containment is a special 
case of containment.)  And it allows to define very flexibly a device 
that would have to sit on two or more buses in the current qdev model. 
More importantly, it keeps the effectiveness of the qbus ops model, 
while removing the constraint of a tree topology.


It is only when this is done, that we should reconsider introspection 
and see if it is useful/sensible to unify the host and guest device 
object models.

Because the host models we have now, with all the defects they may have, 
*work*.  The guest models hardly work for anything but PCs, and even 
then only for the subset of devices which are interesting to 
hot-plug/unplug.  And so it is from them that we should start designing.

Thoughts?

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/26/2011 07:59 AM, Paolo Bonzini wrote:
> For host devices (Char/Block/Network) we have a relatively simple
> interface but we need a richer way to create and destroy the objects at
> runtime. We have it already for block and network, we don't for char,
> but it is done ad hoc for each hierarchy. We do not have a deep
> hierarchy, in fact there is no hierarchy at all (only one abstract class
> per subsystem).
>
> Honestly, it is not too bad. It may not be the cleanest code, but is it
> worth the kind of ramifications that your patch has? Of course, the
> answer could be "yes" if the same model can be applied to devices.
> Devices do have the same introspection needs more or less, and I like
> your ideas there. However, the requirements are very different in terms
> of composition.
>
> Also because there is no hierarchy, composition in host devices can be
> done very easily. A decorator for char/block devices, such as a "tee"
> device, can treat the wrapped object(s) the same independent of the
> actual class. A simple vtable works very well. GObject would also do
> well, unifying the introspection at the cost of significantly more
> boilerplate.

The polymorphism model of QOM is identical to GObject so I'm not sure 
what you mean here.

In the case of tee, it's just an object with two sockets.


> (Of course, we could rewrite QEMU in Vala).
>
> For *guest* devices, however, this is not the case. The PCI host needs
> to know that the device on the other end is a PCI device. Even the
> simplest bus, for example I2C, has methods defined on its children.

I have PCI patches, but didn't post them in the series.  Here's how it 
works:

The PCI host controller, the i440fx, has 32 sockets of PCIDevice. 
PCIDevice is a base class.

The PCI host controller implements a PCIBus interface.  The PCIDevices 
have a socket of a PCIBus

Connecting a PCIDevice to the host bus involves setting the socket on 
the PCI host controller with the PCIDevice and then setting the 
PCIDevice's bus socket with the host controller.

A PCIDevice can also be a PCIBus by implementing the PCIBus interface. 
This is what enables a PCI bridge to make sense in this model.

If you're interested, the tree that has this is 
http://repo.or.cz/w/qemu/aliguori.git/tree/qdev2:/devices

> So, the most important point, it seems to me, is to support composition
> effectively. There need not be any hierarchy at all---not even the
> two-level hierarchy we have now, everything can inherit from
> DeviceState---but the combination between devices must be:

I think composition is being overloaded here.  When I say composition, I 
mean that one device is created out of multiple other devices. 
Something like the PIIX3 is composed of an RTC, UART, etc.

That's different that connecting up the links in the device tree graph.

> 1) in a flexible manner, so that it can express complex topologies (as
> long as "plugs" and "sockets" have the same shape of course);

Right, this is what we do today in QOM.  Plugs and Sockets are typed. 
Those types can be interfaces or base classes so there's a lot of 
flexibility.

> 2) in an easily introspectable manner, so that migration and QMP and so
> on work very well in the presence of said topologies;

The 'qsh' tree can view the entire device model as a synthetic file 
system.  Plugs are represented as sub directories and sockets as 
symbolic links.  I plan on writing a quick FUSE file system too.

There is type information with all of the attributes that's not visible 
in this view but it's there nonetheless.

> 3) in a precise manner, so that the two devices being connected can
> interact effectively;
>
>
> The current qdev fails completely at (1). (2) is very limited if you
> consider qdev only; qdev+VMState is at least decent, though nowhere near
> the potential of QOM.
>
> However, qdev gets (3) right by making interaction between the parent
> and the child go through a bus object that knows the details of both. In
> particular, bus objects and their vtable structures are very effective
> in qdev because they provide flexibility and avoid impedence mismatch,
> with a very limited amount of boilerplate.
>
> By contrast, the plug/socket model in QOM achieves (1) and (2), but in
> my opinion it fails at (3). The model for example is not effective in
> specifying the properties of the socket, which are roughly are the bus
> properties and the fields added by DeviceState abstract subclasses in
> the current qdev;

There are no properties of the socket.

If you look at something like adding a PCI device in qdev, you add a 
child and set properties of the child to identify how the device sits on 
the PCI bus.

I'd characterize this as awkward, at best.  The slot index is not a 
property of the device, it's a property of how the device is connected 
to the PCI bus.

In my attempt at PCI modelling, I had something like:

struct I440FX
{
    Device parent;

    PciDevice slots[32];
};

Which means that to attach to bus 0, slot 3, fn 0, you do:

i440fx->slots[3] = mydevice

Likewise, if slot 4 contains a PCI-to-PCI bridge that ends up being bus 
1, and you want to assign to bus 1, slot 2, fn 0:

i440fx->slots[4]->slots[2] = myotherdevice;

Now you may observe that this is awkward compared to saying "bus 1". 
But bus numbering is actually a guest concept, not a hardware concept. 
When constructing the device tree, the physical topology is actually 
what make sense in terms of constructing devices, not a guest derived 
numbering concept.

The same applies equally to IDE.  You'd have:

struct IDEBus
{
    IDEDevice *master, *slave;
};

struct IDEChipset
{
    IDEBus primary;
    IDEBus secondary;
};

This would result in:

ide->primary.master = disk1;
ide->secondary.master = cdrom;

> it is not clear where they would live in QOM. Perhaps
> in the parent device or in an intermediate object between the parent and
> the child; if the latter, it looks like a lot of pointer chasing and a
> conversion nightmare.

It's a characteristic of the property itself.  It's definitely very 
different than how we do it today.

>
> Based on my earlier conversations with Peter, I thought a bit on how to
> do more incremental changes to qdev in order to overcome the
> inflexibility. Here is a rough overview of the steps:
>
>
> 1) make properties more flexible. In particular, we *absolutely* need
> array properties, either static or dynamic. The current special casing
> of GPIO pins is required exactly because we do not have arrays of
> properties.
>
> Also, since arrays occur a lot in the device state, array properties
> would also be required to be able to introspect on run-time properties
> (which are now part of VMState only). However, I am not going to
> consider run-time introspection much more. It is "a simple matter of
> programming" if your general introspection design is done right.

Arrays of properties already work in QOM.  Part of the trouble with qdev 
today (and QObject for that matter), is that properties are a 
characteristic of a class, not an object.  This is the primary 
difference between the QOM Plug and is required for proper array support.

> 2) add more kinds of first-class (user specifiable) properties. GPIO
> pins, for example, which are already part of qdev but cannot be
> specified by the user on the command line. Possibly memory regions too.
> Anything that can be used to configure a device (with respect to the
> main bus or to its parent) should be a property.
>
>
> 3) at this point, you can get rid of the specialties of SysBus devices.
> IRQs are an array of GPIO properties, same for memory regions. SysBus
> devices can use a naked DeviceState structure.
>
> At this point, many embedded devices could be entirely described in
> terms of configuration files, except for legacy options (-serial kind)
> and for the few devices that attach directly to the CPU. This without
> the need to define artificial buses that do not exist in real hardware.
>
>
> 4) add support for compound properties. A compound properties is such
> that you define a property named (coincidence...) "bus" and you get
> automatically "bus.parent", "bus.addr", "bus.romfile", "bus.rombar" etc.

Because QOM uses visitors for properties, properties can be arbitrarily 
complicated structures.  How that maps to command line arguments, I 
haven't thought through yet but you can certainly do:

plug_set(obj, "bus", { 'parent': x, 'addr': y, ...})

>
> 5) convert buses to compound properties. Rather than inheriting from
> PCIDevice, a PCI device would inherit straight from DeviceState and
> include a PCIDevice struct that defines the backlink from a device to
> its parent. Note that since we're using C, this is not a big change from
> what we're doing now! (Inheritance by containment is a special case of
> containment.) And it allows to define very flexibly a device that would
> have to sit on two or more buses in the current qdev model. More
> importantly, it keeps the effectiveness of the qbus ops model, while
> removing the constraint of a tree topology.

Interfaces are the right way to do this.  Getting MI right is fairly 
hard and since qdev uses a simple container_of, it's not going to work 
very well.  That's another advantage of QOM, casting from interfaces to 
objects Just Works.

>
>
> It is only when this is done, that we should reconsider introspection
> and see if it is useful/sensible to unify the host and guest device
> object models.
>
> Because the host models we have now, with all the defects they may have,
> *work*. The guest models hardly work for anything but PCs, and even then
> only for the subset of devices which are interesting to hot-plug/unplug.
> And so it is from them that we should start designing.
>
> Thoughts?

I think all of the requirements you've outlined are currently handled in 
QOM.  I think it would probably be a good idea to set up a wiki page 
that focused just on qdev-next requirements and that would give us a 
rigorious mechanism to evaluate how well QOM satisfies those requirements.

I haven't thought through how to do OQM incrementally with qdev.  It's 
probably possible but I wanted to focus on the backends first before 
really attacking that.

Regards,

Anthony Liguori

>
> Paolo
>
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/26/2011 04:02 PM, Anthony Liguori wrote:
>> Also because there is no hierarchy, composition in host devices can be
>> done very easily. A decorator for char/block devices, such as a "tee"
>> device, can treat the wrapped object(s) the same independent of the
>> actual class. A simple vtable works very well. GObject would also do
>> well, unifying the introspection at the cost of significantly more
>> boilerplate.
>
> The polymorphism model of QOM is identical to GObject so I'm not sure
> what you mean here.

GObject instead of QOM (just so that we have something that is already 
written).

> In the case of tee, it's just an object with two sockets.

Yes, understood.

> I have PCI patches, but didn't post them in the series. Here's how it
> works:
>
> The PCI host controller, the i440fx, has 32 sockets of PCIDevice.
> PCIDevice is a base class.

And as such it can add data members.  But when a device is on two buses, 
you cannot have both of them adding data members.  I know MI is hard to 
get right, and in fact I'm not proposing to do MI---not even interface 
inheritance.  I don't want to have any base class but DeviceState.

> The PCI host controller implements a PCIBus interface. The PCIDevices
> have a socket of a PCIBus
>
> Connecting a PCIDevice to the host bus involves setting the socket on
> the PCI host controller with the PCIDevice and then setting the
> PCIDevice's bus socket with the host controller.
>
> A PCIDevice can also be a PCIBus by implementing the PCIBus interface.
> This is what enables a PCI bridge to make sense in this model.
>
> If you're interested, the tree that has this is
> http://repo.or.cz/w/qemu/aliguori.git/tree/qdev2:/devices

Yes, this is pretty much what I had imagined.  But it does not scale to 
a topology where you have two parents, both of which want to add data 
members.

>> 1) in a flexible manner, so that it can express complex topologies (as
>> long as "plugs" and "sockets" have the same shape of course);
>
> Right, this is what we do today in QOM. Plugs and Sockets are typed.
> Those types can be interfaces or base classes so there's a lot of
> flexibility.

Interfaces (is-a) are less flexible than embedded objects (has-a).

> There are no properties of the socket.
>
> If you look at something like adding a PCI device in qdev, you add a
> child and set properties of the child to identify how the device sits on
> the PCI bus.
>
> I'd characterize this as awkward, at best. The slot index is not a
> property of the device, it's a property of how the device is connected
> to the PCI bus.

Yes, for a PCI address I agree.  But in a (parallel) SCSI bus, the LUN 
is logically a property of the device.  Same as IDE when you used to set 
jumpers to choose master/slave.  Or ISA interrupt lines.

Once you have something like this for a device that bridges two buses, 
interfaces require a lot of boilerplate for useless getters/setters.

> i440fx->slots[3] = mydevice
>
> Likewise, if slot 4 contains a PCI-to-PCI bridge that ends up being bus
> 1, and you want to assign to bus 1, slot 2, fn 0:
>
> i440fx->slots[4]->slots[2] = myotherdevice;
>
> Now you may observe that this is awkward compared to saying "bus 1".

No, I have no problem with that. :)

> The same applies equally to IDE.
>
> ide->primary.master = disk1;
> ide->secondary.master = cdrom;

For IDE, an equally good model would be:

     ide->primary.add(disk1);
     disk1.masterSlave = MASTER;
     ide->secondary.add(cdrom);
     cdrom.masterSlave = MASTER;

>> 5) convert buses to compound properties. Rather than inheriting from
>> PCIDevice, a PCI device would inherit straight from DeviceState and
>> include a PCIDevice struct that defines the backlink from a device to
>> its parent. Note that since we're using C, this is not a big change from
>> what we're doing now! (Inheritance by containment is a special case of
>> containment.) And it allows to define very flexibly a device that would
>> have to sit on two or more buses in the current qdev model. More
>> importantly, it keeps the effectiveness of the qbus ops model, while
>> removing the constraint of a tree topology.
>
> Interfaces are the right way to do this. Getting MI right is fairly hard

But we don't need is-a, we need has-a.  Multiple is-a is harder than 
single is-a.  Multiple has-a does not add any complication.

> I think all of the requirements you've outlined are currently handled in
> QOM.

They more than likely are.  The question is whether they're handled in 
the most programmer-efficient manner, and whether the advantages of a 
single grand unified object model for host and guest devices is worth 
the effort.

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/26/2011 09:35 AM, Paolo Bonzini wrote:
> On 07/26/2011 04:02 PM, Anthony Liguori wrote:
>>> Also because there is no hierarchy, composition in host devices can be
>>> done very easily. A decorator for char/block devices, such as a "tee"
>>> device, can treat the wrapped object(s) the same independent of the
>>> actual class. A simple vtable works very well. GObject would also do
>>> well, unifying the introspection at the cost of significantly more
>>> boilerplate.
>>
>> The polymorphism model of QOM is identical to GObject so I'm not sure
>> what you mean here.
>
> GObject instead of QOM (just so that we have something that is already
> written).
>
>> In the case of tee, it's just an object with two sockets.
>
> Yes, understood.
>
>> I have PCI patches, but didn't post them in the series. Here's how it
>> works:
>>
>> The PCI host controller, the i440fx, has 32 sockets of PCIDevice.
>> PCIDevice is a base class.
>
> And as such it can add data members. But when a device is on two buses,
> you cannot have both of them adding data members. I know MI is hard to
> get right, and in fact I'm not proposing to do MI---not even interface
> inheritance. I don't want to have any base class but DeviceState.

I could use a concrete example here, but here's how this would be expressed:

class MyWeirdDevice : public MyBaseDevice, implements PciDevice, IsaDevice
{
     PciBus *pci_bus;
     IsaDevice *isa_bus;
};

Which actually models hw pretty well.  A device that exists on two 
busses has to have two sockets that can accept the plugs from the busses 
they're joining.

It's pretty much exactly how it works in real life.

>
>> The PCI host controller implements a PCIBus interface. The PCIDevices
>> have a socket of a PCIBus
>>
>> Connecting a PCIDevice to the host bus involves setting the socket on
>> the PCI host controller with the PCIDevice and then setting the
>> PCIDevice's bus socket with the host controller.
>>
>> A PCIDevice can also be a PCIBus by implementing the PCIBus interface.
>> This is what enables a PCI bridge to make sense in this model.
>>
>> If you're interested, the tree that has this is
>> http://repo.or.cz/w/qemu/aliguori.git/tree/qdev2:/devices
>
> Yes, this is pretty much what I had imagined. But it does not scale to a
> topology where you have two parents, both of which want to add data
> members.

Which would be true MI.  The problem with true MI is that you'll end up 
with a diamond if you try to have anything useful (like properties) in 
the base.

The common solution is to have the ability to has-a implementation of 
the interface.

>>> 1) in a flexible manner, so that it can express complex topologies (as
>>> long as "plugs" and "sockets" have the same shape of course);
>>
>> Right, this is what we do today in QOM. Plugs and Sockets are typed.
>> Those types can be interfaces or base classes so there's a lot of
>> flexibility.
>
> Interfaces (is-a) are less flexible than embedded objects (has-a).

It depends on what you're trying to model I guess.  In some cases where 
the interface is more or less stateless or that state isn't common among 
implementations, interfaces are quite nice.

>> There are no properties of the socket.
>>
>> If you look at something like adding a PCI device in qdev, you add a
>> child and set properties of the child to identify how the device sits on
>> the PCI bus.
>>
>> I'd characterize this as awkward, at best. The slot index is not a
>> property of the device, it's a property of how the device is connected
>> to the PCI bus.
>
> Yes, for a PCI address I agree. But in a (parallel) SCSI bus, the LUN is
> logically a property of the device.

 > Same as IDE when you used to set
> jumpers to choose master/slave. Or ISA interrupt lines.

The ISA dip switches literally select which line of the bus gets routed 
to the device.  All devices have access to all interrupt lines through 
the bus.

So when modelling, it makes sense to have the irq be a property of the 
device and then to have the bus interface expose the irqs.  For instance:

struct IsaBusClass
{
    void (*set_irq_level)(IsaBus *bus, int irq, bool level);
};

I don't know enough about SCSI to know whether it makes sense to have 
LUN be a property of the bus or the device.  If LUN negotiation is aided 
by the controller and fixed after startup, then I think it makes sense 
for there to be a fixed number of sockets in the bus and then for the 
bus to do LUN assignment at realize (possibly looking at each device to 
see what the requested LUN is).

> Once you have something like this for a device that bridges two buses,
> interfaces require a lot of boilerplate for useless getters/setters.

Can you elaborate?

>
>> i440fx->slots[3] = mydevice
>>
>> Likewise, if slot 4 contains a PCI-to-PCI bridge that ends up being bus
>> 1, and you want to assign to bus 1, slot 2, fn 0:
>>
>> i440fx->slots[4]->slots[2] = myotherdevice;
>>
>> Now you may observe that this is awkward compared to saying "bus 1".
>
> No, I have no problem with that. :)
>
>> The same applies equally to IDE.
>>
>> ide->primary.master = disk1;
>> ide->secondary.master = cdrom;
>
> For IDE, an equally good model would be:
>
> ide->primary.add(disk1);
> disk1.masterSlave = MASTER;
> ide->secondary.add(cdrom);
> cdrom.masterSlave = MASTER;

There's a pin in the IDE cable that determines master or slave depending 
on whether it's raised high.  You can think of it as a very simple MUX 
circuit where the pin decodes the single group of 40 wires into two 
distinct groups of 39 wires, each group being a different port on the bus.

I think modelling it that way makes more sense from an end user 
perspective since it prevents the possibility of have two master devices 
(which is incorrect).

>
>>> 5) convert buses to compound properties. Rather than inheriting from
>>> PCIDevice, a PCI device would inherit straight from DeviceState and
>>> include a PCIDevice struct that defines the backlink from a device to
>>> its parent. Note that since we're using C, this is not a big change from
>>> what we're doing now! (Inheritance by containment is a special case of
>>> containment.) And it allows to define very flexibly a device that would
>>> have to sit on two or more buses in the current qdev model. More
>>> importantly, it keeps the effectiveness of the qbus ops model, while
>>> removing the constraint of a tree topology.
>>
>> Interfaces are the right way to do this. Getting MI right is fairly hard
>
> But we don't need is-a, we need has-a. Multiple is-a is harder than
> single is-a. Multiple has-a does not add any complication.

Yeah, that's what plug properties are for :-)  Making has-a work well is 
one of the issues I had with GObject.  GObject is not very good for 
expressing has-a.

>
>> I think all of the requirements you've outlined are currently handled in
>> QOM.
>
> They more than likely are. The question is whether they're handled in
> the most programmer-efficient manner, and whether the advantages of a
> single grand unified object model for host and guest devices is worth
> the effort.

Indeed.  I think that it's a no brainer for the backends and that's why 
I'm starting there.

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/26/2011 05:34 PM, Anthony Liguori wrote:
>> And as such it can add data members. But when a device is on two buses,
>> you cannot have both of them adding data members. I know MI is hard to
>> get right, and in fact I'm not proposing to do MI---not even interface
>> inheritance. I don't want to have any base class but DeviceState.
>
> I could use a concrete example here, but here's how this would be
> expressed:
>
> class MyWeirdDevice : public MyBaseDevice, implements PciDevice, IsaDevice
> {
> PciBus *pci_bus;
> IsaDevice *isa_bus;
> };
>
> Which actually models hw pretty well. A device that exists on two busses
> has to have two sockets that can accept the plugs from the busses
> they're joining.
>
> It's pretty much exactly how it works in real life.

You could just as well say that real life works like this:

class PciSocket {
     PciBus *pci_bus;
     uint8_t *config;
     uint8_t *cmask;
     uint8_t *wmask;
     uint8_t *w1cmask;
     uint8_t *used;
     uint32_t devfn;
     char name[64];
     PCIIORegion io_regions[PCI_NUM_REGIONS];
     ...

};

class IsaSocket {
     IsaBus *isa_bus;
     uint32_t isairq[2];  // not sure this is a good idea, just
     int nirqs;           // mimicking qdev
     uint16_t ioports[32];// statically assigned unlike PCI bars
     int nioports;
}

class MyWeirdDevice : public MyBaseDevice {
     PciSocket pci;
     IsaSocket isa;
};


>> Yes, this is pretty much what I had imagined. But it does not scale to a
>> topology where you have two parents, both of which want to add data
>> members.
>
> Which would be true MI. The problem with true MI is that you'll end up
> with a diamond if you try to have anything useful (like properties) in
> the base.

See above for how you would avoid that.

>>>> 1) in a flexible manner, so that it can express complex topologies (as
>>>> long as "plugs" and "sockets" have the same shape of course);
>>>
>>> Right, this is what we do today in QOM. Plugs and Sockets are typed.
>>> Those types can be interfaces or base classes so there's a lot of
>>> flexibility.
>>
>> Interfaces (is-a) are less flexible than embedded objects (has-a).
>
> It depends on what you're trying to model I guess. In some cases where
> the interface is more or less stateless or that state isn't common among
> implementations, interfaces are quite nice.

Yes, I'm not sure this is the case here. :)

>  > Same as IDE when you used to set
>> jumpers to choose master/slave. Or ISA interrupt lines.
>
> The ISA dip switches literally select which line of the bus gets routed
> to the device. All devices have access to all interrupt lines through
> the bus.
>
> So when modelling, it makes sense to have the irq be a property of the
> device and then to have the bus interface expose the irqs. For instance:
>
> struct IsaBusClass
> {
> void (*set_irq_level)(IsaBus *bus, int irq, bool level);
> };

Agreed.  Though I'm not sure that the same is true for all fields in 
qemu's current PCIDevice.

>> Once you have something like this for a device that bridges two buses,
>> interfaces require a lot of boilerplate for useless getters/setters.
>
> Can you elaborate?

If you store data (configuration space etc.) in the device, and the bus 
has to access it, you need getters/setters in the interface.  Letting 
the bus hold an interior reference to the PciSocket (perhaps adding a 
single get_device_for_socket function to the PciSocketOps) solves the 
problem.

>>> The same applies equally to IDE.
>>>
>>> ide->primary.master = disk1;
>>> ide->secondary.master = cdrom;
>>
>> For IDE, an equally good model would be:
>>
>> ide->primary.add(disk1);
>> disk1.masterSlave = MASTER;
>> ide->secondary.add(cdrom);
>> cdrom.masterSlave = MASTER;
>
> There's a pin in the IDE cable that determines master or slave depending
> on whether it's raised high.

Yes, that's the "newer" way.  There used to be jumpers to choose between 
master, slave and cable-select.

> I think modelling it that way makes more sense from an end user
> perspective since it prevents the possibility of have two master devices
> (which is incorrect).

... but you could do that, if for some reason you wanted to model the 
jumper-based world.  But this is a mostly irrelevant digression.

>>> Interfaces are the right way to do this. Getting MI right is fairly hard
>>
>> But we don't need is-a, we need has-a. Multiple is-a is harder than
>> single is-a. Multiple has-a does not add any complication.
>
> Yeah, that's what plug properties are for :-)

I agree, but at the cost of pointer chasing and making it harder to 
implement get_device_for_socket for buses that need it (in the above 
sketch it can be a simple container_of).

>>> I think all of the requirements you've outlined are currently handled in
>>> QOM.
>>
>> They more than likely are. The question is whether they're handled in
>> the most programmer-efficient manner, and whether the advantages of a
>> single grand unified object model for host and guest devices is worth
>> the effort.
>
> Indeed. I think that it's a no brainer for the backends and that's why
> I'm starting there.

I don't think it's a no brainer.  It's simply much easier, but right now 
it is also a solution in search of a problem (if all you want is dynamic 
creation of character devices, you could do that without a generic 
object model).

If starting from a blank slate, I would be much more enthusiastic.  But 
all that QEMU does _not_ need is yet another incomplete transition.

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/26/2011 01:26 PM, Paolo Bonzini wrote:
> On 07/26/2011 05:34 PM, Anthony Liguori wrote:
>>> And as such it can add data members. But when a device is on two buses,
>>> you cannot have both of them adding data members. I know MI is hard to
>>> get right, and in fact I'm not proposing to do MI---not even interface
>>> inheritance. I don't want to have any base class but DeviceState.
>>
>> I could use a concrete example here, but here's how this would be
>> expressed:
>>
>> class MyWeirdDevice : public MyBaseDevice, implements PciDevice,
>> IsaDevice
>> {
>> PciBus *pci_bus;
>> IsaDevice *isa_bus;
>> };
>>
>> Which actually models hw pretty well. A device that exists on two busses
>> has to have two sockets that can accept the plugs from the busses
>> they're joining.
>>
>> It's pretty much exactly how it works in real life.
>
> You could just as well say that real life works like this:
>
> class PciSocket {
> PciBus *pci_bus;
> uint8_t *config;
> uint8_t *cmask;
> uint8_t *wmask;
> uint8_t *w1cmask;
> uint8_t *used;
> uint32_t devfn;
> char name[64];
> PCIIORegion io_regions[PCI_NUM_REGIONS];
> ...
>
> };
>
> class IsaSocket {
> IsaBus *isa_bus;
> uint32_t isairq[2]; // not sure this is a good idea, just
> int nirqs; // mimicking qdev
> uint16_t ioports[32];// statically assigned unlike PCI bars
> int nioports;
> }
>
> class MyWeirdDevice : public MyBaseDevice {
> PciSocket pci;
> IsaSocket isa;
> };

Hrm, I'm not sure I buy that entirely.  I think it would be:

class MyWeirdPciView : public PciDevice {
   PciBus *bus;
   MyWeirdDevice *dev;
};

class MyWeirdIsaView : public IsaDevice {
   IsaBus *bus;
   MyWeirdDevice *dev;
};

class MyWeirdDevice : public MyBaseDevice {
   MyWeirdPciView pci;
   MyWeirdIsaView isa;
}

The views are basically bridges between PCI/ISA and an internal 
interface (MyWeirdDevice).

I don't think having a generic PciSocket class that offloads PCI 
knowledge to another device is the right model (assuming that's why 
you're suggesting).  It's basically proxying PCI to another device.

>>> Once you have something like this for a device that bridges two buses,
>>> interfaces require a lot of boilerplate for useless getters/setters.
>>
>> Can you elaborate?
>
> If you store data (configuration space etc.) in the device, and the bus
> has to access it, you need getters/setters in the interface. Letting the
> bus hold an interior reference to the PciSocket (perhaps adding a single
> get_device_for_socket function to the PciSocketOps) solves the problem.

I don't think the proxy design pattern is the right thing to use.  95% 
of the time, the device is intrinsically a PCI device.  The other 5% of 
the time, the device has a well defined interface, and then there is 
effectively a PCI bridge.  But that PCI bridge isn't generic, it's 
specific to that custom interface.

>>>> The same applies equally to IDE.
>>>>
>>>> ide->primary.master = disk1;
>>>> ide->secondary.master = cdrom;
>>>
>>> For IDE, an equally good model would be:
>>>
>>> ide->primary.add(disk1);
>>> disk1.masterSlave = MASTER;
>>> ide->secondary.add(cdrom);
>>> cdrom.masterSlave = MASTER;
>>
>> There's a pin in the IDE cable that determines master or slave depending
>> on whether it's raised high.
>
> Yes, that's the "newer" way. There used to be jumpers to choose between
> master, slave and cable-select.

That jumper raises the bit on the wire.

>>>> Interfaces are the right way to do this. Getting MI right is fairly
>>>> hard
>>>
>>> But we don't need is-a, we need has-a. Multiple is-a is harder than
>>> single is-a. Multiple has-a does not add any complication.
>>
>> Yeah, that's what plug properties are for :-)
>
> I agree, but at the cost of pointer chasing and making it harder to
> implement get_device_for_socket for buses that need it (in the above
> sketch it can be a simple container_of).

Can we be a bit more concrete as I'm having a hard time following your 
logic.  You're assuming a generic PciSocket class, right?  I think 
that's not the right approach, as an example:

class Rtl8139PciBridge : public PciDevice
{
    Rtl8139 rtldev;
};

class Rtl8139IsaBridge : public IsaDevice
{
    Rtl8139 rtldev;
};

With Avi's new memory API, we'd have:

class Rtl8139 : public Device
{
    MemoryRegion region[2];
    Pin irq;
};

And then the construction code for Rtl8139PciBridge would register the 
regions as bars, and connect the PCI lnk to rtldev.irq.

The ISA code would do something similar.

>>>> I think all of the requirements you've outlined are currently
>>>> handled in
>>>> QOM.
>>>
>>> They more than likely are. The question is whether they're handled in
>>> the most programmer-efficient manner, and whether the advantages of a
>>> single grand unified object model for host and guest devices is worth
>>> the effort.
>>
>> Indeed. I think that it's a no brainer for the backends and that's why
>> I'm starting there.
>
> I don't think it's a no brainer. It's simply much easier, but right now
> it is also a solution in search of a problem (if all you want is dynamic
> creation of character devices, you could do that without a generic
> object model).

And that solves the problem yet again for one layer.  But what about 
block, fsdev, and DisplayState?  We can keep reinventing the wheel over 
and over again in slightly different ways or we can try to do something 
that will work for more things.

We need more commonality in QEMU.  Things are hard to follow because 
every subsystem is an island.

> If starting from a blank slate, I would be much more enthusiastic. But
> all that QEMU does _not_ need is yet another incomplete transition.

This is an oversimplification and an argument that needs to end.

We need lots of new transitions, we need to strive to make things 
better.  But we need to do things in such a way that:

  (1) we have a good idea of what we're going to end up with at the end 
of the day

  (2) there is incremental value being added to QEMU at every step of 
the way

This cannot be done by simply hacking some bits here and there.  It 
requires design, planning, and flag days when appropriate.

If you look at QAPI, we only merged the bits that had value for a 
specific feature (guest agent).  The remaining bits (QMP conversion), 
has been completely, but will be merged all at once.  The mistake we can 
make in an effort like this is introduce a partial conversion that has 
no value in and of itself.

Because there's little incentive to help with partial conversions if 
there isn't an immediate return on investment.  That's why I've stuck 
with focusing on the char layer to start with.  It's value that stands 
on its own.

That's the thing we need to focus on.  The problem with our past 
attempts at making large changes is that we didn't deconstruct it in 
such a way that each phase added clear value.

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/26/2011 09:23 PM, Anthony Liguori wrote:
> On 07/26/2011 01:26 PM, Paolo Bonzini wrote:
>> On 07/26/2011 05:34 PM, Anthony Liguori wrote:
>> You could just as well say that real life works like this:
>>
>> class PciSocket {
>> PciBus *pci_bus;
>> uint8_t *config;
>> uint8_t *cmask;
>> uint8_t *wmask;
>> uint8_t *w1cmask;
>> uint8_t *used;
>> uint32_t devfn;
>> char name[64];
>> PCIIORegion io_regions[PCI_NUM_REGIONS];
>> ...
>>
>> };
>>
>> class IsaSocket {
>> IsaBus *isa_bus;
>> uint32_t isairq[2]; // not sure this is a good idea, just
>> int nirqs; // mimicking qdev
>> uint16_t ioports[32];// statically assigned unlike PCI bars
>> int nioports;
>> }
>>
>> class MyWeirdDevice : public MyBaseDevice {
>> PciSocket pci;
>> IsaSocket isa;
>> };
>
> Hrm, I'm not sure I buy that entirely. I think it would be:
>
> class MyWeirdPciView : public PciDevice {
> PciBus *bus;
> MyWeirdDevice *dev;
> };
>
> class MyWeirdIsaView : public IsaDevice {
> IsaBus *bus;
> MyWeirdDevice *dev;
> };
>
> class MyWeirdDevice : public MyBaseDevice {
> MyWeirdPciView pci;
> MyWeirdIsaView isa;
> }
>
> The views are basically bridges between PCI/ISA and an internal
> interface (MyWeirdDevice).

That's yet another possibility.  There are two difference with what I 
had written:

1) the back-pointer from MyWeird*View to MyWeirdDevice replaced by 
container_of.  That's cosmetic;

2) I'm not considering the sockets to be separate devices.  That's the 
difference between "proxying PCI to another device" (wrong, hardware 
does not do that) and "isolating PCI knowledge within a sub-object of 
the device" (just an encapsulation choice, hardware is irrelevant).

> I don't think the proxy design pattern is the right thing to use.
> 95% of the time, the device is intrinsically a PCI device.

It's not a proxy.  It's replacing inheritance with containment to get 
the flexibility of data MI without the complexity of diamond hierarchies.

> The other 5% of the
> time, the device has a well defined interface, and then there is
> effectively a PCI bridge. But that PCI bridge isn't generic, it's
> specific to that custom interface.

Yes, that can be represented by composition if you wish (an 
ISASerialState containing an 8250 is the canonical example; the 8139 
example below is another).

>>> There's a pin in the IDE cable that determines master or slave depending
>>> on whether it's raised high.
>>
>> Yes, that's the "newer" way. There used to be jumpers to choose between
>> master, slave and cable-select.
>
> That jumper raises the bit on the wire.

Ah ok, I was confusing it with the cable-select wire.  My point was that 
you can choose the jumper representation (low-level) or the cable-select 
representation (high-level, actually matches modern hardware, even 
better from the user POV).  One is easier to manage and better in all 
aspects, but both make sense.  It's irrelevant to this discussion anyway.

>>>>> Interfaces are the right way to do this. Getting MI right is fairly
>>>>> hard
>>>>
>>>> But we don't need is-a, we need has-a. Multiple is-a is harder than
>>>> single is-a. Multiple has-a does not add any complication.
>>>
>>> Yeah, that's what plug properties are for :-)
>>
>> I agree, but at the cost of pointer chasing and making it harder to
>> implement get_device_for_socket for buses that need it (in the above
>> sketch it can be a simple container_of).
>
> Can we be a bit more concrete as I'm having a hard time following your
> logic. You're assuming a generic PciSocket class, right? I think that's
> not the right approach, as an example:
>
> class Rtl8139PciBridge : public PciDevice
> {
> Rtl8139 rtldev;
> };
>
> class Rtl8139IsaBridge : public IsaDevice
> {
> Rtl8139 rtldev;
> };
>
> With Avi's new memory API, we'd have:
>
> class Rtl8139 : public Device
> {
> MemoryRegion region[2];
> Pin irq;
> };
>
> And then the construction code for Rtl8139PciBridge would register the
> regions as bars, and connect the PCI lnk to rtldev.irq.
>
> The ISA code would do something similar.

Yes, this looks nice (modulo s/Rtl8139/Rtl8139 */).  But it is not that 
much more flexible than qdev 1.0.

You're right that for the case of two parents above we were looking at a 
contrived example.  The Goldfish platform provides a more interesting 
one.  There you have an interrupt controller and a bus enumerator 
device.  Most devices are connected to both, but conflating them is 
wrong for many reasons;

1) the bus enumerator itself uses an interrupt (raised on hotplug);

2) you can enumerate devices that do not have an interrupt line, and you 
shouldn't need to connect such a device to an interrupt controller;

3) the interrupt controller and bus enumerator use two separate MMIO areas;

4) in principle, other devices could use the interrupt controller (which 
is the only component connected to the CPU's interrupt line) without 
being enumerated.

5) A device with two interrupts has two "interrupt interfaces" and only 
one "enumerator interface".

6) The enumerator interface does not have bus semantics.  The enumerator 
also enumerates itself so it would act as both the bus host and a device 
on the bus.

Composition then lets you use something like this:

     class GoldfishPIC : Device {
         Pin parent;
         GoldfishInterruptPin *children[32];
         Pin (*init_socket_for_irq_num) (GoldfishInterruptPin *, int);
     };

     class GoldfishInterruptPin {
         GoldfishPIC *pic;
         Pin irqnum;
     };

     class GoldfishEnumerator : Device {
         GoldfishInterruptPin        irq;
         GoldfishBusDeviceInfo       info;
         List<GoldfishBusDeviceInfo> allDevices;
         ...
     };

     class GoldfishBusDeviceInfo {
         GoldfishEnumerator *parent;
         char *name;
         int id;
         ram_addr_t mmio;
         int mmio_size;
         int irq_base;
         int irq_count;
     };

     class GoldfishTTY : Device {
         GoldfishInterruptPin irq;
         GoldfishBusDeviceInfo info;
         CharDev *chardev;
     };

Here you do not need a bus to mediate the access between the TTY device 
and its parents, it would only introduce unnecessary complication 
(boilerplate and pointer chasing).  It also would not match hardware at 
all in the case of the enumerator.

>>> Indeed. I think that it's a no brainer for the backends and that's why
>>> I'm starting there.
>>
>> I don't think it's a no brainer. It's simply much easier, but right now
>> it is also a solution in search of a problem (if all you want is dynamic
>> creation of character devices, you could do that without a generic
>> object model).
>
> And that solves the problem yet again for one layer. But what about
> block, fsdev, and DisplayState?  We can keep reinventing the wheel over
> and over again in slightly different ways or we can try to do something
> that will work for more things.
>
> We need more commonality in QEMU. Things are hard to follow because
> every subsystem is an island.

I am not saying you're wrong.  I am saying it is very hard to do it on a 
mature (no matter how messy) code base.

> We need lots of new transitions, we need to strive to make things
> better. But we need to do things in such a way that:
>
> (1) we have a good idea of what we're going to end up with at the end of
> the day
>
> (2) there is incremental value being added to QEMU at every step of the way
>
> This cannot be done by simply hacking some bits here and there. It
> requires design, planning, and flag days when appropriate.

Agreed.  The problem I have with QOM is (2).  I am not sure we do get 
incremental value at every step of the way.  We do get incremental value 
in the char layer, but we also get additional complexity until the 
transition is over.

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/27/2011 03:55 AM, Paolo Bonzini wrote:
> Yes, this looks nice (modulo s/Rtl8139/Rtl8139 */). But it is not that
> much more flexible than qdev 1.0.
>
> You're right that for the case of two parents above we were looking at a
> contrived example. The Goldfish platform provides a more interesting
> one. There you have an interrupt controller and a bus enumerator device.
> Most devices are connected to both, but conflating them is wrong for
> many reasons;
>
> 1) the bus enumerator itself uses an interrupt (raised on hotplug);
>
> 2) you can enumerate devices that do not have an interrupt line, and you
> shouldn't need to connect such a device to an interrupt controller;
>
> 3) the interrupt controller and bus enumerator use two separate MMIO areas;
>
> 4) in principle, other devices could use the interrupt controller (which
> is the only component connected to the CPU's interrupt line) without
> being enumerated.
>
> 5) A device with two interrupts has two "interrupt interfaces" and only
> one "enumerator interface".
>
> 6) The enumerator interface does not have bus semantics. The enumerator
> also enumerates itself so it would act as both the bus host and a device
> on the bus.

I think I understand what your saying here.  But interrupt routing is an 
interesting problem and I've been thinking about it differently then we 
do it today.

The core is:

class Pin : public Device {
    bool level;
};

You can connect a signal to the level to detect edge changes.  You can 
also connect two pins together such that if one raises high, the other 
raises high.

An interrupt controller looks like:

struct InterruptController
{
    Pin *irq[256];
};

In the simple case, you have:

struct UART : public Device
{
    Pin irq;
};

And then you'd set the irq by doing:

apic = new InterruptController()
uart = new UART()

apic->irq[0] = &uart->irq;

Or at the qmp layer:

(qemu) plug_get uart irq
uart::irq
(qemu) plug_set apic irq[0] uart::irq

I mention this because I don't think your example below assumes a model 
like this.

> Composition then lets you use something like this:
>
> class GoldfishPIC : Device {
> Pin parent;
> GoldfishInterruptPin *children[32];
> Pin (*init_socket_for_irq_num) (GoldfishInterruptPin *, int);
> };

So the idea here is that the PIC will multiplex a bunch of interrupts 
into a single line?  I would do this simply as:

class GoldfishPIC : Device {
    Pin out;
    Pin *in[32];
};

The IRQ number is implicit in the socket index, no?  I'm also not sure 
that there's a strong need to have a typed Pin.

>
> class GoldfishInterruptPin {
> GoldfishPIC *pic;
> Pin irqnum;
> };
>
> class GoldfishEnumerator : Device {
> GoldfishInterruptPin irq;
> GoldfishBusDeviceInfo info;
> List<GoldfishBusDeviceInfo> allDevices;
> ...
> };
>
> class GoldfishBusDeviceInfo {
> GoldfishEnumerator *parent;
> char *name;
> int id;
> ram_addr_t mmio;
> int mmio_size;
> int irq_base;
> int irq_count;
> };

Is the enumerator something that has an interface to devices where the 
devices hold this info?  Or is the enumerator just a bank of flash 
that's preprogrammed with fixed info?

If it's the later, I would suggest that we model is in that fashion.  No 
need to teach every single device about the enumerator if they wouldn't 
normally have information about it.

>> We need lots of new transitions, we need to strive to make things
>> better. But we need to do things in such a way that:
>>
>> (1) we have a good idea of what we're going to end up with at the end of
>> the day
>>
>> (2) there is incremental value being added to QEMU at every step of
>> the way
>>
>> This cannot be done by simply hacking some bits here and there. It
>> requires design, planning, and flag days when appropriate.
>
> Agreed. The problem I have with QOM is (2). I am not sure we do get
> incremental value at every step of the way. We do get incremental value
> in the char layer, but we also get additional complexity until the
> transition is over.

So roughly speaking, my plan is:

1) Char layer
  - we get dynamic add of CDS, which enables hot plug of virtio-serial
  - we get ability to change CDS properties dynamically

2) Block layer
  - we get -blockdev

3) Display layer
  - we get dynamic add of VGA devices

4) fsdev
  - dynamic add of virtio-9p devices

5) network layer
  - no new features, but better commonality

At each phase, we also get significantly better modularity.  The block 
layer is already good here, but the other backends aren't.

My only real concern is how to attack the device layer incrementally.  I 
don't think it's impossible but it requires careful thought.

I think we can probably retrofit DeviceState as a QOM base class and 
just systematically convert the types over to QOM.  The next step would 
be to change the property registration to be QOM-like.  I think they we 
could probably push out a lot of what's in DeviceState today into 
another base class, then introduce a better Device base class.  Since a 
lot of subsystems should just inherit from Device, that gives us a nice 
way to attack things one subsystem at a time.

I think an approach like this can have incremental value.  The first 
step of retrofitting is pretty systematic and allows for devices be 
created and composed through the plug interfaces.  I think we could 
pretty much eliminate machines after this phase which would be a huge 
win for compatibility.

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/27/2011 02:48 PM, Anthony Liguori wrote:
>
> So the idea here is that the PIC will multiplex a bunch of interrupts
>  into a single line?

Yes, but the device needs to know the interrupt number so it can expose
it through the enumerator interface.  So the configuration cannot be simply

    pic->irq[n] = tty->irq;

Logically, it's more similar to the ISA case, but I doubt the PIC 
distributes all interrupts to everyone in real hardware.

> Is the enumerator something that has an interface to devices where
> the devices hold this info?  Or is the enumerator just a bank of
> flash that's preprogrammed with fixed info?

The former, at least in theory.  Not sure if it also works that way in
real hardware, but that's the model it exposes and the way the Android
guys implemented it.

The device model provides hotplug support, so it is definitely not just 
flash.  Not sure again if this support is used in the hardware.

> At each phase, we also get significantly better modularity.

Fine, but when do we decide it's good enough to merge it?  And what if 
it turns out that it's not suitable for devices?  We unified some 
things, but we also dug ourselves in NIH when we could have used 
GObject.  (GObject definitely does not work for devices, but at least we 
don't need to write the infrastructure).

> My only real concern is how to attack the device layer incrementally.
> I don't think it's impossible but it requires careful thought.

No idea here, honestly. :)

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/27/2011 10:33 AM, Paolo Bonzini wrote:
> On 07/27/2011 02:48 PM, Anthony Liguori wrote:
>>
>> So the idea here is that the PIC will multiplex a bunch of interrupts
>> into a single line?
>
> Yes, but the device needs to know the interrupt number so it can expose
> it through the enumerator interface. So the configuration cannot be simply
>
> pic->irq[n] = tty->irq;
>
> Logically, it's more similar to the ISA case, but I doubt the PIC
> distributes all interrupts to everyone in real hardware.
>
>> Is the enumerator something that has an interface to devices where
>> the devices hold this info? Or is the enumerator just a bank of
>> flash that's preprogrammed with fixed info?
>
> The former, at least in theory. Not sure if it also works that way in
> real hardware, but that's the model it exposes and the way the Android
> guys implemented it.
>
> The device model provides hotplug support, so it is definitely not just
> flash. Not sure again if this support is used in the hardware.

Sounds like I need to read a little about how this enumerator works.  I 
can't see how this would all operate without the enumerating being some 
form of a bus.

>
>> At each phase, we also get significantly better modularity.
>
> Fine, but when do we decide it's good enough to merge it?

I think we should evaluate the complexity vs. value trade off with the 
character device layer (when fully converted) and make the decision in a 
vacuum.

If the complexity seems too high, I can try to also convert the block 
layer and we can reevaluate.  I believe that just with the character 
device layer, it's a net win and I don't think it can be dramatically 
simplified.  The patches are actually not a lot of code.  The only 
complexity is conceptual and that's because it takes into account a lot 
of different problems.

I can even pair things down a bit by removing support for Interfaces and 
simplifying class initialization of need be for the first merge.

> And what if it
> turns out that it's not suitable for devices? We unified some things,
> but we also dug ourselves in NIH when we could have used GObject.
> (GObject definitely does not work for devices, but at least we don't
> need to write the infrastructure).

I tried to use GObject btw, I can share the results with you if you'd 
like.  Even with backends, I couldn't make properties work.  GObject 
uses GValues for properties which roughly models immutable values in a 
variant.  But I couldn't find a reasonable way to express Plugs and 
Sockets in terms of GValues.

I expect that at some point in the future, GObject will grow GVariant 
properties.  But I still think GVariant isn't quite what it needs to be 
since it still assumes immutable variants that can be copied.

I thought about just using GType but I thought using GType without using 
GObject was probably not a great long term plan as I doubt anyone else 
does this.

Regards,

Anthony Liguori

>
>> My only real concern is how to attack the device layer incrementally.
>> I don't think it's impossible but it requires careful thought.
>
> No idea here, honestly. :)
>
> Paolo
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/27/2011 10:33 AM, Paolo Bonzini wrote:
> On 07/27/2011 02:48 PM, Anthony Liguori wrote:
>>
>> So the idea here is that the PIC will multiplex a bunch of interrupts
>> into a single line?
>
> Yes, but the device needs to know the interrupt number so it can expose
> it through the enumerator interface. So the configuration cannot be simply
>
> pic->irq[n] = tty->irq;
>
> Logically, it's more similar to the ISA case, but I doubt the PIC
> distributes all interrupts to everyone in real hardware.
>
>> Is the enumerator something that has an interface to devices where
>> the devices hold this info? Or is the enumerator just a bank of
>> flash that's preprogrammed with fixed info?
>
> The former, at least in theory. Not sure if it also works that way in
> real hardware, but that's the model it exposes and the way the Android
> guys implemented it.

I can't really find what you're describing.  I think all the specs are 
on http://www.milkymist.org/mmsoc.html

It's seems like there are a couple different kinds of busses, and that 
there is a CSR bus that is used to access configuration information but 
I believe only for CSR devices (which are low speed).

Can you point me towards the current code for this?

Regards,

Anthony Liguori

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/27/2011 06:28 PM, Anthony Liguori wrote:
> On 07/27/2011 10:33 AM, Paolo Bonzini wrote:
>> On 07/27/2011 02:48 PM, Anthony Liguori wrote:
>>>
>>> So the idea here is that the PIC will multiplex a bunch of interrupts
>>> into a single line?
>>
>> Yes, but the device needs to know the interrupt number so it can expose
>> it through the enumerator interface. So the configuration cannot be
>> simply
>>
>> pic->irq[n] = tty->irq;
>>
>> Logically, it's more similar to the ISA case, but I doubt the PIC
>> distributes all interrupts to everyone in real hardware.
>>
>>> Is the enumerator something that has an interface to devices where
>>> the devices hold this info? Or is the enumerator just a bank of
>>> flash that's preprogrammed with fixed info?
>>
>> The former, at least in theory. Not sure if it also works that way in
>> real hardware, but that's the model it exposes and the way the Android
>> guys implemented it.
>
> I can't really find what you're describing. I think all the specs are on
> http://www.milkymist.org/mmsoc.html

That's milkymist, not GoldFish.

You can see the code at 
https://github.com/patricksjackson/qemu/blob/android/hw/goldfish_device.c (see 
also 
https://github.com/patricksjackson/qemu/blob/android/hw/goldfish_device.h for 
the structs composing the list).

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/27/2011 01:51 PM, Paolo Bonzini wrote:
> On 07/27/2011 06:28 PM, Anthony Liguori wrote:
>> On 07/27/2011 10:33 AM, Paolo Bonzini wrote:
>>> On 07/27/2011 02:48 PM, Anthony Liguori wrote:
>>>>
>>>> So the idea here is that the PIC will multiplex a bunch of interrupts
>>>> into a single line?
>>>
>>> Yes, but the device needs to know the interrupt number so it can expose
>>> it through the enumerator interface. So the configuration cannot be
>>> simply
>>>
>>> pic->irq[n] = tty->irq;
>>>
>>> Logically, it's more similar to the ISA case, but I doubt the PIC
>>> distributes all interrupts to everyone in real hardware.
>>>
>>>> Is the enumerator something that has an interface to devices where
>>>> the devices hold this info? Or is the enumerator just a bank of
>>>> flash that's preprogrammed with fixed info?
>>>
>>> The former, at least in theory. Not sure if it also works that way in
>>> real hardware, but that's the model it exposes and the way the Android
>>> guys implemented it.
>>
>> I can't really find what you're describing. I think all the specs are on
>> http://www.milkymist.org/mmsoc.html
>
> That's milkymist, not GoldFish.

Oh, Goldfish is fake. It's not real hardware.

The enumerator device is not a real device.  It's weird because it's 
imaginary and was designed specifically within QEMU.

It's not a good example for discussing modelling.

Regards,

Anthony Liguori

>
> You can see the code at
> https://github.com/patricksjackson/qemu/blob/android/hw/goldfish_device.c (see
> also
> https://github.com/patricksjackson/qemu/blob/android/hw/goldfish_device.h for
> the structs composing the list).
>
> Paolo
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/27/2011 10:01 PM, Anthony Liguori wrote:
>>>
>>
>> That's milkymist, not GoldFish.
>
> Oh, Goldfish is fake. It's not real hardware.
>
> The enumerator device is not a real device.  It's weird because it's
> imaginary and was designed specifically within QEMU.
>
> It's not a good example for discussing modelling.

There are plenty of PV interfaces implemented by QEMU.  Would you say 
the same of virtio?

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/28/2011 02:36 AM, Paolo Bonzini wrote:
> On 07/27/2011 10:01 PM, Anthony Liguori wrote:
>>>>
>>>
>>> That's milkymist, not GoldFish.
>>
>> Oh, Goldfish is fake. It's not real hardware.
>>
>> The enumerator device is not a real device. It's weird because it's
>> imaginary and was designed specifically within QEMU.
>>
>> It's not a good example for discussing modelling.
>
> There are plenty of PV interfaces implemented by QEMU. Would you say the
> same of virtio?

Virtio was designed to look like real hardware.

I would say that trying to fit XenStore into QOM would not be a good 
exercise.

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/28/2011 02:46 PM, Anthony Liguori wrote:
>>
>> There are plenty of PV interfaces implemented by QEMU. Would you say the
>> same of virtio?
>
> Virtio was designed to look like real hardware.
>
> I would say that trying to fit XenStore into QOM would not be a good
> exercise.

No doubt about that. :)  I'd put a lot more hope into Goldfish though.

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/28/2011 08:50 AM, Paolo Bonzini wrote:
> On 07/28/2011 02:46 PM, Anthony Liguori wrote:
>>>
>>> There are plenty of PV interfaces implemented by QEMU. Would you say the
>>> same of virtio?
>>
>> Virtio was designed to look like real hardware.
>>
>> I would say that trying to fit XenStore into QOM would not be a good
>> exercise.
>
> No doubt about that. :) I'd put a lot more hope into Goldfish though.

What's unclear to me about the Goldfish enumerator is whether it should 
be filled out through interaction with hardware devices or via some 
other mechanism.

In many ways, it's similar to ACPI and a Device Tree.  In both of those 
cases, firmware actually is responsible for constructing those tables.

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/28/2011 04:03 PM, Anthony Liguori wrote:
>> No doubt about that. :) I'd put a lot more hope into Goldfish though.
>
> What's unclear to me about the Goldfish enumerator is whether it should
> be filled out through interaction with hardware devices or via some
> other mechanism.
>
> In many ways, it's similar to ACPI and a Device Tree.  In both of those
> cases, firmware actually is responsible for constructing those tables.

Yes, it is a flat device tree.

Since it supports hotplug (at least in theory, the Android emulator 
predates qdev so it doesn't support it), I would say it is more similar 
to PCI configuration space.  The difference is that IRQ numbers and MMIO 
base addresses are handed out by hardware (by a piece of the SoC) rather 
than by the firmware.

So yes, the hardware would have some kind of bus to talk to the devices 
and arbitrate hotplug/hotunplug.  The only peculiarity being that the 
bus enumerator hardcodes itself in the list it exposes, in addition to 
the devices on the bus.

But that still means that the devices have two views:

1) the enumerator's view is either "this is my name, my MMIO base, my 
IRQ base" or "I need 4k of MMIO and 1 IRQ line, please tell me 
where/which are those", depending on the device;

2) the PIC's view is "please bring this IRQ line up/down" (the device 
says which line, since the enumerator can assign those dynamically).

The PIC's view is more complicated than a Pin, and more similar to ISA.

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/28/2011 09:41 AM, Paolo Bonzini wrote:
> On 07/28/2011 04:03 PM, Anthony Liguori wrote:
>>> No doubt about that. :) I'd put a lot more hope into Goldfish though.
>>
>> What's unclear to me about the Goldfish enumerator is whether it should
>> be filled out through interaction with hardware devices or via some
>> other mechanism.
>>
>> In many ways, it's similar to ACPI and a Device Tree. In both of those
>> cases, firmware actually is responsible for constructing those tables.
>
> Yes, it is a flat device tree.
>
> Since it supports hotplug (at least in theory, the Android emulator
> predates qdev so it doesn't support it), I would say it is more similar
> to PCI configuration space. The difference is that IRQ numbers and MMIO
> base addresses are handed out by hardware (by a piece of the SoC) rather
> than by the firmware.
>
> So yes, the hardware would have some kind of bus to talk to the devices
> and arbitrate hotplug/hotunplug. The only peculiarity being that the bus
> enumerator hardcodes itself in the list it exposes, in addition to the
> devices on the bus.
>
> But that still means that the devices have two views:
>
> 1) the enumerator's view is either "this is my name, my MMIO base, my
> IRQ base" or "I need 4k of MMIO and 1 IRQ line, please tell me
> where/which are those", depending on the device;

I think it's important to ask, how would this be implemented in 
hardware.  The only way I can see is to teach each device about this 
interface and then have a common bus.  That implies that you have:

class GoldfishEnumerator : public Device {
      GoldfishDevice *slots[N];
};

interface GoldfishDevice {
      const char *get_name();
      uint64_t get_mmio_base();
      ...
};

class GoldfishNic : public Device, implements GoldfishDevice
{
     const char *get_name(void) {
         return "nic";
     }
};

With respect to hotplug, that means that you have to hot plug the device 
to multiple busses.

> 2) the PIC's view is "please bring this IRQ line up/down" (the device
> says which line, since the enumerator can assign those dynamically).
>
> The PIC's view is more complicated than a Pin, and more similar to ISA.

ISA is just a pin.  The ISA bus extender literally has five pins 
corresponding to the ISA IRQs 7, 6, 5, 4, 3.

EISA adds 5 more pins for 10, 11, 12, 14, 15.

ISA devices "choose" their IRQ line by hardwiring their IRQ output pin 
to a specific IRQ line on the bus.

Regards,

Anthony Liguori

>
> Paolo
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/28/2011 05:04 PM, Anthony Liguori wrote:
> The only way I can see is to teach each device about this interface and
> then have a common bus.  That implies that you have:
>
> class GoldfishEnumerator : public Device {
>       GoldfishDevice *slots[N];

FWIW, there's no hardcoded limit in the interface, and the list of 
devices is unordered.  But that only means you should attach it with

     plug-set goldfish_tty::enumerator goldfish_enum

rather than

     plug-set goldfish_enum::slots[12] goldfish_tty

If you can confirm that, that's fine.

> };
>
> interface GoldfishDevice {
>       const char *get_name();
>       uint64_t get_mmio_base();
>       ...
> };
>
> class GoldfishNic : public Device, implements GoldfishDevice
> {
>      const char *get_name(void) {
>          return "nic";
>      }

      uint64_t mmio_base;
      uint64_t get_mmio_base() { return mmio_base; }
      uint64_t set_mmio_base(uint64_t addr) { mmio_base = addr; }

> };

And that's exactly my point.  It's a "stupid" interface full of 
getters/setters, which is what you get if you use only interface 
inheritance instead of, where appropriate, data containment.

Interfaces should be reserved for what really depends on the 
_implementation_ of the GoldfishNic, not for accessing a bunch of 
numbers.  There is no implementation-dependent detail of that kind in 
the GoldfishDevice (unlike other buses, even simple ones like I2C).

>> The PIC's view is more complicated than a Pin, and more similar to ISA.
>
> ISA is just a pin.  The ISA bus extender literally has five pins
> corresponding to the ISA IRQs 7, 6, 5, 4, 3.

ISA is many pins. :)  Goldfish looks similar (32 pins).

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/28/2011 10:47 AM, Paolo Bonzini wrote:
> On 07/28/2011 05:04 PM, Anthony Liguori wrote:
>> The only way I can see is to teach each device about this interface and
>> then have a common bus. That implies that you have:
>>
>> class GoldfishEnumerator : public Device {
>> GoldfishDevice *slots[N];
>
> FWIW, there's no hardcoded limit in the interface, and the list of
> devices is unordered. But that only means you should attach it with
>
> plug-set goldfish_tty::enumerator goldfish_enum
>
> rather than
>
> plug-set goldfish_enum::slots[12] goldfish_tty
>
> If you can confirm that, that's fine.

Yes, you can certainly have an enumerator socket that when set, 
automatically connects the appropriate properties in the enumerator.

That way you don't have to connect things by hand.

>> };
>>
>> interface GoldfishDevice {
>> const char *get_name();
>> uint64_t get_mmio_base();
>> ...
>> };
>>
>> class GoldfishNic : public Device, implements GoldfishDevice
>> {
>> const char *get_name(void) {
>> return "nic";
>> }
>
> uint64_t mmio_base;
> uint64_t get_mmio_base() { return mmio_base; }
> uint64_t set_mmio_base(uint64_t addr) { mmio_base = addr; }
>
>> };
>
> And that's exactly my point. It's a "stupid" interface full of
> getters/setters, which is what you get if you use only interface
> inheritance instead of, where appropriate, data containment.
>

You can certainly do:

struct GoldfishEnumInfo
{
     const char *name;
     uint64_t mmio_base;
};

interface GoldfishDevice {
     GoldfishEnumInfo *get_info();
}

And then:

GoldfishEnumInfo *goldfish_nic_get_info(GoldFishNic *nic)
{
     return nic->enuminfo;
}

> Interfaces should be reserved for what really depends on the
> _implementation_ of the GoldfishNic, not for accessing a bunch of
> numbers.

Just define an interface that returns a struct then.  It's no more 
complicated than that.

What I struggle with is whether you're suggesting that the info isn't 
part of the device or whether it is part of the device and you just 
think that we shouldn't need 10 different accessors.

> There is no implementation-dependent detail of that kind in the
> GoldfishDevice (unlike other buses, even simple ones like I2C).
>
>>> The PIC's view is more complicated than a Pin, and more similar to ISA.
>>
>> ISA is just a pin. The ISA bus extender literally has five pins
>> corresponding to the ISA IRQs 7, 6, 5, 4, 3.
>
> ISA is many pins. :) Goldfish looks similar (32 pins).

Sorry, I meant to say ISA IRQs are just exposed as pins.

My real point is, doing:

class IsaDevice : public Device {
    Pin irq[16];
};

class MyIsaDevice : public IsaDevice {
    int irq_index;
};

And then doing:

my_isa_device->irq_index = 5;

Is a very good way to model ISA IRQ selection.

You can simplify it by having a single IRQ output for each ISA device 
instead of any possible IRQ and then route the IRQ as part of the bus 
connection.

Regards,

Anthony Liguori

> Paolo
>

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Paolo Bonzini]

On 07/28/2011 07:59 PM, Anthony Liguori wrote:
> Just define an interface that returns a struct then.  It's no more
> complicated than that.

Ok, so we're debating whether to: 1) add an interface returning a 
pointer to an internal struct; 2) include in the internal struct a 
pointer-to-function that does a container_of and returns the outer 
Device object.  Otherwise, we're on the same page.  I'm quite relieved. ;)

I can see advantages to both approach.  The main advantage to (2) is 
that it scales better when you have multiple interfaces of the same kind 
exposed by the device.  You cannot implement an interface twice.

Paolo

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Peter Maydell]

On 26 July 2011 15:02, Anthony Liguori <anthony@codemonkey.ws> wrote:
> In my attempt at PCI modelling, I had something like:
>
> struct I440FX
> {
>   Device parent;
>
>   PciDevice slots[32];
> };
>
> Which means that to attach to bus 0, slot 3, fn 0, you do:
>
> i440fx->slots[3] = mydevice

So what I don't really understand about this model is why the PCI
connection is "privileged" in the sense that 'mydevice' is actually
an instance of something you can stick directly into one of the PCIBus
device's slots. I would expect that 'mydevice' would have a bunch of
(effectively) interfaces it exposed, and you'd connect i440fx->slots[3]
to mydevice.pciconnector or something. [Which ought to both let the
PCIBus call functions implemented by the device, and vice versa.]

(What would a model of a two-PCI-slot graphics card look like?)

Maybe it would be better to use some other example. After all, the
cases like PCI are the ones our device model already handles pretty
well, so the interesting cases to look at are where we're not so
good at it. What does the implementation of omap2_gpio look like,
for instance? (it's not qdev at the moment but there's a set of
patches on the list which make it so; the guts of the qdevification
are http://patchwork.ozlabs.org/patch/103905/ .)
 That's a device which exposes:
 * 4,5, or 6 MMIO regions (depending on how many 'modules' the
   particular revision you ask for has)
 * a variably sized array of gpio inputs
 * ditto, gpio outputs
 * for each 'module', three specific named outgoing gpio lines
   "mpu_irq", "dsp_irq" and "wakeup"
 * a number of omap_clk connections, where an omap_clk* represents
   a connection to the OMAP clock tree, and in practice is an
   interface where the omap_gpio can (a) call a function to
   ask "what rate is this clock running at?" and (b) provide
   a gpio line which will be raised when the clock ticks.
   [omap_gpio doesn't actually use its clock connections but
   other omap devices do; they're interesting because we have
   function calls going in both directions over the interface.]

None of these exposed interfaces are particularly obviously
more important than any of the others -- they're just all
things that might need to be wired up.

I'm particularly interested in how much effort is involved
in defining ad-hoc platform-specific interface types like
omap_clk.

-- PMM

Re: [Qemu-devel] [RFC][PATCH 0/21] QEMU Object Model

[Anthony Liguori]

On 07/27/2011 04:33 PM, Peter Maydell wrote:
> On 26 July 2011 15:02, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> In my attempt at PCI modelling, I had something like:
>>
>> struct I440FX
>> {
>>    Device parent;
>>
>>    PciDevice slots[32];
>> };
>>
>> Which means that to attach to bus 0, slot 3, fn 0, you do:
>>
>> i440fx->slots[3] = mydevice
>
> So what I don't really understand about this model is why the PCI
> connection is "privileged" in the sense that 'mydevice' is actually
> an instance of something you can stick directly into one of the PCIBus
> device's slots.


Oh, it's not privileged at all.  It just happens to be that modelling 
something simple is simple.  That's goodness.

The example that I used for most of my early testing was meant to be a 
bit more interesting.  I created a Source, Sink, XorGate, AndGate, and 
OrGate and then used a config file to build a full working 8-bit adder.

You can see this at:

http://repo.or.cz/w/qemu/aliguori.git/blob/refs/heads/qdev2

Check out devices/gates and vm.cfg.

Here's a rough model of how it works:

struct XorGate
{
    Device parent;
    Pin out;
    Pin *in[2];
};

In XorGate's initfn, it registers three properties, 'in[0]' and 'in[1]' 
as 'socket<Pin>'s and 'out' as 'plug<Pin>'.

When XorGate is realized, it makes sure that both in[0] and in[1] have 
been set (sockets are RW whereas plugs are RO).  It then connects to the 
edge event on the in[0] and in[1] pins and when either event fires, it 
will compute a value, and set the level of out.

XorGate is-a Device, but so is Pin.  The following things are valid:

(qemu) plug_create xor-gate gate0
(qemu) plug_create xor-gate gate1
(qemu) plug_create xor-gate gate2
(qemu) plug_get gate1 out
gate1::out
(qemu) plug_set xor-gate in[0] gate1::out
(qemu) plug_get gate2 out
gate2::out
(qemu) plug_set xor-gate in[1] gate2::out

Which creates a simple tree of two xor gates who's outputs are tied to 
another xor gate.  Or:

            XorGate /
- XorGate /        \
           \
            XorGate /
                    \

The 'out' property of XorGate is really a child device.  All devices 
(all plugs for that matter) have a global namespace.  By convention, 
uniqueness in name is guaranteed for child devices by reserving the '::' 
separator and naming children based on 'parentname::propertyname'.  But 
for correctness, the right way to figure out the name of a child is by 
reading the property.

So to answer your above question, a multi PCI slot graphics adapter is 
simply:

struct GraphicsSlot {
    PciDevice parent;
};

struct MultislotGraphicsCard {
    Device parent;

    GraphicsSlot slot[2];
};

Then conceptually:

mgc = new MultislotGraphicsCard();

i440fx->slot[1] = mgc.slot[0];
i440fx->slot[2] = mgc.slot[1];

You do not need to ever assign a device directly to a socket.  This is a 
fundamental different between QOM and qdev.  There is no concept of 
"parent bus".  There's no concept of parent--even with composition. 
Devices can have links to zero or more other Devices.  They can also 
create devices within themselves but those devices don't have any 
special knowledge of that fact.


>  I would expect that 'mydevice' would have a bunch of
> (effectively) interfaces it exposed, and you'd connect i440fx->slots[3]
> to mydevice.pciconnector or something. [Which ought to both let the
> PCIBus call functions implemented by the device, and vice versa.]
>
> (What would a model of a two-PCI-slot graphics card look like?)
>
> Maybe it would be better to use some other example. After all, the
> cases like PCI are the ones our device model already handles pretty
> well, so the interesting cases to look at are where we're not so
> good at it. What does the implementation of omap2_gpio look like,
> for instance? (it's not qdev at the moment but there's a set of
> patches on the list which make it so; the guts of the qdevification
> are http://patchwork.ozlabs.org/patch/103905/ .)
>   That's a device which exposes:
>   * 4,5, or 6 MMIO regions (depending on how many 'modules' the
>     particular revision you ask for has)
>   * a variably sized array of gpio inputs
>   * ditto, gpio outputs

struct OmapGpio
{
     Device parent;

     size_t nb_regions;
     MemoryRegion *regions;

     size_t nb_out;
     Pin *out;

     size_t nb_in;
     Pin **in;
};

void omap_gpio_set_nb_out(OmapGpio *obj, size_t value);
void omap_gpio_set_nb_in(OmapGpio *obj, size_t value);
void omap_gpio_set_nb_regions(OmapGpio *obj, size_t value);

At initfn(), you register three integer properties, 'nb_out', 'nb_in', 
'nb_regions'.

In the respective setters, you allocate the appropriate array and then 
unregister the old properties, and then register teh new ones.  For 
instance:

void omap_gpio_set_nb_out(OmapGpio *obj, size_t value)
{
    /* remove old properties */
    for (i = 0; i < obj->nb_out; i++) {
        plug_del_property_plug(PLUG(obj), "out[%d]", i);
    }

    obj->out = realloc(obj->out, new_size);
    obj->nb_out = value;

    /* add new properties */
    for (i = 0; i < obj->nb_out; i++) {
        plug_add_property_plug(PLUG(obj), &obj->pin[i],
                               TYPE_PIN, "out[%d]", i);
    }
}

This would let you work with the device like this:

[OmapGpio "gpio"]
nb_out = 4
nb_in = 2
in[0] = "foo.out[0]"
in[1] = "foo.out[1]"

[OmapFoo "foo"]
bar = "gpio.out[3]"

It's exactly this use-case which makes GObject unsuitable.  Properties 
need to be associated with the instance, not the class.  qdev also 
suffers from this problem.

Regards,

Anthony Liguori

>   * for each 'module', three specific named outgoing gpio lines
>     "mpu_irq", "dsp_irq" and "wakeup"
>   * a number of omap_clk connections, where an omap_clk* represents
>     a connection to the OMAP clock tree, and in practice is an
>     interface where the omap_gpio can (a) call a function to
>     ask "what rate is this clock running at?" and (b) provide
>     a gpio line which will be raised when the clock ticks.
>     [omap_gpio doesn't actually use its clock connections but
>     other omap devices do; they're interesting because we have
>     function calls going in both directions over the interface.]
>
> None of these exposed interfaces are particularly obviously
> more important than any of the others -- they're just all
> things that might need to be wired up.
>
> I'm particularly interested in how much effort is involved
> in defining ad-hoc platform-specific interface types like
> omap_clk.
>
> -- PMM
>