[Qemu-devel] [PATCH 0/8] CPU DeviceState v10

[Qemu-devel] [PATCH 0/8] CPU DeviceState v10

[Eduardo Habkost]

Changes on v10:
 - Set no_user=1 on CPU class
 - Coding style fixes
 - Sending as PATCH instead of RFC, now

v9:
 - Instead of moving qemu_[un]register_reset() to reset.c and including
   it on *-user, create stubs for them on libqemustub.a
 - This is based on afaerber's qom-cpu branch, that has some header cleanup
   changes. You can get the complete series in a git tree at:
   https://github.com/ehabkost/qemu-hacks/tree/cpu_qdev.v9
   git://github.com/ehabkost/qemu-hacks.git cpu_qdev.v9

v8:
 - Use a simpler copyright header on qdev-properties-system.c
 - Use the new libqemustub.a mechanism instead of the (now exting)
   QEMU_WEAK_ALIAS mechanism
 - Move the reset-handler registration code to a new hw/reset.c file

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

v6:
 - Simple rebase against latest qemu.git master
 - Patch 13: some new typedefs were added and others were removed
 - Patch 19: trivial rebase

v5:
 - Tons of header cleanups just to eliminate qlist.h <-> cpu-common.h circular
   dependency (patches 1-17)
 - Add copyright/license information to qdev-properties.c (patch 17)
 - Add copyright/license information to qdev-properties-system.c (patch 22)
 - use error_report()+abort() instead of hw_error() on qdev.c (patch 18)
 - Move qemu_[un]register_reset() and qemu_devices_reset() to qdev-core.c
   (patch 19)
 - Make vmstate_[un]register() weak stubs, instead of a new function (patch 20)
 - Make sysbus_get_default() weak stub, instead of new qbus reset (un)register
   functions (patch 21)
 - Eliminate qdev-system.c (all code is kept on qdev.c, now) (patch 22)

v4:
  - Add GCC_WEAK_DECL to functions that have GCC_WEAK versions
  - Updated the qdev_init_gpio_in() code on qdev-system.c to current version
  - Patch description updates (moved changelog below "---" and/or move info
    about changes made by different authors between SoB lines)

v3 (submitted by Igor):
  - rebased on top of 8b4a3df (today's master)
  - slight code reshuffling in (see commit's changelog)
     "qdev: separate core from the code used only by qemu-system-*"
     "move qemu_irq typedef out of cpu-common.h"
  - commit messages cleanup

v2:
  Removes the CONFIG_USER_ONLY ifdefs, and use weak symbols to move
  the vmstate and qemu_register_reset() handling to qdev-system.c


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

References to previous versions:
  v9: http://marc.info/?l=qemu-devel&m=135462995431137
  v8: http://article.gmane.org/gmane.comp.emulators.qemu/182589
  v7: http://article.gmane.org/gmane.comp.emulators.qemu/179969
  v6: http://article.gmane.org/gmane.comp.emulators.qemu/179918
  v5: http://article.gmane.org/gmane.comp.emulators.qemu/177426
  v4: http://article.gmane.org/gmane.comp.emulators.qemu/176127
  v3: http://article.gmane.org/gmane.comp.emulators.qemu/175980
  v2: http://article.gmane.org/gmane.comp.emulators.qemu/173909
  v1: http://article.gmane.org/gmane.comp.emulators.qemu/166630


Eduardo Habkost (8):
  Move -I$(SRC_PATH)/include compiler flag to Makefile.objs
  libqemustub: Add qemu_[un]register_reset() stubs
  libqemustub: vmstate register/unregister stubs
  libqemustub: sysbus_get_default() stub
  qdev: Coding style fixes
  qdev-properties.c: Separate core from the code used only by
    qemu-system-*
  include qdev code into *-user, too
  qom: Make CPU a child of DeviceState

 Makefile                    |   1 -
 Makefile.objs               |  23 ++-
 hw/Makefile.objs            |  10 +-
 hw/qdev-properties-system.c | 358 ++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev-properties.c        | 356 +++----------------------------------------
 hw/qdev-properties.h        |   1 +
 hw/qdev.c                   |  13 --
 include/qemu/cpu.h          |   6 +-
 qom/cpu.c                   |   5 +-
 stubs/Makefile.objs         |   3 +
 stubs/reset.c               |  13 ++
 stubs/sysbus.c              |   6 +
 stubs/vmstate.c             |  17 +++
 13 files changed, 454 insertions(+), 358 deletions(-)
 create mode 100644 hw/qdev-properties-system.c
 create mode 100644 stubs/reset.c
 create mode 100644 stubs/sysbus.c
 create mode 100644 stubs/vmstate.c

-- 
1.7.11.7

[Qemu-devel] [PATCH 1/8] Move -I$(SRC_PATH)/include compiler flag to Makefile.objs

[Eduardo Habkost]

The flag is necessary for code that doesn't use the variables from
Makefile (but use Makefile.objs), like libcacard/ and stubs/.

This also moves the existing CFLAGS lines from Makefile.objs at the
beginning of the file, to keep them all in the same place.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 Makefile      |  1 -
 Makefile.objs | 15 +++++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 9ecbcbb..739d9cd 100644
--- a/Makefile
+++ b/Makefile
@@ -145,7 +145,6 @@ audio/audio.o audio/fmodaudio.o: QEMU_CFLAGS += $(FMOD_CFLAGS)
 
 QEMU_CFLAGS+=$(CURL_CFLAGS)
 
-QEMU_CFLAGS += -I$(SRC_PATH)/include
 
 ui/cocoa.o: ui/cocoa.m
 
diff --git a/Makefile.objs b/Makefile.objs
index 3c7abca..0a0a33a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,4 +1,13 @@
 #######################################################################
+# general compiler flags
+
+QEMU_CFLAGS += $(GLIB_CFLAGS)
+QEMU_CFLAGS += -I$(SRC_PATH)/include
+
+vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
+vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
+
+#######################################################################
 # Stub library, linked in tools
 stub-obj-y = stubs/
 
@@ -236,12 +245,6 @@ universal-obj-y += $(qapi-obj-y)
 qga-obj-y = qga/ qemu-ga.o module.o qemu-tool.o
 qga-obj-$(CONFIG_POSIX) += qemu-sockets.o qemu-option.o
 
-vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
-
-vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
-
-QEMU_CFLAGS+=$(GLIB_CFLAGS)
-
 nested-vars += \
 	stub-obj-y \
 	qga-obj-y \
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 1/8] Move -I$(SRC_PATH)/include compiler flag to Makefile.objs

[Andreas Färber]

Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> The flag is necessary for code that doesn't use the variables from
> Makefile (but use Makefile.objs), like libcacard/ and stubs/.
> 
> This also moves the existing CFLAGS lines from Makefile.objs at the
> beginning of the file, to keep them all in the same place.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Waiting for ack or nack from Paolo here. I am expecting some overlap
with his header file reorganization series.

My previous (unanswered?) question was why you are moving vl.o lines in
addition to the QEMU_CFLAGS lines that you mention in the commit message.

Andreas

> ---
>  Makefile      |  1 -
>  Makefile.objs | 15 +++++++++------
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 9ecbcbb..739d9cd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -145,7 +145,6 @@ audio/audio.o audio/fmodaudio.o: QEMU_CFLAGS += $(FMOD_CFLAGS)
>  
>  QEMU_CFLAGS+=$(CURL_CFLAGS)
>  
> -QEMU_CFLAGS += -I$(SRC_PATH)/include
>  
>  ui/cocoa.o: ui/cocoa.m
>  
> diff --git a/Makefile.objs b/Makefile.objs
> index 3c7abca..0a0a33a 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -1,4 +1,13 @@
>  #######################################################################
> +# general compiler flags
> +
> +QEMU_CFLAGS += $(GLIB_CFLAGS)
> +QEMU_CFLAGS += -I$(SRC_PATH)/include
> +
> +vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> +vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
> +
> +#######################################################################
>  # Stub library, linked in tools
>  stub-obj-y = stubs/
>  
> @@ -236,12 +245,6 @@ universal-obj-y += $(qapi-obj-y)
>  qga-obj-y = qga/ qemu-ga.o module.o qemu-tool.o
>  qga-obj-$(CONFIG_POSIX) += qemu-sockets.o qemu-option.o
>  
> -vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> -
> -vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
> -
> -QEMU_CFLAGS+=$(GLIB_CFLAGS)
> -
>  nested-vars += \
>  	stub-obj-y \
>  	qga-obj-y \
> 


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

Re: [Qemu-devel] [PATCH 1/8] Move -I$(SRC_PATH)/include compiler flag to Makefile.objs

[Paolo Bonzini]

Il 14/12/2012 16:34, Andreas Färber ha scritto:
> Am 05.12.2012 17:49, schrieb Eduardo Habkost:
>> The flag is necessary for code that doesn't use the variables from
>> Makefile (but use Makefile.objs), like libcacard/ and stubs/.
>>
>> This also moves the existing CFLAGS lines from Makefile.objs at the
>> beginning of the file, to keep them all in the same place.
>>
>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Waiting for ack or nack from Paolo here. I am expecting some overlap
> with his header file reorganization series.

Yeah, I expect that series to go in early next week.  Waiting for Blue
to pull from Alexander.

Paolo

> My previous (unanswered?) question was why you are moving vl.o lines in
> addition to the QEMU_CFLAGS lines that you mention in the commit message.
> 
> Andreas
> 
>> ---
>>  Makefile      |  1 -
>>  Makefile.objs | 15 +++++++++------
>>  2 files changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 9ecbcbb..739d9cd 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -145,7 +145,6 @@ audio/audio.o audio/fmodaudio.o: QEMU_CFLAGS += $(FMOD_CFLAGS)
>>  
>>  QEMU_CFLAGS+=$(CURL_CFLAGS)
>>  
>> -QEMU_CFLAGS += -I$(SRC_PATH)/include
>>  
>>  ui/cocoa.o: ui/cocoa.m
>>  
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 3c7abca..0a0a33a 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -1,4 +1,13 @@
>>  #######################################################################
>> +# general compiler flags
>> +
>> +QEMU_CFLAGS += $(GLIB_CFLAGS)
>> +QEMU_CFLAGS += -I$(SRC_PATH)/include
>> +
>> +vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>> +vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
>> +
>> +#######################################################################
>>  # Stub library, linked in tools
>>  stub-obj-y = stubs/
>>  
>> @@ -236,12 +245,6 @@ universal-obj-y += $(qapi-obj-y)
>>  qga-obj-y = qga/ qemu-ga.o module.o qemu-tool.o
>>  qga-obj-$(CONFIG_POSIX) += qemu-sockets.o qemu-option.o
>>  
>> -vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>> -
>> -vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
>> -
>> -QEMU_CFLAGS+=$(GLIB_CFLAGS)
>> -
>>  nested-vars += \
>>  	stub-obj-y \
>>  	qga-obj-y \
>>
> 
> 

Re: [Qemu-devel] [PATCH 1/8] Move -I$(SRC_PATH)/include compiler flag to Makefile.objs

[Eduardo Habkost]

On Fri, Dec 14, 2012 at 04:34:29PM +0100, Andreas Färber wrote:
> Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> > The flag is necessary for code that doesn't use the variables from
> > Makefile (but use Makefile.objs), like libcacard/ and stubs/.
> > 
> > This also moves the existing CFLAGS lines from Makefile.objs at the
> > beginning of the file, to keep them all in the same place.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Waiting for ack or nack from Paolo here. I am expecting some overlap
> with his header file reorganization series.
> 
> My previous (unanswered?) question was why you are moving vl.o lines in
> addition to the QEMU_CFLAGS lines that you mention in the commit message.


I thought this note in the commit message would answer the question:

> > This also moves the existing CFLAGS lines from Makefile.objs at the
> > beginning of the file, to keep them all in the same place.

In other words: it's cosmetic, just to keep all the QEMU_CLFAGS lines
inside Makefile.objs grouped in a visible place at the beginning of the
file.

(You noticed that I am moving the vl.o lines _inside_ Makefile.obj,
right? They are not being moved between different files.)

> 
> Andreas
> 
> > ---
> >  Makefile      |  1 -
> >  Makefile.objs | 15 +++++++++------
> >  2 files changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 9ecbcbb..739d9cd 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -145,7 +145,6 @@ audio/audio.o audio/fmodaudio.o: QEMU_CFLAGS += $(FMOD_CFLAGS)
> >  
> >  QEMU_CFLAGS+=$(CURL_CFLAGS)
> >  
> > -QEMU_CFLAGS += -I$(SRC_PATH)/include
> >  
> >  ui/cocoa.o: ui/cocoa.m
> >  
> > diff --git a/Makefile.objs b/Makefile.objs
> > index 3c7abca..0a0a33a 100644
> > --- a/Makefile.objs
> > +++ b/Makefile.objs
> > @@ -1,4 +1,13 @@
> >  #######################################################################
> > +# general compiler flags
> > +
> > +QEMU_CFLAGS += $(GLIB_CFLAGS)
> > +QEMU_CFLAGS += -I$(SRC_PATH)/include
> > +
> > +vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> > +vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
> > +
> > +#######################################################################
> >  # Stub library, linked in tools
> >  stub-obj-y = stubs/
> >  
> > @@ -236,12 +245,6 @@ universal-obj-y += $(qapi-obj-y)
> >  qga-obj-y = qga/ qemu-ga.o module.o qemu-tool.o
> >  qga-obj-$(CONFIG_POSIX) += qemu-sockets.o qemu-option.o
> >  
> > -vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> > -
> > -vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS)
> > -
> > -QEMU_CFLAGS+=$(GLIB_CFLAGS)
> > -
> >  nested-vars += \
> >  	stub-obj-y \
> >  	qga-obj-y \
> > 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/8] Move -I$(SRC_PATH)/include compiler flag to Makefile.objs

[Andreas Färber]

Am 14.12.2012 18:21, schrieb Eduardo Habkost:
> On Fri, Dec 14, 2012 at 04:34:29PM +0100, Andreas Färber wrote:
>> Waiting for ack or nack from Paolo here. I am expecting some overlap
>> with his header file reorganization series.
>>
>> My previous (unanswered?) question was why you are moving vl.o lines in
>> addition to the QEMU_CFLAGS lines that you mention in the commit message.
> 
> 
> I thought this note in the commit message would answer the question:
> 
>>> This also moves the existing CFLAGS lines from Makefile.objs at the
>>> beginning of the file, to keep them all in the same place.
> 
> In other words: it's cosmetic, just to keep all the QEMU_CLFAGS lines
> inside Makefile.objs grouped in a visible place at the beginning of the
> file.
> 
> (You noticed that I am moving the vl.o lines _inside_ Makefile.obj,
> right? They are not being moved between different files.)

Nah, you caught me there, must've misread that on a previous submission
(or it changed or whatever).

Anyway, Paolo's header reorganization was pulled by now, so this patch
no longer seems necessary, series compiles without. Please shout if I'm
misreading this!

Andreas

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

Re: [Qemu-devel] [PATCH 1/8] Move -I$(SRC_PATH)/include compiler flag to Makefile.objs

[Eduardo Habkost]

On Wed, Jan 02, 2013 at 02:48:00PM +0100, Andreas Färber wrote:
> Am 14.12.2012 18:21, schrieb Eduardo Habkost:
> > On Fri, Dec 14, 2012 at 04:34:29PM +0100, Andreas Färber wrote:
> >> Waiting for ack or nack from Paolo here. I am expecting some overlap
> >> with his header file reorganization series.
> >>
> >> My previous (unanswered?) question was why you are moving vl.o lines in
> >> addition to the QEMU_CFLAGS lines that you mention in the commit message.
> > 
> > 
> > I thought this note in the commit message would answer the question:
> > 
> >>> This also moves the existing CFLAGS lines from Makefile.objs at the
> >>> beginning of the file, to keep them all in the same place.
> > 
> > In other words: it's cosmetic, just to keep all the QEMU_CLFAGS lines
> > inside Makefile.objs grouped in a visible place at the beginning of the
> > file.
> > 
> > (You noticed that I am moving the vl.o lines _inside_ Makefile.obj,
> > right? They are not being moved between different files.)
> 
> Nah, you caught me there, must've misread that on a previous submission
> (or it changed or whatever).
> 
> Anyway, Paolo's header reorganization was pulled by now, so this patch
> no longer seems necessary, series compiles without. Please shout if I'm
> misreading this!

Correct, commit 9d9199a003 from Paolo makes this patch unnecessary.

-- 
Eduardo

[Qemu-devel] [PATCH 2/8] libqemustub: Add qemu_[un]register_reset() stubs

[Eduardo Habkost]

This will be useful for code that don't call qemu_devices_reset() (e.g.
*-user). If qemu_devices_reset() is never called, it means we don't need
to keep track of the reset handler list.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 stubs/Makefile.objs |  1 +
 stubs/reset.c       | 13 +++++++++++++
 2 files changed, 14 insertions(+)
 create mode 100644 stubs/reset.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 035b29a..00f0b64 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -5,4 +5,5 @@ stub-obj-y += fdset-get-fd.o
 stub-obj-y += fdset-remove-fd.o
 stub-obj-y += get-fd.o
 stub-obj-y += set-fd-handler.o
+stub-obj-y += reset.o
 stub-obj-$(CONFIG_WIN32) += fd-register.o
diff --git a/stubs/reset.c b/stubs/reset.c
new file mode 100644
index 0000000..ad28725
--- /dev/null
+++ b/stubs/reset.c
@@ -0,0 +1,13 @@
+#include "hw/hw.h"
+
+/* Stub functions for binaries that never call qemu_devices_reset(),
+ * and don't need to keep track of the reset handler list.
+ */
+
+void qemu_register_reset(QEMUResetHandler *func, void *opaque)
+{
+}
+
+void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
+{
+}
-- 
1.7.11.7

[Qemu-devel] [PATCH 3/8] libqemustub: vmstate register/unregister stubs

[Eduardo Habkost]

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

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

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

Changes v2 -> v3:
 - Subject: qdev: add weak aliases for vmstate handling on qdev.c
 - Make vmstate_register_with_alias_id()/vmstate_unregister()
   have GCC_WEAK versions, instead of creating a new function
 - Kept qdev_get_vmsd() inside qdev.c

Changss v3 -> v4:
 - Use the new QEMU_WEAK_ALIAS system instead of GCC_WEAK

Changes v4 -> v5:
 - Use the new libqemustub.a, instead of QEMU_WEAK_ALIAS

Changes v5 -> v6:
 - Cosmetic whitespace changes
---
 stubs/Makefile.objs |  1 +
 stubs/vmstate.c     | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 stubs/vmstate.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 00f0b64..ca2197e 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -6,4 +6,5 @@ stub-obj-y += fdset-remove-fd.o
 stub-obj-y += get-fd.o
 stub-obj-y += set-fd-handler.o
 stub-obj-y += reset.o
+stub-obj-y += vmstate.o
 stub-obj-$(CONFIG_WIN32) += fd-register.o
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
new file mode 100644
index 0000000..badf79e
--- /dev/null
+++ b/stubs/vmstate.c
@@ -0,0 +1,17 @@
+#include "qemu-common.h"
+#include "vmstate.h"
+
+int vmstate_register_with_alias_id(DeviceState *dev,
+                                   int instance_id,
+                                   const VMStateDescription *vmsd,
+                                   void *base, int alias_id,
+                                   int required_for_version)
+{
+    return 0;
+}
+
+void vmstate_unregister(DeviceState *dev,
+                        const VMStateDescription *vmsd,
+                        void *opaque)
+{
+}
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 3/8] libqemustub: vmstate register/unregister stubs

[Andreas Färber]

Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> diff --git a/stubs/vmstate.c b/stubs/vmstate.c
> new file mode 100644
> index 0000000..badf79e
> --- /dev/null
> +++ b/stubs/vmstate.c
> @@ -0,0 +1,17 @@
> +#include "qemu-common.h"
> +#include "vmstate.h"

Needed to update this to "migration/vmstate.h" since Paolo's header file
reorganization.

Andreas

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

[Qemu-devel] [PATCH 4/8] libqemustub: sysbus_get_default() stub

[Eduardo Habkost]

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

Note that code that uses NULL as the bus with qdev{_try,}_create()
implicitly uses sysbus_get_default() as the bus, and will still require
sysbus.c to be compiled in.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
 - Use the new QEMU_WEAK_ALIAS mechanism, instead of GCC_WEAK

Changes v2 -> v3:
 - Use the new libqemustub.a mechanism, instead of QEMU_WEAK_ALIAS
---
 stubs/Makefile.objs | 1 +
 stubs/sysbus.c      | 6 ++++++
 2 files changed, 7 insertions(+)
 create mode 100644 stubs/sysbus.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index ca2197e..7672c69 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -7,4 +7,5 @@ stub-obj-y += get-fd.o
 stub-obj-y += set-fd-handler.o
 stub-obj-y += reset.o
 stub-obj-y += vmstate.o
+stub-obj-y += sysbus.o
 stub-obj-$(CONFIG_WIN32) += fd-register.o
diff --git a/stubs/sysbus.c b/stubs/sysbus.c
new file mode 100644
index 0000000..e134965
--- /dev/null
+++ b/stubs/sysbus.c
@@ -0,0 +1,6 @@
+#include "hw/qdev-core.h"
+
+BusState *sysbus_get_default(void)
+{
+    return NULL;
+}
-- 
1.7.11.7

[Qemu-devel] [PATCH 5/8] qdev: Coding style fixes

[Eduardo Habkost]

Add missing braces and break lines larger than 80 chars.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/qdev-properties.c | 53 ++++++++++++++++++++++++++++++++++------------------
 hw/qdev.c            |  3 ++-
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 81d901c..67543fd 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -95,10 +95,11 @@ static void bit_prop_set(DeviceState *dev, Property *props, bool val)
 {
     uint32_t *p = qdev_get_prop_ptr(dev, props);
     uint32_t mask = qdev_get_prop_mask(props);
-    if (val)
+    if (val) {
         *p |= mask;
-    else
+    } else {
         *p &= ~mask;
+    }
 }
 
 static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
@@ -420,11 +421,13 @@ static void release_string(Object *obj, const char *name, void *opaque)
     g_free(*(char **)qdev_get_prop_ptr(DEVICE(obj), prop));
 }
 
-static int print_string(DeviceState *dev, Property *prop, char *dest, size_t len)
+static int print_string(DeviceState *dev, Property *prop, char *dest,
+                        size_t len)
 {
     char **ptr = qdev_get_prop_ptr(dev, prop);
-    if (!*ptr)
+    if (!*ptr) {
         return snprintf(dest, len, "<null>");
+    }
     return snprintf(dest, len, "\"%s\"", *ptr);
 }
 
@@ -483,10 +486,12 @@ static int parse_drive(DeviceState *dev, const char *str, void **ptr)
     BlockDriverState *bs;
 
     bs = bdrv_find(str);
-    if (bs == NULL)
+    if (bs == NULL) {
         return -ENOENT;
-    if (bdrv_attach_dev(bs, dev) < 0)
+    }
+    if (bdrv_attach_dev(bs, dev) < 0) {
         return -EEXIST;
+    }
     *ptr = bs;
     return 0;
 }
@@ -749,16 +754,20 @@ static void set_mac(Object *obj, Visitor *v, void *opaque,
     }
 
     for (i = 0, pos = 0; i < 6; i++, pos += 3) {
-        if (!qemu_isxdigit(str[pos]))
+        if (!qemu_isxdigit(str[pos])) {
             goto inval;
-        if (!qemu_isxdigit(str[pos+1]))
+        }
+        if (!qemu_isxdigit(str[pos+1])) {
             goto inval;
+        }
         if (i == 5) {
-            if (str[pos+2] != '\0')
+            if (str[pos+2] != '\0') {
                 goto inval;
+            }
         } else {
-            if (str[pos+2] != ':' && str[pos+2] != '-')
+            if (str[pos+2] != ':' && str[pos+2] != '-') {
                 goto inval;
+            }
         }
         mac->a[i] = strtol(str+pos, &p, 16);
     }
@@ -864,7 +873,8 @@ invalid:
     g_free(str);
 }
 
-static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t len)
+static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest,
+                           size_t len)
 {
     int32_t *ptr = qdev_get_prop_ptr(dev, prop);
 
@@ -1038,11 +1048,13 @@ PropertyInfo qdev_prop_pci_host_devaddr = {
 
 static Property *qdev_prop_walk(Property *props, const char *name)
 {
-    if (!props)
+    if (!props) {
         return NULL;
+    }
     while (props->name) {
-        if (strcmp(props->name, name) == 0)
+        if (strcmp(props->name, name) == 0) {
             return props;
+        }
         props++;
     }
     return NULL;
@@ -1158,7 +1170,8 @@ void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value)
     assert_no_error(errp);
 }
 
-int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *value)
+int qdev_prop_set_drive(DeviceState *dev, const char *name,
+                        BlockDriverState *value)
 {
     Error *errp = NULL;
     const char *bdrv_name = value ? bdrv_get_device_name(value) : "";
@@ -1172,13 +1185,15 @@ int qdev_prop_set_drive(DeviceState *dev, const char *name, BlockDriverState *va
     return 0;
 }
 
-void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, BlockDriverState *value)
+void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
+                                BlockDriverState *value)
 {
     if (qdev_prop_set_drive(dev, name, value) < 0) {
         exit(1);
     }
 }
-void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *value)
+void qdev_prop_set_chr(DeviceState *dev, const char *name,
+                       CharDriverState *value)
 {
     Error *errp = NULL;
     assert(!value || value->label);
@@ -1187,7 +1202,8 @@ void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *valu
     assert_no_error(errp);
 }
 
-void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value)
+void qdev_prop_set_netdev(DeviceState *dev, const char *name,
+                          NetClientState *value)
 {
     Error *errp = NULL;
     assert(!value || value->name);
@@ -1229,7 +1245,8 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
     *ptr = value;
 }
 
-static QTAILQ_HEAD(, GlobalProperty) global_props = QTAILQ_HEAD_INITIALIZER(global_props);
+static QTAILQ_HEAD(, GlobalProperty) global_props =
+        QTAILQ_HEAD_INITIALIZER(global_props);
 
 static void qdev_prop_register_global(GlobalProperty *prop)
 {
diff --git a/hw/qdev.c b/hw/qdev.c
index 599382c..0f8b878 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -315,8 +315,9 @@ void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
 {
     qdev_prop_set_macaddr(dev, "mac", nd->macaddr.a);
-    if (nd->netdev)
+    if (nd->netdev) {
         qdev_prop_set_netdev(dev, "netdev", nd->netdev);
+    }
     if (nd->nvectors != DEV_NVECTORS_UNSPECIFIED &&
         object_property_find(OBJECT(dev), "vectors", NULL)) {
         qdev_prop_set_uint32(dev, "vectors", nd->nvectors);
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 5/8] qdev: Coding style fixes

[Andreas Färber]

Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> Add missing braces and break lines larger than 80 chars.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

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

Andreas

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

[Qemu-devel] [PATCH 6/8] qdev-properties.c: Separate core from the code used only by qemu-system-*

[Eduardo Habkost]

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

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

The copyright/license of qdev-properties.c isn't explicitly stated at
the file, so add a simple copyright/license header pointing to the
commit ID of the original file.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Detailed changelog:

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

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

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

Changes v4 -> v5:
 - Remove large copyright header and instead just point to the original
   file it was based on

Changes v5 -> v6:
 - Removed inter-SoB line changelog from commit message

Changes v6 -> v7:
 - Incorporate qdev-properties.c coding style fixes
---
 hw/Makefile.objs            |   1 +
 hw/qdev-properties-system.c | 358 ++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev-properties.c        | 327 +---------------------------------------
 hw/qdev-properties.h        |   1 +
 hw/qdev.c                   |  14 --
 5 files changed, 361 insertions(+), 340 deletions(-)
 create mode 100644 hw/qdev-properties-system.c

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

Re: [Qemu-devel] [PATCH 6/8] qdev-properties.c: Separate core from the code used only by qemu-system-*

[Andreas Färber]

Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> This separates the qdev properties code in two parts:
>  - qdev-properties.c, that contains most of the qdev properties code;
>  - qdev-properties-system.c for code specific for qemu-system-*,
>    containing:
>    - Property types: drive, chr, netdev, vlan, that depend on code that
>      won't be included on *-user
>    - qemu_add_globals(), that depends on qemu-config.o.
> 
> This change should help on two things:
>  - Allowing DeviceState to be used by *-user without pulling
>    dependencies that are specific for qemu-system-*;
>  - Writing qdev unit tests without pulling too many dependencies.
> 
> The copyright/license of qdev-properties.c isn't explicitly stated at
> the file, so add a simple copyright/license header pointing to the
> commit ID of the original file.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

I dropped a trailing white line in qdev-properties-system.c; otherwise
it passed checkpatch.pl cleanly now. I verified that nothing got lost or
accidentally changed during the code movement.

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

Andreas

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

[Qemu-devel] [PATCH 7/8] include qdev code into *-user, too

[Eduardo Habkost]

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

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

Changes v2 -> v3:
 - Add reset.o to hw-core-obj-y

Changes v3 -> v4:
 - Removed reset.o again (it was replaced by stubs on libqemustub.a)
---
 Makefile.objs    | 8 ++++++++
 hw/Makefile.objs | 9 +++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 0a0a33a..8fe4991 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -31,6 +31,13 @@ qom-obj-y = qom/
 universal-obj-y += $(qom-obj-y)
 
 #######################################################################
+# Core hw code (qdev core)
+hw-core-obj-y += hw/
+hw-core-obj-y += qemu-option.o
+
+universal-obj-y += $(hw-core-obj-y)
+
+#######################################################################
 # oslib-obj-y is code depending on the OS (win32 vs posix)
 oslib-obj-y = osdep.o cutils.o qemu-timer-common.o
 oslib-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o
@@ -253,5 +260,6 @@ nested-vars += \
 	block-obj-y \
 	user-obj-y \
 	common-obj-y \
+	hw-core-obj-y \
 	extra-obj-y
 dummy := $(call unnest-vars)
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 96a8365..1815536 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,3 +1,9 @@
+# core qdev-related obj files, also used by *-user:
+hw-core-obj-y += qdev.o qdev-properties.o
+# irq.o needed for qdev GPIO handling:
+hw-core-obj-y += irq.o
+
+
 common-obj-y = usb/ ide/
 common-obj-y += loader.o
 common-obj-$(CONFIG_VIRTIO) += virtio-console.o
@@ -158,7 +164,6 @@ common-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 common-obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/
 
 common-obj-y += usb/
-common-obj-y += irq.o
 common-obj-$(CONFIG_PTIMER) += ptimer.o
 common-obj-$(CONFIG_MAX7310) += max7310.o
 common-obj-$(CONFIG_WM8750) += wm8750.o
@@ -184,7 +189,7 @@ common-obj-$(CONFIG_SD) += sd.o
 common-obj-y += bt.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o
 common-obj-y += bt-hci-csr.o
 common-obj-y += msmouse.o ps2.o
-common-obj-y += qdev.o qdev-properties.o qdev-monitor.o
+common-obj-y += qdev-monitor.o
 common-obj-y += qdev-properties-system.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
 
-- 
1.7.11.7

[Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Eduardo Habkost]

This finally makes the CPU class a child of DeviceState, allowing us to
start using DeviceState properties on CPU subclasses.

It has no_user=1, as creating CPUs using -device doesn't work yet.

(based on a previous patch from Igor Mammedov)

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 (imammedo) -> v2 (ehabkost):
 - Change CPU type declaration to hae TYPE_DEVICE as parent

Changes v2 -> v3 (ehabkost):
 - Set no_user=1 on the CPU class
---
 include/qemu/cpu.h | 6 +++---
 qom/cpu.c          | 5 ++++-
 2 files changed, 7 insertions(+), 4 deletions(-)

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

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Andreas Färber]

Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> This finally makes the CPU class a child of DeviceState, allowing us to
> start using DeviceState properties on CPU subclasses.
> 
> It has no_user=1, as creating CPUs using -device doesn't work yet.
> 
> (based on a previous patch from Igor Mammedov)
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 (imammedo) -> v2 (ehabkost):
>  - Change CPU type declaration to hae TYPE_DEVICE as parent
> 
> Changes v2 -> v3 (ehabkost):
>  - Set no_user=1 on the CPU class
> ---
>  include/qemu/cpu.h | 6 +++---
>  qom/cpu.c          | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> index 61b7698..bc004fd 100644
> --- a/include/qemu/cpu.h
> +++ b/include/qemu/cpu.h
> @@ -20,7 +20,7 @@
>  #ifndef QEMU_CPU_H
>  #define QEMU_CPU_H
>  
> -#include "qemu/object.h"
> +#include "hw/qdev-core.h"
>  #include "qemu-thread.h"
>  
>  /**
> @@ -46,7 +46,7 @@ typedef struct CPUState CPUState;
>   */
>  typedef struct CPUClass {
>      /*< private >*/
> -    ObjectClass parent_class;
> +    DeviceClass parent_class;
>      /*< public >*/
>  
>      void (*reset)(CPUState *cpu);
> @@ -62,7 +62,7 @@ typedef struct CPUClass {
>   */
>  struct CPUState {
>      /*< private >*/
> -    Object parent_obj;
> +    DeviceState parent_obj;
>      /*< public >*/
>  
>      struct QemuThread *thread;
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 5b36046..d301f72 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -20,6 +20,7 @@
>  
>  #include "qemu/cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev-core.h"
>  
>  void cpu_reset(CPUState *cpu)
>  {
> @@ -36,14 +37,16 @@ static void cpu_common_reset(CPUState *cpu)
>  
>  static void cpu_class_init(ObjectClass *klass, void *data)
>  {
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>      CPUClass *k = CPU_CLASS(klass);
>  
>      k->reset = cpu_common_reset;
> +    dc->no_user = 1;
>  }
>  
>  static TypeInfo cpu_type_info = {
>      .name = TYPE_CPU,
> -    .parent = TYPE_OBJECT,
> +    .parent = TYPE_DEVICE,
>      .instance_size = sizeof(CPUState),
>      .abstract = true,
>      .class_size = sizeof(CPUClass),

This patch makes the CPU a device and looks good so far but does not
initialize the devices in cpu_*_init() like Anthony did in his previous
patch. I am unsure whether you forgot to do so or whether you wanted to
help keep our new CPU code clean of old-style qdev_init_nofail() calls?
Since you don't add a qdev initfn here the main difference will be the
devices internally staying in "created" rather than "initialized" state.

If we merge some patch that adds the "realized" property first (probably
not through qom-cpu tree then) we could avoid qdev_init*() but the end
result targetted by Paolo was not to have object creators worry about
realization at all through recursive realization. So either solution
needs to be changed later on.

Andreas

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

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Eduardo Habkost]

On Wed, Dec 12, 2012 at 02:34:08PM +0100, Andreas Färber wrote:
> Am 05.12.2012 17:49, schrieb Eduardo Habkost:
[...]
> >  static TypeInfo cpu_type_info = {
> >      .name = TYPE_CPU,
> > -    .parent = TYPE_OBJECT,
> > +    .parent = TYPE_DEVICE,
> >      .instance_size = sizeof(CPUState),
> >      .abstract = true,
> >      .class_size = sizeof(CPUClass),
> 
> This patch makes the CPU a device and looks good so far but does not
> initialize the devices in cpu_*_init() like Anthony did in his previous
> patch. I am unsure whether you forgot to do so or whether you wanted to
> help keep our new CPU code clean of old-style qdev_init_nofail() calls?
> Since you don't add a qdev initfn here the main difference will be the
> devices internally staying in "created" rather than "initialized" state.

I think I used a version without the qdev_init_nofail() as base for this
series (we had multiple proposals being sent in parallel, in the
beginning), and in the end I forgot that we had a version with those
calls being added.

The CPU classes don't set any DeviceClass.init() method, so in theory
the missing qdev_init() calls wouldn't make any difference by now. On
the other hand, keeping the device in "created" state sounds bad... but
maybe this acceptable while we are still converting the CPU realize()
functions to fit inside the DeviceState initialization abstraction?


> 
> If we merge some patch that adds the "realized" property first (probably
> not through qom-cpu tree then) we could avoid qdev_init*() but the end
> result targetted by Paolo was not to have object creators worry about
> realization at all through recursive realization. So either solution
> needs to be changed later on.

I am interested in this series mainly because of the other features
provided by DeviceState: the static property and global-properties
system. The initialization abstraction provided by DeviceState will be
useful as well, but I also don't know what will be the best approach to
finally make the cpu_*init() functions sane. I still need to take a
better look at your QOM realize() RFC.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Igor Mammedov]

On Wed, 12 Dec 2012 11:59:01 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Wed, Dec 12, 2012 at 02:34:08PM +0100, Andreas Färber wrote:
> > Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> [...]
> > >  static TypeInfo cpu_type_info = {
> > >      .name = TYPE_CPU,
> > > -    .parent = TYPE_OBJECT,
> > > +    .parent = TYPE_DEVICE,
> > >      .instance_size = sizeof(CPUState),
> > >      .abstract = true,
> > >      .class_size = sizeof(CPUClass),
> > 
> > This patch makes the CPU a device and looks good so far but does not
> > initialize the devices in cpu_*_init() like Anthony did in his previous
> > patch. I am unsure whether you forgot to do so or whether you wanted to
> > help keep our new CPU code clean of old-style qdev_init_nofail() calls?
> > Since you don't add a qdev initfn here the main difference will be the
> > devices internally staying in "created" rather than "initialized" state.
> 
> I think I used a version without the qdev_init_nofail() as base for this
> series (we had multiple proposals being sent in parallel, in the
> beginning), and in the end I forgot that we had a version with those
> calls being added.
> 
> The CPU classes don't set any DeviceClass.init() method, so in theory
> the missing qdev_init() calls wouldn't make any difference by now. On
> the other hand, keeping the device in "created" state sounds bad... but
> maybe this acceptable while we are still converting the CPU realize()
> functions to fit inside the DeviceState initialization abstraction?
Testing shows that lack of qdev_create()/init() doesn't break anything so
far. I was planing to send hot-plug RFC after properties and subclasses for
x86 are done and temporally wrap x86_cpu_realize() inside DeviceClass.init()
so we could use qdev_create()/init() and device_add() for CPU.
It's still on my TODO list. Perhaps after I resubmit properties series (I
hope to do it this week), I'll redo hot-plug prototype using as a base my
experimental subclasses branch
https://github.com/imammedo/qemu/commits/x86-cpu-classes.WIP

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Eduardo Habkost]

On Wed, Dec 12, 2012 at 03:27:24PM +0100, Igor Mammedov wrote:
> On Wed, 12 Dec 2012 11:59:01 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Wed, Dec 12, 2012 at 02:34:08PM +0100, Andreas Färber wrote:
> > > Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> > [...]
> > > >  static TypeInfo cpu_type_info = {
> > > >      .name = TYPE_CPU,
> > > > -    .parent = TYPE_OBJECT,
> > > > +    .parent = TYPE_DEVICE,
> > > >      .instance_size = sizeof(CPUState),
> > > >      .abstract = true,
> > > >      .class_size = sizeof(CPUClass),
> > > 
> > > This patch makes the CPU a device and looks good so far but does not
> > > initialize the devices in cpu_*_init() like Anthony did in his previous
> > > patch. I am unsure whether you forgot to do so or whether you wanted to
> > > help keep our new CPU code clean of old-style qdev_init_nofail() calls?
> > > Since you don't add a qdev initfn here the main difference will be the
> > > devices internally staying in "created" rather than "initialized" state.
> > 
> > I think I used a version without the qdev_init_nofail() as base for this
> > series (we had multiple proposals being sent in parallel, in the
> > beginning), and in the end I forgot that we had a version with those
> > calls being added.
> > 
> > The CPU classes don't set any DeviceClass.init() method, so in theory
> > the missing qdev_init() calls wouldn't make any difference by now. On
> > the other hand, keeping the device in "created" state sounds bad... but
> > maybe this acceptable while we are still converting the CPU realize()
> > functions to fit inside the DeviceState initialization abstraction?
> Testing shows that lack of qdev_create()/init() doesn't break anything so
> far. I was planing to send hot-plug RFC after properties and subclasses for
> x86 are done and temporally wrap x86_cpu_realize() inside DeviceClass.init()
> so we could use qdev_create()/init() and device_add() for CPU.

It seems to be easy to gradually replace the existing cpu_*_realize()
calls inside cpu_*_init() with qdev_init[_nofail]() calls (making
DeviceClas.init point to cpu_*_realize()), one architecture at a time.

I mean: one day we should kill the cpu_*_init() functions and make it
CPU creation/initialization generic code. But while we don't do that, a
simple cpu_*_realize()->qdev_init() replacement sounds trivial.

But first I want to understand the difference between DeviceClass.init()
and the proposed DeviceClass.realize() abstraction (they look exactly
the same to me). I just sent a question as reply to Andreas' realizefn
RFC.

> It's still on my TODO list. Perhaps after I resubmit properties series (I
> hope to do it this week), I'll redo hot-plug prototype using as a base my
> experimental subclasses branch
> https://github.com/imammedo/qemu/commits/x86-cpu-classes.WIP

-- 
Eduardo

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Andreas Färber]

Am 12.12.2012 15:27, schrieb Igor Mammedov:
> On Wed, 12 Dec 2012 11:59:01 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>> On Wed, Dec 12, 2012 at 02:34:08PM +0100, Andreas Färber wrote:
>>> This patch makes the CPU a device and looks good so far but does not
>>> initialize the devices in cpu_*_init() like Anthony did in his previous
>>> patch. I am unsure whether you forgot to do so or whether you wanted to
>>> help keep our new CPU code clean of old-style qdev_init_nofail() calls?
>>> Since you don't add a qdev initfn here the main difference will be the
>>> devices internally staying in "created" rather than "initialized" state.
>>
>> I think I used a version without the qdev_init_nofail() as base for this
>> series (we had multiple proposals being sent in parallel, in the
>> beginning), and in the end I forgot that we had a version with those
>> calls being added.
>>
>> The CPU classes don't set any DeviceClass.init() method, so in theory
>> the missing qdev_init() calls wouldn't make any difference by now. On
>> the other hand, keeping the device in "created" state sounds bad... but
>> maybe this acceptable while we are still converting the CPU realize()
>> functions to fit inside the DeviceState initialization abstraction?
> Testing shows that lack of qdev_create()/init() doesn't break anything so
> far.

The latest motivation for making the CPU a device was to have the static
properties infrastructure for machine/CPU versioning. The global
property defaults are set in qdev's instance_init, so object_new() seems
fine for that.

qdev_[try_]create() would further set the parent bus to SysBus if NULL.
The CPU is not a SysBusDevice so I think not using qdev_create() may be
safer... Maybe Anthony or Paolo can confirm?

Andreas

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

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Paolo Bonzini]

Il 14/12/2012 16:29, Andreas Färber ha scritto:
> The latest motivation for making the CPU a device was to have the static
> properties infrastructure for machine/CPU versioning. The global
> property defaults are set in qdev's instance_init, so object_new() seems
> fine for that.
> 
> qdev_[try_]create() would further set the parent bus to SysBus if NULL.
> The CPU is not a SysBusDevice so I think not using qdev_create() may be
> safer... Maybe Anthony or Paolo can confirm?

I think various parts of qdev assume there is a bus, so actually using
SysBus would be safer (though uglier).

Paolo

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Andreas Färber]

Am 14.12.2012 16:40, schrieb Paolo Bonzini:
> Il 14/12/2012 16:29, Andreas Färber ha scritto:
>> The latest motivation for making the CPU a device was to have the static
>> properties infrastructure for machine/CPU versioning. The global
>> property defaults are set in qdev's instance_init, so object_new() seems
>> fine for that.
>>
>> qdev_[try_]create() would further set the parent bus to SysBus if NULL.
>> The CPU is not a SysBusDevice so I think not using qdev_create() may be
>> safer... Maybe Anthony or Paolo can confirm?
> 
> I think various parts of qdev assume there is a bus, so actually using
> SysBus would be safer (though uglier).

Hm, Anthony told me with one of his qbus refactoring patches back in
qom-next the last remaining assumptions (info qdm) were removed...

Probably we're the first to test though. ;)

Andreas

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

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Eduardo Habkost]

On Fri, Dec 14, 2012 at 04:44:24PM +0100, Andreas Färber wrote:
> Am 14.12.2012 16:40, schrieb Paolo Bonzini:
> > Il 14/12/2012 16:29, Andreas Färber ha scritto:
> >> The latest motivation for making the CPU a device was to have the static
> >> properties infrastructure for machine/CPU versioning. The global
> >> property defaults are set in qdev's instance_init, so object_new() seems
> >> fine for that.
> >>
> >> qdev_[try_]create() would further set the parent bus to SysBus if NULL.
> >> The CPU is not a SysBusDevice so I think not using qdev_create() may be
> >> safer... Maybe Anthony or Paolo can confirm?
> > 
> > I think various parts of qdev assume there is a bus, so actually using
> > SysBus would be safer (though uglier).
> 
> Hm, Anthony told me with one of his qbus refactoring patches back in
> qom-next the last remaining assumptions (info qdm) were removed...
> 
> Probably we're the first to test though. ;)

BTW, we're also not including SysBus, on *-user.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Andreas Färber]

Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> This finally makes the CPU class a child of DeviceState, allowing us to
> start using DeviceState properties on CPU subclasses.

To avoid confusion with child<> properties and DeviceState vs.
DeviceClass I have reworded this to "subclass of Device" in my
qom-cpu-dev queue.

> 
> It has no_user=1, as creating CPUs using -device doesn't work yet.
> 

> (based on a previous patch from Igor Mammedov)

Can this comment be turned into or amended by the usual Signed-off-by?

> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 (imammedo) -> v2 (ehabkost):
>  - Change CPU type declaration to hae TYPE_DEVICE as parent
> 
> Changes v2 -> v3 (ehabkost):
>  - Set no_user=1 on the CPU class
> ---
>  include/qemu/cpu.h | 6 +++---
>  qom/cpu.c          | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> index 61b7698..bc004fd 100644
> --- a/include/qemu/cpu.h
> +++ b/include/qemu/cpu.h
> @@ -20,7 +20,7 @@
>  #ifndef QEMU_CPU_H
>  #define QEMU_CPU_H
>  
> -#include "qemu/object.h"
> +#include "hw/qdev-core.h"
>  #include "qemu-thread.h"
>  
>  /**
[...]
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 5b36046..d301f72 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -20,6 +20,7 @@
>  
>  #include "qemu/cpu.h"
>  #include "qemu-common.h"
> +#include "hw/qdev-core.h"

Already included via qom/cpu.h (formerly qemu/cpu.h) above, dropping.

>  
>  void cpu_reset(CPUState *cpu)
>  {
> @@ -36,14 +37,16 @@ static void cpu_common_reset(CPUState *cpu)
>  
>  static void cpu_class_init(ObjectClass *klass, void *data)
>  {
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>      CPUClass *k = CPU_CLASS(klass);
>  
>      k->reset = cpu_common_reset;
> +    dc->no_user = 1;
>  }

I wonder if we should add a comment that we are intentionally not
hooking up dc->reset (yet)?

>  
>  static TypeInfo cpu_type_info = {

Would like to add the missing const while touching this.

>      .name = TYPE_CPU,
> -    .parent = TYPE_OBJECT,
> +    .parent = TYPE_DEVICE,
>      .instance_size = sizeof(CPUState),
>      .abstract = true,
>      .class_size = sizeof(CPUClass),

My testing so far confirms that the combination of object_new() without
qdev_init[_nofail]() is working fine.

Using qdev_create() in the current state of stubs would lead to a silly
if-bus-is-NULL-set-it-to-NULL sequence on top of object_new(). I do not
expect qdev_create() to grow in functionality, so continuing to use
object_new() should be okay - SoCs like my Tegra model may want to use
object_initialize() so we cannot prescribe using qdev_create() anyway.

qdev_init_nofail() would call the qdev initfn (to be replaced by
realizefn, not used for CPU in this patch), then if no parent add it to
/machine/unassigned, register VMSD if not NULL, update the internal
state (blocking static property changes) and if hotplugged reset (unused
due to dc->no_user and lack of dc->reset). The /machine/unassigned part
may be interesting, e.g., for APIC modelling (so that we can model the
former ptr property / now pointer-setting as a link<> property).

With these considerations I am leaning towards accepting this patch if
nobody objects, so that we can move on to the next refactorings...

Regards,
Andreas

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

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Igor Mammedov]

On Wed, 02 Jan 2013 16:08:42 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> > This finally makes the CPU class a child of DeviceState, allowing us to
> > start using DeviceState properties on CPU subclasses.
> 
> To avoid confusion with child<> properties and DeviceState vs.
> DeviceClass I have reworded this to "subclass of Device" in my
> qom-cpu-dev queue.
> 
> > 
> > It has no_user=1, as creating CPUs using -device doesn't work yet.
> > 
> 
> > (based on a previous patch from Igor Mammedov)
> 
> Can this comment be turned into or amended by the usual Signed-off-by?
Signed-off-by should be ok.

> 
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v1 (imammedo) -> v2 (ehabkost):
> >  - Change CPU type declaration to hae TYPE_DEVICE as parent
> > 
> > Changes v2 -> v3 (ehabkost):
> >  - Set no_user=1 on the CPU class
> > ---
> >  include/qemu/cpu.h | 6 +++---
> >  qom/cpu.c          | 5 ++++-
> >  2 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> > index 61b7698..bc004fd 100644
> > --- a/include/qemu/cpu.h
> > +++ b/include/qemu/cpu.h
> > @@ -20,7 +20,7 @@
> >  #ifndef QEMU_CPU_H
> >  #define QEMU_CPU_H
> >  
> > -#include "qemu/object.h"
> > +#include "hw/qdev-core.h"
> >  #include "qemu-thread.h"
> >  
> >  /**
> [...]
> > diff --git a/qom/cpu.c b/qom/cpu.c
> > index 5b36046..d301f72 100644
> > --- a/qom/cpu.c
> > +++ b/qom/cpu.c
> > @@ -20,6 +20,7 @@
> >  
> >  #include "qemu/cpu.h"
> >  #include "qemu-common.h"
> > +#include "hw/qdev-core.h"
> 
> Already included via qom/cpu.h (formerly qemu/cpu.h) above, dropping.
> 
> >  
> >  void cpu_reset(CPUState *cpu)
> >  {
> > @@ -36,14 +37,16 @@ static void cpu_common_reset(CPUState *cpu)
> >  
> >  static void cpu_class_init(ObjectClass *klass, void *data)
> >  {
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> >      CPUClass *k = CPU_CLASS(klass);
> >  
> >      k->reset = cpu_common_reset;
> > +    dc->no_user = 1;
> >  }
> 
> I wonder if we should add a comment that we are intentionally not
> hooking up dc->reset (yet)?
not relevant to this patch, could be separate patch though.

> 
> >  
> >  static TypeInfo cpu_type_info = {
> 
> Would like to add the missing const while touching this.
> 
> >      .name = TYPE_CPU,
> > -    .parent = TYPE_OBJECT,
> > +    .parent = TYPE_DEVICE,
> >      .instance_size = sizeof(CPUState),
> >      .abstract = true,
> >      .class_size = sizeof(CPUClass),
> 
> My testing so far confirms that the combination of object_new() without
> qdev_init[_nofail]() is working fine.
+1, I tested this combo for (x86)-(softmmu|linux-user) targets, no issues were
found so far.

> 
> Using qdev_create() in the current state of stubs would lead to a silly
> if-bus-is-NULL-set-it-to-NULL sequence on top of object_new(). I do not
> expect qdev_create() to grow in functionality, so continuing to use
> object_new() should be okay - SoCs like my Tegra model may want to use
> object_initialize() so we cannot prescribe using qdev_create() anyway.
> 
> qdev_init_nofail() would call the qdev initfn (to be replaced by
> realizefn, not used for CPU in this patch), then if no parent add it to
> /machine/unassigned, register VMSD if not NULL, update the internal
> state (blocking static property changes) and if hotplugged reset (unused
> due to dc->no_user and lack of dc->reset). The /machine/unassigned part
> may be interesting, e.g., for APIC modelling (so that we can model the
> former ptr property / now pointer-setting as a link<> property).
> 
> With these considerations I am leaning towards accepting this patch if
> nobody objects, so that we can move on to the next refactorings...
+1

> 
> Regards,
> Andreas
> 

Re: [Qemu-devel] [PATCH 8/8] qom: Make CPU a child of DeviceState

[Eduardo Habkost]

On Wed, Jan 02, 2013 at 05:40:58PM +0100, Igor Mammedov wrote:
> On Wed, 02 Jan 2013 16:08:42 +0100
> Andreas Färber <afaerber@suse.de> wrote:
> 
> > Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> > > This finally makes the CPU class a child of DeviceState, allowing us to
> > > start using DeviceState properties on CPU subclasses.
> > 
> > To avoid confusion with child<> properties and DeviceState vs.
> > DeviceClass I have reworded this to "subclass of Device" in my
> > qom-cpu-dev queue.
> > 
> > > 
> > > It has no_user=1, as creating CPUs using -device doesn't work yet.
> > > 
> > 
> > > (based on a previous patch from Igor Mammedov)
> > 
> > Can this comment be turned into or amended by the usual Signed-off-by?
> Signed-off-by should be ok.

OK to me, as well. Should I resubmit, or can Andreas edit it when
committing the patch?

> 
> > 
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > > ---
> > > Changes v1 (imammedo) -> v2 (ehabkost):
> > >  - Change CPU type declaration to hae TYPE_DEVICE as parent
> > > 
> > > Changes v2 -> v3 (ehabkost):
> > >  - Set no_user=1 on the CPU class
> > > ---
> > >  include/qemu/cpu.h | 6 +++---
> > >  qom/cpu.c          | 5 ++++-
> > >  2 files changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
> > > index 61b7698..bc004fd 100644
> > > --- a/include/qemu/cpu.h
> > > +++ b/include/qemu/cpu.h
> > > @@ -20,7 +20,7 @@
> > >  #ifndef QEMU_CPU_H
> > >  #define QEMU_CPU_H
> > >  
> > > -#include "qemu/object.h"
> > > +#include "hw/qdev-core.h"
> > >  #include "qemu-thread.h"
> > >  
> > >  /**
> > [...]
> > > diff --git a/qom/cpu.c b/qom/cpu.c
> > > index 5b36046..d301f72 100644
> > > --- a/qom/cpu.c
> > > +++ b/qom/cpu.c
> > > @@ -20,6 +20,7 @@
> > >  
> > >  #include "qemu/cpu.h"
> > >  #include "qemu-common.h"
> > > +#include "hw/qdev-core.h"
> > 
> > Already included via qom/cpu.h (formerly qemu/cpu.h) above, dropping.
> > 
> > >  
> > >  void cpu_reset(CPUState *cpu)
> > >  {
> > > @@ -36,14 +37,16 @@ static void cpu_common_reset(CPUState *cpu)
> > >  
> > >  static void cpu_class_init(ObjectClass *klass, void *data)
> > >  {
> > > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > >      CPUClass *k = CPU_CLASS(klass);
> > >  
> > >      k->reset = cpu_common_reset;
> > > +    dc->no_user = 1;
> > >  }
> > 
> > I wonder if we should add a comment that we are intentionally not
> > hooking up dc->reset (yet)?
> not relevant to this patch, could be separate patch though.
> 
> > 
> > >  
> > >  static TypeInfo cpu_type_info = {
> > 
> > Would like to add the missing const while touching this.
> > 
> > >      .name = TYPE_CPU,
> > > -    .parent = TYPE_OBJECT,
> > > +    .parent = TYPE_DEVICE,
> > >      .instance_size = sizeof(CPUState),
> > >      .abstract = true,
> > >      .class_size = sizeof(CPUClass),
> > 
> > My testing so far confirms that the combination of object_new() without
> > qdev_init[_nofail]() is working fine.
> +1, I tested this combo for (x86)-(softmmu|linux-user) targets, no issues were
> found so far.
> 
> > 
> > Using qdev_create() in the current state of stubs would lead to a silly
> > if-bus-is-NULL-set-it-to-NULL sequence on top of object_new(). I do not
> > expect qdev_create() to grow in functionality, so continuing to use
> > object_new() should be okay - SoCs like my Tegra model may want to use
> > object_initialize() so we cannot prescribe using qdev_create() anyway.
> > 
> > qdev_init_nofail() would call the qdev initfn (to be replaced by
> > realizefn, not used for CPU in this patch), then if no parent add it to
> > /machine/unassigned, register VMSD if not NULL, update the internal
> > state (blocking static property changes) and if hotplugged reset (unused
> > due to dc->no_user and lack of dc->reset). The /machine/unassigned part
> > may be interesting, e.g., for APIC modelling (so that we can model the
> > former ptr property / now pointer-setting as a link<> property).
> > 
> > With these considerations I am leaning towards accepting this patch if
> > nobody objects, so that we can move on to the next refactorings...
> +1
> 
> > 
> > Regards,
> > Andreas
> > 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/8] CPU DeviceState v10

[Andreas Färber]

Am 05.12.2012 17:49, schrieb Eduardo Habkost:
> Eduardo Habkost (8):
[...]
>   libqemustub: Add qemu_[un]register_reset() stubs
>   libqemustub: vmstate register/unregister stubs
>   libqemustub: sysbus_get_default() stub
[...]
>   include qdev code into *-user, too
>   qom: Make CPU a child of DeviceState

Thanks, remainder of series applied to qom-cpu queue (with
rebasing/modifications previously mentioned):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

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