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

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

[mrezanin]

From: Miroslav Rezanina <mrezanin@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 <mrezanin@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 0000000..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

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

[Markus Armbruster]

mrezanin@redhat.com writes:

> From: Miroslav Rezanina <mrezanin@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 <mrezanin@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 0000000..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

[Miroslav Rezanina]

On Mon, May 11, 2015 at 08:46:19AM +0200, Markus Armbruster wrote:
> mrezanin@redhat.com writes:
> 
> > From: Miroslav Rezanina <mrezanin@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 <mrezanin@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 0000000..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

[Markus Armbruster]

Miroslav Rezanina <mrezanin@redhat.com> writes:

> On Mon, May 11, 2015 at 08:46:19AM +0200, Markus Armbruster wrote:
>> mrezanin@redhat.com writes:
>> 
>> > From: Miroslav Rezanina <mrezanin@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 <mrezanin@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 0000000..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

[Paolo Bonzini]

On 11/05/2015 07:38, mrezanin@redhat.com wrote:
> From: Miroslav Rezanina <mrezanin@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 <mrezanin@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 0000000..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

[Miroslav Rezanina]

On Mon, May 11, 2015 at 10:40:04AM +0200, Paolo Bonzini wrote:
> 
> 
> On 11/05/2015 07:38, mrezanin@redhat.com wrote:
> > From: Miroslav Rezanina <mrezanin@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 <mrezanin@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 0000000..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

[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

[Markus Armbruster]

Paolo Bonzini <pbonzini@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

[Paolo Bonzini]

On 11/05/2015 17:52, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@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

[Peter Maydell]

On 11 May 2015 at 16:52, Markus Armbruster <armbru@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

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

[Markus Armbruster]

mrezanin@redhat.com writes:

> From: Miroslav Rezanina <mrezanin@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 <mrezanin@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 0000000..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

[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