Re: [Qemu-devel] [PATCHv3 1/2] Move parallel_hds_isa_init to hw/isa/isa-bus.c

2015-05-13 Thread Miroslav Rezanina 
On Wed, May 13, 2015 at 10:01:12AM +0200, Markus Armbruster wrote:
 mreza...@redhat.com writes:
 
  From: Miroslav Rezanina mreza...@redhat.com
 
  Disabling CONFIG_PARALLEL cause removing parallel_hds_isa_init defined in
  parallel.c. This function is called during initialization of some boards so
  disabling CONFIG_PARALLEL cause build failure.
 
  This patch moves parallel_hds_isa_init to hw/isa/isa-bus.c so it is included
  in case of disabled CONFIG_PARALLEL. Build is successful but qemu will abort
  with Unknown device error when function is called.
 
 Can't reproduce this error.  Test case: compile with CONFIG_PARALLEL=y
 taken out, run like
 
 $ qemu-system-x86_64 -nodefaults -S -display none -parallel stdio
 
  Signed-off-by: Miroslav Rezanina mreza...@redhat.com
  ---
   hw/char/parallel.c | 25 -
   hw/isa/isa-bus.c   | 29 +
   2 files changed, 29 insertions(+), 25 deletions(-)
 
  diff --git a/hw/char/parallel.c b/hw/char/parallel.c
  index 4079554..c2b553f 100644
  --- a/hw/char/parallel.c
  +++ b/hw/char/parallel.c
  @@ -641,28 +641,3 @@ static void parallel_register_types(void)
   }
   
   type_init(parallel_register_types)
  -
  -static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
  -{
  -DeviceState *dev;
  -ISADevice *isadev;
  -
  -isadev = isa_create(bus, isa-parallel);
  -dev = DEVICE(isadev);
  -qdev_prop_set_uint32(dev, index, index);
  -qdev_prop_set_chr(dev, chardev, chr);
  -qdev_init_nofail(dev);
  -}
  -
  -void parallel_hds_isa_init(ISABus *bus, int n)
  -{
  -int i;
  -
  -assert(n = MAX_PARALLEL_PORTS);
  -
  -for (i = 0; i  n; i++) {
  -if (parallel_hds[i]) {
  -parallel_init(bus, i, parallel_hds[i]);
  -}
  -}
  -}
  diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
  index 825aa62..94f645c 100644
  --- a/hw/isa/isa-bus.c
  +++ b/hw/isa/isa-bus.c
  @@ -21,6 +21,7 @@
   #include hw/sysbus.h
   #include sysemu/sysemu.h
   #include hw/isa/isa.h
  +#include hw/i386/pc.h
   
   static ISABus *isabus;
   
  @@ -267,3 +268,31 @@ MemoryRegion *isa_address_space_io(ISADevice *dev)
   }
   
   type_init(isabus_register_types)
  +
  +static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
  +{
  +DeviceState *dev;
  +ISADevice *isadev;
  +
  +isadev = isa_try_create(bus, isa-parallel);
  +if (!isadev) {
  +   return;
  +}
 
 Not just code motion!  You change the code to ignore the error.
 
 If that's what we want, the commit message needs fixing.

Ouch,this is mixed in change from different code version. Intention was to move 
code only. This is the reason to different behavior. I will send fixed
version.

Mirek
 
  +dev = DEVICE(isadev);
  +qdev_prop_set_uint32(dev, index, index);
  +qdev_prop_set_chr(dev, chardev, chr);
  +qdev_init_nofail(dev);
  +}
  +
  +void parallel_hds_isa_init(ISABus *bus, int n)
  +{
  +int i;
  +
  +assert(n = MAX_PARALLEL_PORTS);
  +
  +for (i = 0; i  n; i++) {
  +if (parallel_hds[i]) {
  +parallel_init(bus, i, parallel_hds[i]);
  +}
  +}
  +}

Re: [Qemu-devel] [PATCHv3 1/2] Move parallel_hds_isa_init to hw/isa/isa-bus.c

2015-05-13 Thread Markus Armbruster 
mreza...@redhat.com writes:

 From: Miroslav Rezanina mreza...@redhat.com

 Disabling CONFIG_PARALLEL cause removing parallel_hds_isa_init defined in
 parallel.c. This function is called during initialization of some boards so
 disabling CONFIG_PARALLEL cause build failure.

 This patch moves parallel_hds_isa_init to hw/isa/isa-bus.c so it is included
 in case of disabled CONFIG_PARALLEL. Build is successful but qemu will abort
 with Unknown device error when function is called.

Can't reproduce this error.  Test case: compile with CONFIG_PARALLEL=y
taken out, run like

$ qemu-system-x86_64 -nodefaults -S -display none -parallel stdio

 Signed-off-by: Miroslav Rezanina mreza...@redhat.com
 ---
  hw/char/parallel.c | 25 -
  hw/isa/isa-bus.c   | 29 +
  2 files changed, 29 insertions(+), 25 deletions(-)

 diff --git a/hw/char/parallel.c b/hw/char/parallel.c
 index 4079554..c2b553f 100644
 --- a/hw/char/parallel.c
 +++ b/hw/char/parallel.c
 @@ -641,28 +641,3 @@ static void parallel_register_types(void)
  }
  
  type_init(parallel_register_types)
 -
 -static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
 -{
 -DeviceState *dev;
 -ISADevice *isadev;
 -
 -isadev = isa_create(bus, isa-parallel);
 -dev = DEVICE(isadev);
 -qdev_prop_set_uint32(dev, index, index);
 -qdev_prop_set_chr(dev, chardev, chr);
 -qdev_init_nofail(dev);
 -}
 -
 -void parallel_hds_isa_init(ISABus *bus, int n)
 -{
 -int i;
 -
 -assert(n = MAX_PARALLEL_PORTS);
 -
 -for (i = 0; i  n; i++) {
 -if (parallel_hds[i]) {
 -parallel_init(bus, i, parallel_hds[i]);
 -}
 -}
 -}
 diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
 index 825aa62..94f645c 100644
 --- a/hw/isa/isa-bus.c
 +++ b/hw/isa/isa-bus.c
 @@ -21,6 +21,7 @@
  #include hw/sysbus.h
  #include sysemu/sysemu.h
  #include hw/isa/isa.h
 +#include hw/i386/pc.h
  
  static ISABus *isabus;
  
 @@ -267,3 +268,31 @@ MemoryRegion *isa_address_space_io(ISADevice *dev)
  }
  
  type_init(isabus_register_types)
 +
 +static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
 +{
 +DeviceState *dev;
 +ISADevice *isadev;
 +
 +isadev = isa_try_create(bus, isa-parallel);
 +if (!isadev) {
 +   return;
 +}

Not just code motion!  You change the code to ignore the error.

If that's what we want, the commit message needs fixing.

 +dev = DEVICE(isadev);
 +qdev_prop_set_uint32(dev, index, index);
 +qdev_prop_set_chr(dev, chardev, chr);
 +qdev_init_nofail(dev);
 +}
 +
 +void parallel_hds_isa_init(ISABus *bus, int n)
 +{
 +int i;
 +
 +assert(n = MAX_PARALLEL_PORTS);
 +
 +for (i = 0; i  n; i++) {
 +if (parallel_hds[i]) {
 +parallel_init(bus, i, parallel_hds[i]);
 +}
 +}
 +}

[Qemu-devel] [PATCHv3 1/2] Move parallel_hds_isa_init to hw/isa/isa-bus.c

2015-05-12 Thread mrezanin 
From: Miroslav Rezanina mreza...@redhat.com

Disabling CONFIG_PARALLEL cause removing parallel_hds_isa_init defined in
parallel.c. This function is called during initialization of some boards so
disabling CONFIG_PARALLEL cause build failure.

This patch moves parallel_hds_isa_init to hw/isa/isa-bus.c so it is included
in case of disabled CONFIG_PARALLEL. Build is successful but qemu will abort
with Unknown device error when function is called.

Signed-off-by: Miroslav Rezanina mreza...@redhat.com
---
 hw/char/parallel.c | 25 -
 hw/isa/isa-bus.c   | 29 +
 2 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 4079554..c2b553f 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -641,28 +641,3 @@ static void parallel_register_types(void)
 }
 
 type_init(parallel_register_types)
-
-static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
-{
-DeviceState *dev;
-ISADevice *isadev;
-
-isadev = isa_create(bus, isa-parallel);
-dev = DEVICE(isadev);
-qdev_prop_set_uint32(dev, index, index);
-qdev_prop_set_chr(dev, chardev, chr);
-qdev_init_nofail(dev);
-}
-
-void parallel_hds_isa_init(ISABus *bus, int n)
-{
-int i;
-
-assert(n = MAX_PARALLEL_PORTS);
-
-for (i = 0; i  n; i++) {
-if (parallel_hds[i]) {
-parallel_init(bus, i, parallel_hds[i]);
-}
-}
-}
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 825aa62..94f645c 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -21,6 +21,7 @@
 #include hw/sysbus.h
 #include sysemu/sysemu.h
 #include hw/isa/isa.h
+#include hw/i386/pc.h
 
 static ISABus *isabus;
 
@@ -267,3 +268,31 @@ MemoryRegion *isa_address_space_io(ISADevice *dev)
 }
 
 type_init(isabus_register_types)
+
+static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
+{
+DeviceState *dev;
+ISADevice *isadev;
+
+isadev = isa_try_create(bus, isa-parallel);
+if (!isadev) {
+   return;
+}
+dev = DEVICE(isadev);
+qdev_prop_set_uint32(dev, index, index);
+qdev_prop_set_chr(dev, chardev, chr);
+qdev_init_nofail(dev);
+}
+
+void parallel_hds_isa_init(ISABus *bus, int n)
+{
+int i;
+
+assert(n = MAX_PARALLEL_PORTS);
+
+for (i = 0; i  n; i++) {
+if (parallel_hds[i]) {
+parallel_init(bus, i, parallel_hds[i]);
+}
+}
+}
-- 
2.1.0

Re: [Qemu-devel] [PATCHv3 1/2] Move parallel_hds_isa_init to hw/isa/isa-bus.c

2015-05-12 Thread Paolo Bonzini 


On 12/05/2015 08:22, mreza...@redhat.com wrote:
 From: Miroslav Rezanina mreza...@redhat.com
 
 Disabling CONFIG_PARALLEL cause removing parallel_hds_isa_init defined in
 parallel.c. This function is called during initialization of some boards so
 disabling CONFIG_PARALLEL cause build failure.
 
 This patch moves parallel_hds_isa_init to hw/isa/isa-bus.c so it is included
 in case of disabled CONFIG_PARALLEL. Build is successful but qemu will abort
 with Unknown device error when function is called.
 
 Signed-off-by: Miroslav Rezanina mreza...@redhat.com
 ---
  hw/char/parallel.c | 25 -
  hw/isa/isa-bus.c   | 29 +
  2 files changed, 29 insertions(+), 25 deletions(-)
 
 diff --git a/hw/char/parallel.c b/hw/char/parallel.c
 index 4079554..c2b553f 100644
 --- a/hw/char/parallel.c
 +++ b/hw/char/parallel.c
 @@ -641,28 +641,3 @@ static void parallel_register_types(void)
  }
  
  type_init(parallel_register_types)
 -
 -static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
 -{
 -DeviceState *dev;
 -ISADevice *isadev;
 -
 -isadev = isa_create(bus, isa-parallel);
 -dev = DEVICE(isadev);
 -qdev_prop_set_uint32(dev, index, index);
 -qdev_prop_set_chr(dev, chardev, chr);
 -qdev_init_nofail(dev);
 -}
 -
 -void parallel_hds_isa_init(ISABus *bus, int n)
 -{
 -int i;
 -
 -assert(n = MAX_PARALLEL_PORTS);
 -
 -for (i = 0; i  n; i++) {
 -if (parallel_hds[i]) {
 -parallel_init(bus, i, parallel_hds[i]);
 -}
 -}
 -}
 diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
 index 825aa62..94f645c 100644
 --- a/hw/isa/isa-bus.c
 +++ b/hw/isa/isa-bus.c
 @@ -21,6 +21,7 @@
  #include hw/sysbus.h
  #include sysemu/sysemu.h
  #include hw/isa/isa.h
 +#include hw/i386/pc.h
  
  static ISABus *isabus;
  
 @@ -267,3 +268,31 @@ MemoryRegion *isa_address_space_io(ISADevice *dev)
  }
  
  type_init(isabus_register_types)
 +
 +static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
 +{
 +DeviceState *dev;
 +ISADevice *isadev;
 +
 +isadev = isa_try_create(bus, isa-parallel);
 +if (!isadev) {
 +   return;
 +}
 +dev = DEVICE(isadev);
 +qdev_prop_set_uint32(dev, index, index);
 +qdev_prop_set_chr(dev, chardev, chr);
 +qdev_init_nofail(dev);
 +}
 +
 +void parallel_hds_isa_init(ISABus *bus, int n)
 +{
 +int i;
 +
 +assert(n = MAX_PARALLEL_PORTS);
 +
 +for (i = 0; i  n; i++) {
 +if (parallel_hds[i]) {
 +parallel_init(bus, i, parallel_hds[i]);
 +}
 +}
 +}
 

ACK

Paolo