Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

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

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

 Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored
 out initialization to parallel_hds_isa_init function in hw/char/parallel.c 
 that is not build. 

 Stub file is added to be able to disable CONFIG_PARALLEL. This file is used
 in targets using parallel_hds_isa_init and provide empty definition of this
 function.

 Signed-off-by: Miroslav Rezanina mreza...@redhat.com

 ---
  hw/i386/Makefile.objs| 1 +
  hw/mips/Makefile.objs| 2 ++
  hw/sparc64/Makefile.objs | 2 ++
  stubs/parallel-stub.c| 7 +++

Nitpick: the existing stub/*.c naming practice suggests
stubs/parallel.c.

  4 files changed, 12 insertions(+)
  create mode 100644 stubs/parallel-stub.c

 diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
 index e058a39..2b7131a 100644
 --- a/hw/i386/Makefile.objs
 +++ b/hw/i386/Makefile.objs
 @@ -4,6 +4,7 @@ obj-y += pc.o pc_piix.o pc_q35.o
  obj-y += pc_sysfw.o
  obj-y += intel_iommu.o
  obj-$(CONFIG_XEN) += ../xenpv/ xen/
 +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
  
  obj-y += kvmvapic.o
  obj-y += acpi-build.o

Can we rely on the linker to pull parallel-stub.o from a suitable .a
libqemustub.a when needed?

 diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
 index 0a652f8..2e65305 100644
 --- a/hw/mips/Makefile.objs
 +++ b/hw/mips/Makefile.objs
 @@ -2,3 +2,5 @@ obj-y += mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
  obj-y += addr.o cputimer.o mips_int.o
  obj-$(CONFIG_FULONG) += mips_fulong2e.o
  obj-y += gt64xxx_pci.o
 +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
 +
 diff --git a/hw/sparc64/Makefile.objs b/hw/sparc64/Makefile.objs
 index a84cfe3..7696611 100644
 --- a/hw/sparc64/Makefile.objs
 +++ b/hw/sparc64/Makefile.objs
 @@ -1 +1,3 @@
  obj-y += sun4u.o
 +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
 +
 diff --git a/stubs/parallel-stub.c b/stubs/parallel-stub.c
 new file mode 100644
 index 000..949c1b2
 --- /dev/null
 +++ b/stubs/parallel-stub.c
 @@ -0,0 +1,7 @@
 +#include qemu/typedefs.h
 +#include hw/isa/isa.h
 +#include hw/i386/pc.h
 +
 +void parallel_hds_isa_init(ISABus *bus, int n)
 +{
 +}

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

2015-05-11 Thread Paolo Bonzini 


On 11/05/2015 07:38, mreza...@redhat.com wrote:
 From: Miroslav Rezanina mreza...@redhat.com
 
 Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored
 out initialization to parallel_hds_isa_init function in hw/char/parallel.c 
 that is not build. 
 
 Stub file is added to be able to disable CONFIG_PARALLEL. This file is used
 in targets using parallel_hds_isa_init and provide empty definition of this
 function.
 
 Signed-off-by: Miroslav Rezanina mreza...@redhat.com

This patch will make -parallel a nop.  The right thing to do is to
fail startup whenever -parallel is passed and CONFIG_PARALLEL is disabled.

You can move parallel_hds_isa_init and parallel_init to
hw/isa/isa-bus.c, or to a new file hw/isa/isa-devices.c.

Paolo

 ---
  hw/i386/Makefile.objs| 1 +
  hw/mips/Makefile.objs| 2 ++
  hw/sparc64/Makefile.objs | 2 ++
  stubs/parallel-stub.c| 7 +++
  4 files changed, 12 insertions(+)
  create mode 100644 stubs/parallel-stub.c
 
 diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
 index e058a39..2b7131a 100644
 --- a/hw/i386/Makefile.objs
 +++ b/hw/i386/Makefile.objs
 @@ -4,6 +4,7 @@ obj-y += pc.o pc_piix.o pc_q35.o
  obj-y += pc_sysfw.o
  obj-y += intel_iommu.o
  obj-$(CONFIG_XEN) += ../xenpv/ xen/
 +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
  
  obj-y += kvmvapic.o
  obj-y += acpi-build.o
 diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
 index 0a652f8..2e65305 100644
 --- a/hw/mips/Makefile.objs
 +++ b/hw/mips/Makefile.objs
 @@ -2,3 +2,5 @@ obj-y += mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
  obj-y += addr.o cputimer.o mips_int.o
  obj-$(CONFIG_FULONG) += mips_fulong2e.o
  obj-y += gt64xxx_pci.o
 +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
 +
 diff --git a/hw/sparc64/Makefile.objs b/hw/sparc64/Makefile.objs
 index a84cfe3..7696611 100644
 --- a/hw/sparc64/Makefile.objs
 +++ b/hw/sparc64/Makefile.objs
 @@ -1 +1,3 @@
  obj-y += sun4u.o
 +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
 +
 diff --git a/stubs/parallel-stub.c b/stubs/parallel-stub.c
 new file mode 100644
 index 000..949c1b2
 --- /dev/null
 +++ b/stubs/parallel-stub.c
 @@ -0,0 +1,7 @@
 +#include qemu/typedefs.h
 +#include hw/isa/isa.h
 +#include hw/i386/pc.h
 +
 +void parallel_hds_isa_init(ISABus *bus, int n)
 +{
 +}

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

2015-05-11 Thread Paolo Bonzini 


On 11/05/2015 11:36, Miroslav Rezanina wrote:
  This patch will make -parallel a nop.  The right thing to do is to
  fail startup whenever -parallel is passed and CONFIG_PARALLEL is disabled.
  
 This was original behavior before 07dc788. Intention of this patch is to
 make qemu buildable with CONFIG_PARALLEL disabled.

Understood, but in the meanwhile Markus wrote commit 4bc6a3e (parallel:
parallel_hds_isa_init() shouldn't fail, 2015-02-04), and you should
preserve the logic of that commit.

  You can move parallel_hds_isa_init and parallel_init to
  hw/isa/isa-bus.c, or to a new file hw/isa/isa-devices.c.
  
 Moving functions will cause abort with Unknown device error.

This is the right behavior that we want: exit QEMU, not go on silently
without the parallel port.

If you do not like the abort, you should revert commit 4bc6a3e, and make
parallel_hds_isa_init check for failure of parallel_init.  But for me
it's okay to just let it abort.

Paolo

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

2015-05-11 Thread Miroslav Rezanina 
On Mon, May 11, 2015 at 10:40:04AM +0200, Paolo Bonzini wrote:
 
 
 On 11/05/2015 07:38, mreza...@redhat.com wrote:
  From: Miroslav Rezanina mreza...@redhat.com
  
  Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored
  out initialization to parallel_hds_isa_init function in hw/char/parallel.c 
  that is not build. 
  
  Stub file is added to be able to disable CONFIG_PARALLEL. This file is used
  in targets using parallel_hds_isa_init and provide empty definition of this
  function.
  
  Signed-off-by: Miroslav Rezanina mreza...@redhat.com
 
 This patch will make -parallel a nop.  The right thing to do is to
 fail startup whenever -parallel is passed and CONFIG_PARALLEL is disabled.
 
This was original behavior before 07dc788. Intention of this patch is to
make qemu buildable with CONFIG_PARALLEL disabled.

 You can move parallel_hds_isa_init and parallel_init to
 hw/isa/isa-bus.c, or to a new file hw/isa/isa-devices.c.
 

Moving functions will cause abort with Unknown device error.

 Paolo
 
  ---
   hw/i386/Makefile.objs| 1 +
   hw/mips/Makefile.objs| 2 ++
   hw/sparc64/Makefile.objs | 2 ++
   stubs/parallel-stub.c| 7 +++
   4 files changed, 12 insertions(+)
   create mode 100644 stubs/parallel-stub.c
  
  diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
  index e058a39..2b7131a 100644
  --- a/hw/i386/Makefile.objs
  +++ b/hw/i386/Makefile.objs
  @@ -4,6 +4,7 @@ obj-y += pc.o pc_piix.o pc_q35.o
   obj-y += pc_sysfw.o
   obj-y += intel_iommu.o
   obj-$(CONFIG_XEN) += ../xenpv/ xen/
  +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
   
   obj-y += kvmvapic.o
   obj-y += acpi-build.o
  diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
  index 0a652f8..2e65305 100644
  --- a/hw/mips/Makefile.objs
  +++ b/hw/mips/Makefile.objs
  @@ -2,3 +2,5 @@ obj-y += mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
   obj-y += addr.o cputimer.o mips_int.o
   obj-$(CONFIG_FULONG) += mips_fulong2e.o
   obj-y += gt64xxx_pci.o
  +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
  +
  diff --git a/hw/sparc64/Makefile.objs b/hw/sparc64/Makefile.objs
  index a84cfe3..7696611 100644
  --- a/hw/sparc64/Makefile.objs
  +++ b/hw/sparc64/Makefile.objs
  @@ -1 +1,3 @@
   obj-y += sun4u.o
  +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
  +
  diff --git a/stubs/parallel-stub.c b/stubs/parallel-stub.c
  new file mode 100644
  index 000..949c1b2
  --- /dev/null
  +++ b/stubs/parallel-stub.c
  @@ -0,0 +1,7 @@
  +#include qemu/typedefs.h
  +#include hw/isa/isa.h
  +#include hw/i386/pc.h
  +
  +void parallel_hds_isa_init(ISABus *bus, int n)
  +{
  +}

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

2015-05-11 Thread Miroslav Rezanina 
On Mon, May 11, 2015 at 08:46:19AM +0200, Markus Armbruster wrote:
 mreza...@redhat.com writes:
 
  From: Miroslav Rezanina mreza...@redhat.com
 
  Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored
  out initialization to parallel_hds_isa_init function in hw/char/parallel.c 
  that is not build. 
 
  Stub file is added to be able to disable CONFIG_PARALLEL. This file is used
  in targets using parallel_hds_isa_init and provide empty definition of this
  function.
 
  Signed-off-by: Miroslav Rezanina mreza...@redhat.com
 
  ---
   hw/i386/Makefile.objs| 1 +
   hw/mips/Makefile.objs| 2 ++
   hw/sparc64/Makefile.objs | 2 ++
   stubs/parallel-stub.c| 7 +++
 
 Nitpick: the existing stub/*.c naming practice suggests
 stubs/parallel.c.

Yeah...I forget to rename it after moving from repository
root to stub directory. I originally have it in repository
root as it is not included in libqemustub. So the naming can
be treated as hint that something is different. However,
I can rename it to follow stubs/* naming.
 
   4 files changed, 12 insertions(+)
   create mode 100644 stubs/parallel-stub.c
 
  diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
  index e058a39..2b7131a 100644
  --- a/hw/i386/Makefile.objs
  +++ b/hw/i386/Makefile.objs
  @@ -4,6 +4,7 @@ obj-y += pc.o pc_piix.o pc_q35.o
   obj-y += pc_sysfw.o
   obj-y += intel_iommu.o
   obj-$(CONFIG_XEN) += ../xenpv/ xen/
  +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
   
   obj-y += kvmvapic.o
   obj-y += acpi-build.o
 
 Can we rely on the linker to pull parallel-stub.o from a suitable .a
 libqemustub.a when needed?

We do not have to as parallel-stub.o is not included in libqemustub.a.
It is linked directly in case CONFIG_PARALLEL is not defined (for
targets using parallel_hds_isa_init).

Mirek

 
  diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
  index 0a652f8..2e65305 100644
  --- a/hw/mips/Makefile.objs
  +++ b/hw/mips/Makefile.objs
  @@ -2,3 +2,5 @@ obj-y += mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
   obj-y += addr.o cputimer.o mips_int.o
   obj-$(CONFIG_FULONG) += mips_fulong2e.o
   obj-y += gt64xxx_pci.o
  +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
  +
  diff --git a/hw/sparc64/Makefile.objs b/hw/sparc64/Makefile.objs
  index a84cfe3..7696611 100644
  --- a/hw/sparc64/Makefile.objs
  +++ b/hw/sparc64/Makefile.objs
  @@ -1 +1,3 @@
   obj-y += sun4u.o
  +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
  +
  diff --git a/stubs/parallel-stub.c b/stubs/parallel-stub.c
  new file mode 100644
  index 000..949c1b2
  --- /dev/null
  +++ b/stubs/parallel-stub.c
  @@ -0,0 +1,7 @@
  +#include qemu/typedefs.h
  +#include hw/isa/isa.h
  +#include hw/i386/pc.h
  +
  +void parallel_hds_isa_init(ISABus *bus, int n)
  +{
  +}

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

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

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

 Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored
 out initialization to parallel_hds_isa_init function in hw/char/parallel.c 
 that is not build. 

 Stub file is added to be able to disable CONFIG_PARALLEL. This file is used
 in targets using parallel_hds_isa_init and provide empty definition of this
 function.

 Signed-off-by: Miroslav Rezanina mreza...@redhat.com

 ---
  hw/i386/Makefile.objs| 1 +
  hw/mips/Makefile.objs| 2 ++
  hw/sparc64/Makefile.objs | 2 ++
  stubs/parallel-stub.c| 7 +++
  4 files changed, 12 insertions(+)
  create mode 100644 stubs/parallel-stub.c

 diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
 index e058a39..2b7131a 100644
 --- a/hw/i386/Makefile.objs
 +++ b/hw/i386/Makefile.objs
 @@ -4,6 +4,7 @@ obj-y += pc.o pc_piix.o pc_q35.o
  obj-y += pc_sysfw.o
  obj-y += intel_iommu.o
  obj-$(CONFIG_XEN) += ../xenpv/ xen/
 +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
  
  obj-y += kvmvapic.o
  obj-y += acpi-build.o
 diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
 index 0a652f8..2e65305 100644
 --- a/hw/mips/Makefile.objs
 +++ b/hw/mips/Makefile.objs
 @@ -2,3 +2,5 @@ obj-y += mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
  obj-y += addr.o cputimer.o mips_int.o
  obj-$(CONFIG_FULONG) += mips_fulong2e.o
  obj-y += gt64xxx_pci.o
 +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
 +

git-am complains new blank line at EOF.

 diff --git a/hw/sparc64/Makefile.objs b/hw/sparc64/Makefile.objs
 index a84cfe3..7696611 100644
 --- a/hw/sparc64/Makefile.objs
 +++ b/hw/sparc64/Makefile.objs
 @@ -1 +1,3 @@
  obj-y += sun4u.o
 +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
 +

Likewise.

 diff --git a/stubs/parallel-stub.c b/stubs/parallel-stub.c
 new file mode 100644
 index 000..949c1b2
 --- /dev/null
 +++ b/stubs/parallel-stub.c
 @@ -0,0 +1,7 @@
 +#include qemu/typedefs.h
 +#include hw/isa/isa.h
 +#include hw/i386/pc.h
 +
 +void parallel_hds_isa_init(ISABus *bus, int n)
 +{
 +}

Fails to link if I disable CONFIG_PARALLEL in
default-configs/mips-softmmu.mak:

  LINK  mips-softmmu/qemu-system-mips
hw/mips/mips_jazz.o: In function `mips_jazz_init':
/home/armbru/work/qemu/hw/mips/mips_jazz.c:323: undefined reference to 
`parallel_mm_init'
collect2: error: ld returned 1 exit status
make[1]: *** [qemu-system-mips] Error 1

To fix that, you'd need to stub out parallel_mm_init(), too.

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

2015-05-11 Thread Markus Armbruster 
Miroslav Rezanina mreza...@redhat.com writes:

 On Mon, May 11, 2015 at 08:46:19AM +0200, Markus Armbruster wrote:
 mreza...@redhat.com writes:
 
  From: Miroslav Rezanina mreza...@redhat.com
 
  Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored
  out initialization to parallel_hds_isa_init function in hw/char/parallel.c 
  that is not build. 
 
  Stub file is added to be able to disable CONFIG_PARALLEL. This file is used
  in targets using parallel_hds_isa_init and provide empty definition of this
  function.
 
  Signed-off-by: Miroslav Rezanina mreza...@redhat.com
 
  ---
   hw/i386/Makefile.objs| 1 +
   hw/mips/Makefile.objs| 2 ++
   hw/sparc64/Makefile.objs | 2 ++
   stubs/parallel-stub.c| 7 +++
 
 Nitpick: the existing stub/*.c naming practice suggests
 stubs/parallel.c.

 Yeah...I forget to rename it after moving from repository
 root to stub directory. I originally have it in repository
 root as it is not included in libqemustub. So the naming can
 be treated as hint that something is different. However,
 I can rename it to follow stubs/* naming.
 
   4 files changed, 12 insertions(+)
   create mode 100644 stubs/parallel-stub.c
 
  diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
  index e058a39..2b7131a 100644
  --- a/hw/i386/Makefile.objs
  +++ b/hw/i386/Makefile.objs
  @@ -4,6 +4,7 @@ obj-y += pc.o pc_piix.o pc_q35.o
   obj-y += pc_sysfw.o
   obj-y += intel_iommu.o
   obj-$(CONFIG_XEN) += ../xenpv/ xen/
  +obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
   
   obj-y += kvmvapic.o
   obj-y += acpi-build.o
 
 Can we rely on the linker to pull parallel-stub.o from a suitable .a
 libqemustub.a when needed?

 We do not have to as parallel-stub.o is not included in libqemustub.a.
 It is linked directly in case CONFIG_PARALLEL is not defined (for
 targets using parallel_hds_isa_init).

No other stub is linked conditionally like that.  Instead we simply let
the linker pick up stubs as needed.  Let's do the same here.  Sketch:

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 8beff4c..ad4e110 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -24,6 +24,7 @@ stub-obj-y += mon-printf.o
 stub-obj-y += mon-set-error.o
 stub-obj-y += monitor-init.o
 stub-obj-y += notify-event.o
+stub-obj-y += parallel.o
 stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
 stub-obj-y += qtest.o
 stub-obj-y += reset.o
diff --git a/stubs/parallel.c b/stubs/parallel.c
new file mode 100644
index 000..2fa7e3a
--- /dev/null
+++ b/stubs/parallel.c
@@ -0,0 +1,12 @@
+#include hw/i386/pc.h
+
+void parallel_hds_isa_init(ISABus *bus, int n)
+{
+}
+
+bool parallel_mm_init(MemoryRegion *address_space,
+  hwaddr base, int it_shift, qemu_irq irq,
+  CharDriverState *chr)
+{
+return false;
+}

Links fine with this crude test patch on top:

diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index 5931cc8..c736436 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -1,6 +1,6 @@
 common-obj-$(CONFIG_IPACK) += ipoctal232.o
 common-obj-$(CONFIG_ESCC) += escc.o
-common-obj-$(CONFIG_PARALLEL) += parallel.o
+#common-obj-$(CONFIG_PARALLEL) += parallel.o
 common-obj-$(CONFIG_PL011) += pl011.o
 common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
 common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

2015-05-11 Thread Markus Armbruster 
Paolo Bonzini pbonz...@redhat.com writes:

 On 11/05/2015 11:36, Miroslav Rezanina wrote:
  This patch will make -parallel a nop.  The right thing to do is to
  fail startup whenever -parallel is passed and CONFIG_PARALLEL is disabled.
  
 This was original behavior before 07dc788. Intention of this patch is to
 make qemu buildable with CONFIG_PARALLEL disabled.

 Understood, but in the meanwhile Markus wrote commit 4bc6a3e (parallel:
 parallel_hds_isa_init() shouldn't fail, 2015-02-04), and you should
 preserve the logic of that commit.

I have to admit didn't consider CONFIG_PARALLEL when I wrote the commit.

  You can move parallel_hds_isa_init and parallel_init to
  hw/isa/isa-bus.c, or to a new file hw/isa/isa-devices.c.
  
 Moving functions will cause abort with Unknown device error.

 This is the right behavior that we want: exit QEMU, not go on silently
 without the parallel port.

I agree silently ignoring command line options isn't nice, but it's
unfortunately what QEMU has always done.

In particular, -parallel is silently ignored with the vast majority of
machine types.  The few machine types that implement it silently ignore
it only when they fail to create the device.

I'm fine with changing -parallel to either create the device or fail.
Seems outside the scope of this series, though.

 If you do not like the abort, you should revert commit 4bc6a3e, and make
 parallel_hds_isa_init check for failure of parallel_init.  But for me
 it's okay to just let it abort.

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

2015-05-11 Thread Paolo Bonzini 


On 11/05/2015 17:52, Markus Armbruster wrote:
 Paolo Bonzini pbonz...@redhat.com writes:
 
 On 11/05/2015 11:36, Miroslav Rezanina wrote:
 This patch will make -parallel a nop.  The right thing to do is to
 fail startup whenever -parallel is passed and CONFIG_PARALLEL is disabled.

 This was original behavior before 07dc788. Intention of this patch is to
 make qemu buildable with CONFIG_PARALLEL disabled.

 Understood, but in the meanwhile Markus wrote commit 4bc6a3e (parallel:
 parallel_hds_isa_init() shouldn't fail, 2015-02-04), and you should
 preserve the logic of that commit.
 
 I have to admit didn't consider CONFIG_PARALLEL when I wrote the commit.
 
 You can move parallel_hds_isa_init and parallel_init to
 hw/isa/isa-bus.c, or to a new file hw/isa/isa-devices.c.

 Moving functions will cause abort with Unknown device error.

 This is the right behavior that we want: exit QEMU, not go on silently
 without the parallel port.
 
 I agree silently ignoring command line options isn't nice, but it's
 unfortunately what QEMU has always done.
 
 In particular, -parallel is silently ignored with the vast majority of
 machine types.  The few machine types that implement it silently ignore
 it only when they fail to create the device.

Right.  However, if I move a VM (that has a parallel port, which already
puts us in a kind of reductio as absurdum) from a QEMU that has parallel
ports to a QEMU that doesn't have them, _and the board does something
about -parallel_, I think there should be a failure.

This is because whoever compiled that QEMU is crippling a board, no
matter what their reasons are.

 I'm fine with changing -parallel to either create the device or fail.
 Seems outside the scope of this series, though.

Why?  Your patch is _already_ trying to create the device or fail,
even if the failure mode isn't particularly clean.  The thing that can
be debated is whether to keep the abort or require a nicer check, and
I'm not requiring it.

Paolo

 If you do not like the abort, you should revert commit 4bc6a3e, and make
 parallel_hds_isa_init check for failure of parallel_init.  But for me
 it's okay to just let it abort.

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

2015-05-11 Thread Paolo Bonzini 


On 11/05/2015 17:09, Markus Armbruster wrote:
 Fails to link if I disable CONFIG_PARALLEL in
 default-configs/mips-softmmu.mak:
 
   LINK  mips-softmmu/qemu-system-mips
 hw/mips/mips_jazz.o: In function `mips_jazz_init':
 /home/armbru/work/qemu/hw/mips/mips_jazz.c:323: undefined reference to 
 `parallel_mm_init'
 collect2: error: ld returned 1 exit status
 make[1]: *** [qemu-system-mips] Error 1
 
 To fix that, you'd need to stub out parallel_mm_init(), too.

I would ignore that.  parallel_mm_init isn't even qdev-ified, and
qdevification would fix that problem too.

Paolo

Re: [Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

2015-05-11 Thread Peter Maydell 
On 11 May 2015 at 16:52, Markus Armbruster arm...@redhat.com wrote:
 I agree silently ignoring command line options isn't nice, but it's
 unfortunately what QEMU has always done.

Actually, we're inconsistent (who'd have guessed? :-)).
For instance if CONFIG_CURSES isn't defined then we print an error
if you try to use the curses option, similarly for iscsi, slirp
related options, tpmdev, and others. But there are also many
options where (as you note) we just silently ignore them, and
even a few where we print a warning but continue to boot (eg
some of the networking config options).

-- PMM

[Qemu-devel] [PATCHv2] parallel: Allow to disable CONFIG_PARALLEL

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

Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored
out initialization to parallel_hds_isa_init function in hw/char/parallel.c 
that is not build. 

Stub file is added to be able to disable CONFIG_PARALLEL. This file is used
in targets using parallel_hds_isa_init and provide empty definition of this
function.

Signed-off-by: Miroslav Rezanina mreza...@redhat.com

---
 hw/i386/Makefile.objs| 1 +
 hw/mips/Makefile.objs| 2 ++
 hw/sparc64/Makefile.objs | 2 ++
 stubs/parallel-stub.c| 7 +++
 4 files changed, 12 insertions(+)
 create mode 100644 stubs/parallel-stub.c

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index e058a39..2b7131a 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -4,6 +4,7 @@ obj-y += pc.o pc_piix.o pc_q35.o
 obj-y += pc_sysfw.o
 obj-y += intel_iommu.o
 obj-$(CONFIG_XEN) += ../xenpv/ xen/
+obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
 
 obj-y += kvmvapic.o
 obj-y += acpi-build.o
diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
index 0a652f8..2e65305 100644
--- a/hw/mips/Makefile.objs
+++ b/hw/mips/Makefile.objs
@@ -2,3 +2,5 @@ obj-y += mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
 obj-y += addr.o cputimer.o mips_int.o
 obj-$(CONFIG_FULONG) += mips_fulong2e.o
 obj-y += gt64xxx_pci.o
+obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
+
diff --git a/hw/sparc64/Makefile.objs b/hw/sparc64/Makefile.objs
index a84cfe3..7696611 100644
--- a/hw/sparc64/Makefile.objs
+++ b/hw/sparc64/Makefile.objs
@@ -1 +1,3 @@
 obj-y += sun4u.o
+obj-$(call lnot,$(CONFIG_PARALLEL)) += ../../stubs/parallel-stub.o
+
diff --git a/stubs/parallel-stub.c b/stubs/parallel-stub.c
new file mode 100644
index 000..949c1b2
--- /dev/null
+++ b/stubs/parallel-stub.c
@@ -0,0 +1,7 @@
+#include qemu/typedefs.h
+#include hw/isa/isa.h
+#include hw/i386/pc.h
+
+void parallel_hds_isa_init(ISABus *bus, int n)
+{
+}
-- 
2.1.0