[Qemu-devel] [PATCH 0/4] fix/add CONFIG_* options for VMWare device emulation

[Qemu-devel] [PATCH 0/4] fix/add CONFIG_* options for VMWare device emulation

[Eduardo Habkost]

Hi,

This series makes CONFIG_VMWARE_VGA actually work (today we can't disable the
option without getting a build error).

It also add two new options: CONFIG_VMMOUSE and CONFIG_VMPORT, for vmmouse.o
and vmport.o.

Eduardo Habkost (4):
  Add config-devices.h again
  skip pci_vmsvga_init() calls if CONFIG_VMWARE_VGA is disabled
  add CONFIG_VMMOUSE option
  add CONFIG_VMPORT option

 Makefile                           |    7 +++++--
 Makefile.target                    |    8 ++++++--
 config.h                           |   11 +++++++++++
 default-configs/i386-softmmu.mak   |    3 +++
 default-configs/x86_64-softmmu.mak |    3 +++
 hw/mips_malta.c                    |    4 ++++
 hw/pc.c                            |    6 ++++++
 hw/pc_piix.c                       |    2 ++
 8 files changed, 40 insertions(+), 4 deletions(-)

-- 
1.7.3.2

[Qemu-devel] [PATCH 1/4] Add config-devices.h again

[Eduardo Habkost]

This reverts part of commit a992fe3d0fc185112677286f7a02204d8245b61e.

We do have code that needs #ifdefs depending on the list of enabled devices,
but currently that code breaks when we try to disable a feature that is enabled
by default.

For example, if we try to disable CONFIG_VMWARE_VGA, we get the following:

   LINK  x86_64-softmmu/qemu-system-x86_64
  pc.o: In function `pc_vga_init':
  /home/ehabkost/pessoal/proj/virt/qemu/qemu/hw/pc.c:991: undefined reference to `pci_vmsvga_init'
  collect2: ld returned 1 exit status
  make[1]: *** [qemu-system-x86_64] Error 1
  rm config-devices.h-timestamp
  make: *** [subdir-x86_64-softmmu] Error 2

config-devices.h will allow us to add an #ifdef to fix the above error, and
other similar cases.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 Makefile        |    7 +++++--
 Makefile.target |    2 +-
 config.h        |   11 +++++++++++
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 4e120a2..22b53f6 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,6 @@
 # Makefile for QEMU.
 
-GENERATED_HEADERS = config-host.h trace.h qemu-options.def
+GENERATED_HEADERS = config-host.h trace.h qemu-options.def config-all-devices.h
 ifeq ($(TRACE_BACKEND),dtrace)
 GENERATED_HEADERS += trace-dtrace.h
 endif
@@ -77,6 +77,9 @@ config-host.h-timestamp: config-host.mak
 qemu-options.def: $(SRC_PATH)/qemu-options.hx
 	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $@")
 
+config-all-devices.h: config-all-devices.h-timestamp
+config-all-devices.h-timestamp: config-all-devices.mak
+
 SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS))
 
 subdir-%: $(GENERATED_HEADERS)
@@ -190,7 +193,7 @@ clean:
 
 distclean: clean
 	rm -f config-host.mak config-host.h* config-host.ld $(DOCS) qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
-	rm -f config-all-devices.mak
+	rm -f config-all-devices.mak config-all-devices.h*
 	rm -f roms/seabios/config.mak roms/vgabios/config.mak
 	rm -f qemu-doc.info qemu-doc.aux qemu-doc.cp qemu-doc.dvi qemu-doc.fn qemu-doc.info qemu-doc.ky qemu-doc.log qemu-doc.pdf qemu-doc.pg qemu-doc.toc qemu-doc.tp qemu-doc.vr
 	rm -f qemu-tech.info qemu-tech.aux qemu-tech.cp qemu-tech.dvi qemu-tech.fn qemu-tech.info qemu-tech.ky qemu-tech.log qemu-tech.pdf qemu-tech.pg qemu-tech.toc qemu-tech.tp qemu-tech.vr
diff --git a/Makefile.target b/Makefile.target
index 2800f47..03fc486 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -1,6 +1,6 @@
 # -*- Mode: makefile -*-
 
-GENERATED_HEADERS = config-target.h
+GENERATED_HEADERS = config-target.h config-devices.h
 CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
 
 include ../config-host.mak
diff --git a/config.h b/config.h
index e20f786..07d79d4 100644
--- a/config.h
+++ b/config.h
@@ -1,2 +1,13 @@
+
 #include "config-host.h"
 #include "config-target.h"
+
+/* We want to include different config files for specific targets
+   And for the common library.  They need a different name because
+   we don't want to rely in paths */
+
+#if defined(NEED_CPU_H)
+#include "config-devices.h"
+#else
+#include "config-all-devices.h"
+#endif
-- 
1.7.3.2

[Qemu-devel] [PATCH 2/4] skip pci_vmsvga_init() calls if CONFIG_VMWARE_VGA is disabled

[Eduardo Habkost]

I was planning to add the check for CONFIG_VMWARE to the command-line
parsing code in vl.c, but vl.c is not built by Makefile.target, so we
can't test for a per-target config option there.

It is not the best solution, but it is better than simply having a
CONFIG_VMWARE_VGA option that doesn't work and can't be disabled. I
don't see a good way to implement it that wouldn't involve heavily
refactoring completely the '-vga' option parsing code.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/mips_malta.c |    4 ++++
 hw/pc.c         |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 6be8aa7..b4164a0 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -975,7 +975,11 @@ void mips_malta_init (ram_addr_t ram_size,
     if (cirrus_vga_enabled) {
         pci_cirrus_vga_init(pci_bus);
     } else if (vmsvga_enabled) {
+#ifdef CONFIG_VMWARE_VGA
         pci_vmsvga_init(pci_bus);
+#else
+        fprintf(stderr, "%s: vmware_vga support is not compiled in\n", __FUNCTION__);
+#endif /* CONFIG_VMWARE_VGA */
     } else if (std_vga_enabled) {
         pci_vga_init(pci_bus);
     }
diff --git a/hw/pc.c b/hw/pc.c
index 119c110..e872a7b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -987,10 +987,14 @@ void pc_vga_init(PCIBus *pci_bus)
             isa_cirrus_vga_init();
         }
     } else if (vmsvga_enabled) {
+#ifdef CONFIG_VMWARE_VGA
         if (pci_bus)
             pci_vmsvga_init(pci_bus);
         else
             fprintf(stderr, "%s: vmware_vga: no PCI bus\n", __FUNCTION__);
+#else
+        fprintf(stderr, "%s: vmware_vga support is not compiled in\n", __FUNCTION__);
+#endif /* CONFIG_VMWARE_VGA */
     } else if (std_vga_enabled) {
         if (pci_bus) {
             pci_vga_init(pci_bus);
-- 
1.7.3.2

[Qemu-devel] [PATCH 3/4] add CONFIG_VMMOUSE option

[Eduardo Habkost]

This will allow vmmouse to be disabled at build time if necessary.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 Makefile.target                    |    5 ++++-
 default-configs/i386-softmmu.mak   |    1 +
 default-configs/x86_64-softmmu.mak |    1 +
 hw/pc.c                            |    2 ++
 4 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 03fc486..c184af6 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -220,11 +220,14 @@ obj-$(CONFIG_KVM) += ivshmem.o
 obj-i386-y += vga.o
 obj-i386-y += mc146818rtc.o i8259.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
-obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o
+obj-i386-y += vmport.o hpet.o applesmc.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
 
+obj-i386-$(CONFIG_VMMOUSE) += vmmouse.o
+
+
 # shared objects
 obj-ppc-y = ppc.o
 obj-ppc-y += vga.o
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index ed00471..a4450bc 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -23,3 +23,4 @@ CONFIG_NE2000_ISA=y
 CONFIG_PIIX_PCI=y
 CONFIG_SOUND=y
 CONFIG_VIRTIO_PCI=y
+CONFIG_VMMOUSE=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 5183203..658b249 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -23,3 +23,4 @@ CONFIG_NE2000_ISA=y
 CONFIG_PIIX_PCI=y
 CONFIG_SOUND=y
 CONFIG_VIRTIO_PCI=y
+CONFIG_VMMOUSE=y
diff --git a/hw/pc.c b/hw/pc.c
index e872a7b..31ac075 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1059,7 +1059,9 @@ void pc_basic_device_init(qemu_irq *isa_irq,
     a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 1);
     i8042 = isa_create_simple("i8042");
     i8042_setup_a20_line(i8042, a20_line);
+#ifdef CONFIG_VMMOUSE
     vmmouse_init(i8042);
+#endif
 
     cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1);
     DMA_init(0, cpu_exit_irq);
-- 
1.7.3.2

[Qemu-devel] [PATCH 4/4] add CONFIG_VMPORT option

[Eduardo Habkost]

This allows vmport to be easily enabled or disabled at build time.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 Makefile.target                    |    3 ++-
 default-configs/i386-softmmu.mak   |    2 ++
 default-configs/x86_64-softmmu.mak |    2 ++
 hw/pc_piix.c                       |    2 ++
 4 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index c184af6..c55951a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -220,12 +220,13 @@ obj-$(CONFIG_KVM) += ivshmem.o
 obj-i386-y += vga.o
 obj-i386-y += mc146818rtc.o i8259.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
-obj-i386-y += vmport.o hpet.o applesmc.o
+obj-i386-y += hpet.o applesmc.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
 
 obj-i386-$(CONFIG_VMMOUSE) += vmmouse.o
+obj-i386-$(CONFIG_VMPORT) += vmport.o
 
 
 # shared objects
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index a4450bc..e195b47 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -23,4 +23,6 @@ CONFIG_NE2000_ISA=y
 CONFIG_PIIX_PCI=y
 CONFIG_SOUND=y
 CONFIG_VIRTIO_PCI=y
+CONFIG_VMPORT=y
+#NOTE: VMMOUSE depends on VMPORT
 CONFIG_VMMOUSE=y
diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 658b249..8782cb9 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -23,4 +23,6 @@ CONFIG_NE2000_ISA=y
 CONFIG_PIIX_PCI=y
 CONFIG_SOUND=y
 CONFIG_VIRTIO_PCI=y
+CONFIG_VMPORT=y
+#NOTE: VMMOUSE depends on VMPORT
 CONFIG_VMMOUSE=y
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 7d29d43..c0697e0 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -85,7 +85,9 @@ static void pc_init1(ram_addr_t ram_size,
 
     pc_cpus_init(cpu_model);
 
+#ifdef CONFIG_VMPORT
     vmport_init();
+#endif
 
     /* allocate ram and load rom/bios */
     pc_memory_init(ram_size, kernel_filename, kernel_cmdline, initrd_filename,
-- 
1.7.3.2

Re: [Qemu-devel] [PATCH 0/4] fix/add CONFIG_* options for VMWare device emulation

[Blue Swirl]

On Tue, Feb 1, 2011 at 4:53 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> Hi,
>
> This series makes CONFIG_VMWARE_VGA actually work (today we can't disable the
> option without getting a build error).
>
> It also add two new options: CONFIG_VMMOUSE and CONFIG_VMPORT, for vmmouse.o
> and vmport.o.

Nack, see the list archives for the discussion.

One way to solve this which would preserve the device model would be
to add stub devices. For example, hw/vmmouse-stub.c would be:
void *vmmouse_init(void *m)
{
    return NULL;
}

Re: [Qemu-devel] [PATCH 1/4] Add config-devices.h again

[Stefan Weil]

Am 01.02.2011 17:53, schrieb Eduardo Habkost:
> This reverts part of commit a992fe3d0fc185112677286f7a02204d8245b61e.
>
> We do have code that needs #ifdefs depending on the list of enabled devices,
> but currently that code breaks when we try to disable a feature that is enabled
> by default.
>
> For example, if we try to disable CONFIG_VMWARE_VGA, we get the following:
>
>     LINK  x86_64-softmmu/qemu-system-x86_64
>    pc.o: In function `pc_vga_init':
>    /home/ehabkost/pessoal/proj/virt/qemu/qemu/hw/pc.c:991: undefined reference to `pci_vmsvga_init'
>    collect2: ld returned 1 exit status
>    make[1]: *** [qemu-system-x86_64] Error 1
>    rm config-devices.h-timestamp
>    make: *** [subdir-x86_64-softmmu] Error 2
>
> config-devices.h will allow us to add an #ifdef to fix the above error, and
> other similar cases.
>
> Signed-off-by: Eduardo Habkost<ehabkost@redhat.com>
> ---
>   Makefile        |    7 +++++--
>   Makefile.target |    2 +-
>   config.h        |   11 +++++++++++
>   3 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 4e120a2..22b53f6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,6 +1,6 @@
>   # Makefile for QEMU.
>
> -GENERATED_HEADERS = config-host.h trace.h qemu-options.def
> +GENERATED_HEADERS = config-host.h trace.h qemu-options.def config-all-devices.h
>   ifeq ($(TRACE_BACKEND),dtrace)
>   GENERATED_HEADERS += trace-dtrace.h
>   endif
> @@ -77,6 +77,9 @@ config-host.h-timestamp: config-host.mak
>   qemu-options.def: $(SRC_PATH)/qemu-options.hx
>   	$(call quiet-command,sh $(SRC_PATH)/hxtool -h<  $<  >  $@,"  GEN   $@")
>
> +config-all-devices.h: config-all-devices.h-timestamp
> +config-all-devices.h-timestamp: config-all-devices.mak
> +
>   SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS))
>
>   subdir-%: $(GENERATED_HEADERS)
> @@ -190,7 +193,7 @@ clean:
>
>   distclean: clean
>   	rm -f config-host.mak config-host.h* config-host.ld $(DOCS) qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
> -	rm -f config-all-devices.mak
> +	rm -f config-all-devices.mak config-all-devices.h*
>   	rm -f roms/seabios/config.mak roms/vgabios/config.mak
>   	rm -f qemu-doc.info qemu-doc.aux qemu-doc.cp qemu-doc.dvi qemu-doc.fn qemu-doc.info qemu-doc.ky qemu-doc.log qemu-doc.pdf qemu-doc.pg qemu-doc.toc qemu-doc.tp qemu-doc.vr
>   	rm -f qemu-tech.info qemu-tech.aux qemu-tech.cp qemu-tech.dvi qemu-tech.fn qemu-tech.info qemu-tech.ky qemu-tech.log qemu-tech.pdf qemu-tech.pg qemu-tech.toc qemu-tech.tp qemu-tech.vr
> diff --git a/Makefile.target b/Makefile.target
> index 2800f47..03fc486 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -1,6 +1,6 @@
>   # -*- Mode: makefile -*-
>
> -GENERATED_HEADERS = config-target.h
> +GENERATED_HEADERS = config-target.h config-devices.h
>   CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
>
>   include ../config-host.mak
> diff --git a/config.h b/config.h
> index e20f786..07d79d4 100644
> --- a/config.h
> +++ b/config.h
> @@ -1,2 +1,13 @@
> +
>   #include "config-host.h"
>   #include "config-target.h"
> +
> +/* We want to include different config files for specific targets
> +   And for the common library.  They need a different name because
> +   we don't want to rely in paths */
>    

on paths?

> +
> +#if defined(NEED_CPU_H)
> +#include "config-devices.h"
> +#else
> +#include "config-all-devices.h"
> +#endif
>    

[Qemu-devel] Re: [PATCH 0/4] fix/add CONFIG_* options for VMWare device emulation

[Juan Quintela]

Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, Feb 1, 2011 at 4:53 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> Hi,
>>
>> This series makes CONFIG_VMWARE_VGA actually work (today we can't disable the
>> option without getting a build error).
>>
>> It also add two new options: CONFIG_VMMOUSE and CONFIG_VMPORT, for vmmouse.o
>> and vmport.o.
>
> Nack, see the list archives for the discussion.
>
> One way to solve this which would preserve the device model would be
> to add stub devices. For example, hw/vmmouse-stub.c would be:
> void *vmmouse_init(void *m)
> {
>     return NULL;
> }

I read the archives, and I still think that this is a stop backwards.  I
was (one of the much) proposing the feature.

If this features is requested each 3 months for different people, could
it be that really, really, our device model is not good enough?

We have three config files:
- config-host.mak
- config-target.mak
- config-devices.mak

First two have a .h associated file, last one don't have it. And people
is requesting it each 3 months.


Later, Juan.

Re: [Qemu-devel] Re: [PATCH 0/4] fix/add CONFIG_* options for VMWare device emulation

[David Ahern]

On 02/01/11 20:01, Juan Quintela wrote:
> Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Tue, Feb 1, 2011 at 4:53 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
>>> Hi,
>>>
>>> This series makes CONFIG_VMWARE_VGA actually work (today we can't disable the
>>> option without getting a build error).
>>>
>>> It also add two new options: CONFIG_VMMOUSE and CONFIG_VMPORT, for vmmouse.o
>>> and vmport.o.
>>
>> Nack, see the list archives for the discussion.
>>
>> One way to solve this which would preserve the device model would be
>> to add stub devices. For example, hw/vmmouse-stub.c would be:
>> void *vmmouse_init(void *m)
>> {
>>     return NULL;
>> }
> 
> I read the archives, and I still think that this is a stop backwards.  I
> was (one of the much) proposing the feature.
> 
> If this features is requested each 3 months for different people, could
> it be that really, really, our device model is not good enough?
> 
> We have three config files:
> - config-host.mak
> - config-target.mak
> - config-devices.mak
> 
> First two have a .h associated file, last one don't have it. And people
> is requesting it each 3 months.

And that's what I was going after a couple of weeks ago with
  http://www.mail-archive.com/qemu-devel@nongnu.org/msg51779.html

A small change to get the current design at least usable by adding the
device configs to the existing header files.

I understand the desire for a proper config architecture, but it is not
happening. Meanwhile people continue to try to use the existing broken
design, tripping over the same problems.

Massive changes in the wrong direction are thing;  resisting 2 liners
around init functions (#ifdef ... #endif) which fall in line with
existing code is a bit idealistic and not benefiting anyone.

David


> 
> 
> Later, Juan.
> 
> 

[Qemu-devel] Re: [PATCH 0/4] fix/add CONFIG_* options for VMWare device emulation

[Paolo Bonzini]

On 02/01/2011 07:10 PM, Blue Swirl wrote:
> One way to solve this which would preserve the device model would be
> to add stub devices. For example, hw/vmmouse-stub.c would be:
> void *vmmouse_init(void *m)
> {
>      return NULL;
> }

This is the wrong direction, unless you can somehow automatically 
generate the stub file.

The only other solution I can think of is weak symbols.  There are 
subtle differences between Windows and Linux weak symbols, but luckily 
we do not care about it.  See 
http://cygwin.com/ml/cygwin/2010-04/msg00281.html

Paolo

[Qemu-devel] Re: [PATCH 0/4] fix/add CONFIG_* options for VMWare device emulation

[Blue Swirl]

On Wed, Feb 2, 2011 at 7:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 02/01/2011 07:10 PM, Blue Swirl wrote:
>>
>> One way to solve this which would preserve the device model would be
>> to add stub devices. For example, hw/vmmouse-stub.c would be:
>> void *vmmouse_init(void *m)
>> {
>>     return NULL;
>> }
>
> This is the wrong direction, unless you can somehow automatically generate
> the stub file.
>
> The only other solution I can think of is weak symbols.  There are subtle
> differences between Windows and Linux weak symbols, but luckily we do not
> care about it.  See http://cygwin.com/ml/cygwin/2010-04/msg00281.html

Boards should create optional devices so that failure in the device
creation is handled more intelligently. For example, pci_vmsvga_init()
is just a wrapper (a bit useless one also) to pci_create_simple(bus,
-1, "vmware-svga"). pci_create_simple() in turn uses
qdev_init_nofail() which aborts if the device can't be created.
Instead, non-aborting version should be used so that if 'vmware-svga'
device can't be initialized, a fallback device (VGA) could be used or
error message printed.

In this case, vmmouse and vmport are not converted to qdev but that
should be done anyway.

[Qemu-devel] Re: [PATCH 0/4] fix/add CONFIG_* options for VMWare device emulation

[Eduardo Habkost]

On Wed, Feb 02, 2011 at 05:16:23PM +0000, Blue Swirl wrote:
> On Wed, Feb 2, 2011 at 7:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > On 02/01/2011 07:10 PM, Blue Swirl wrote:
> >>
> >> One way to solve this which would preserve the device model would be
> >> to add stub devices. For example, hw/vmmouse-stub.c would be:
> >> void *vmmouse_init(void *m)
> >> {
> >>     return NULL;
> >> }
> >
> > This is the wrong direction, unless you can somehow automatically generate
> > the stub file.
> >
> > The only other solution I can think of is weak symbols.  There are subtle
> > differences between Windows and Linux weak symbols, but luckily we do not
> > care about it.  See http://cygwin.com/ml/cygwin/2010-04/msg00281.html
> 
> Boards should create optional devices so that failure in the device
> creation is handled more intelligently. For example, pci_vmsvga_init()
> is just a wrapper (a bit useless one also) to pci_create_simple(bus,
> -1, "vmware-svga"). pci_create_simple() in turn uses
> qdev_init_nofail() which aborts if the device can't be created.
> Instead, non-aborting version should be used so that if 'vmware-svga'
> device can't be initialized, a fallback device (VGA) could be used or
> error message printed.
> 
> In this case, vmmouse and vmport are not converted to qdev but that
> should be done anyway.

In the meantime, what should we do? Keep the CONFIG_VMWARE_VGA option
broken and useless just to avoid an #ifdef?

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/4] Add config-devices.h again

[Eduardo Habkost]

On Tue, Feb 01, 2011 at 07:14:00PM +0100, Stefan Weil wrote:
> >+
> >  #include "config-host.h"
> >  #include "config-target.h"
> >+
> >+/* We want to include different config files for specific targets
> >+   And for the common library.  They need a different name because
> >+   we don't want to rely in paths */
> 
> on paths?

Is this a grammar question (english is not my native language, sorry),
or you didn't understand what the message above meant?

BTW, this comes from the original patch by Juan (I just restored it), so
he may have a better explanation for this comment.

-- 
Eduardo

[Qemu-devel] Re: [PATCH 0/4] fix/add CONFIG_* options for VMWare device emulation

[Blue Swirl]

On Wed, Feb 2, 2011 at 5:37 PM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, Feb 02, 2011 at 05:16:23PM +0000, Blue Swirl wrote:
>> On Wed, Feb 2, 2011 at 7:55 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> > On 02/01/2011 07:10 PM, Blue Swirl wrote:
>> >>
>> >> One way to solve this which would preserve the device model would be
>> >> to add stub devices. For example, hw/vmmouse-stub.c would be:
>> >> void *vmmouse_init(void *m)
>> >> {
>> >>     return NULL;
>> >> }
>> >
>> > This is the wrong direction, unless you can somehow automatically generate
>> > the stub file.
>> >
>> > The only other solution I can think of is weak symbols.  There are subtle
>> > differences between Windows and Linux weak symbols, but luckily we do not
>> > care about it.  See http://cygwin.com/ml/cygwin/2010-04/msg00281.html
>>
>> Boards should create optional devices so that failure in the device
>> creation is handled more intelligently. For example, pci_vmsvga_init()
>> is just a wrapper (a bit useless one also) to pci_create_simple(bus,
>> -1, "vmware-svga"). pci_create_simple() in turn uses
>> qdev_init_nofail() which aborts if the device can't be created.
>> Instead, non-aborting version should be used so that if 'vmware-svga'
>> device can't be initialized, a fallback device (VGA) could be used or
>> error message printed.
>>
>> In this case, vmmouse and vmport are not converted to qdev but that
>> should be done anyway.
>
> In the meantime, what should we do? Keep the CONFIG_VMWARE_VGA option
> broken and useless just to avoid an #ifdef?

Please try the patch set I just sent.

[Qemu-devel] Re: [PATCH 1/4] Add config-devices.h again

[Juan Quintela]

Stefan Weil <weil@mail.berlios.de> wrote:
> Am 01.02.2011 17:53, schrieb Eduardo Habkost:
>> This reverts part of commit a992fe3d0fc185112677286f7a02204d8245b61e.
>>
>> We do have code that needs #ifdefs depending on the list of enabled devices,
>> but currently that code breaks when we try to disable a feature that is enabled
>> by default.
>>
>> For example, if we try to disable CONFIG_VMWARE_VGA, we get the following:
>>
>>     LINK  x86_64-softmmu/qemu-system-x86_64
>>    pc.o: In function `pc_vga_init':
>>    /home/ehabkost/pessoal/proj/virt/qemu/qemu/hw/pc.c:991: undefined reference to `pci_vmsvga_init'
>>    collect2: ld returned 1 exit status
>>    make[1]: *** [qemu-system-x86_64] Error 1
>>    rm config-devices.h-timestamp
>>    make: *** [subdir-x86_64-softmmu] Error 2
>>
>> config-devices.h will allow us to add an #ifdef to fix the above error, and
>> other similar cases.
>>
>> Signed-off-by: Eduardo Habkost<ehabkost@redhat.com>
>> ---
>>   Makefile        |    7 +++++--
>>   Makefile.target |    2 +-
>>   config.h        |   11 +++++++++++
>>   3 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 4e120a2..22b53f6 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1,6 +1,6 @@
>>   # Makefile for QEMU.
>>
>> -GENERATED_HEADERS = config-host.h trace.h qemu-options.def
>> +GENERATED_HEADERS = config-host.h trace.h qemu-options.def config-all-devices.h
>>   ifeq ($(TRACE_BACKEND),dtrace)
>>   GENERATED_HEADERS += trace-dtrace.h
>>   endif
>> @@ -77,6 +77,9 @@ config-host.h-timestamp: config-host.mak
>>   qemu-options.def: $(SRC_PATH)/qemu-options.hx
>>   	$(call quiet-command,sh $(SRC_PATH)/hxtool -h<  $<  >  $@,"  GEN   $@")
>>
>> +config-all-devices.h: config-all-devices.h-timestamp
>> +config-all-devices.h-timestamp: config-all-devices.mak
>> +
>>   SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS))
>>
>>   subdir-%: $(GENERATED_HEADERS)
>> @@ -190,7 +193,7 @@ clean:
>>
>>   distclean: clean
>>   	rm -f config-host.mak config-host.h* config-host.ld $(DOCS) qemu-options.texi qemu-img-cmds.texi qemu-monitor.texi
>> -	rm -f config-all-devices.mak
>> +	rm -f config-all-devices.mak config-all-devices.h*
>>   	rm -f roms/seabios/config.mak roms/vgabios/config.mak
>>   	rm -f qemu-doc.info qemu-doc.aux qemu-doc.cp qemu-doc.dvi qemu-doc.fn qemu-doc.info qemu-doc.ky qemu-doc.log qemu-doc.pdf qemu-doc.pg qemu-doc.toc qemu-doc.tp qemu-doc.vr
>>   	rm -f qemu-tech.info qemu-tech.aux qemu-tech.cp qemu-tech.dvi qemu-tech.fn qemu-tech.info qemu-tech.ky qemu-tech.log qemu-tech.pdf qemu-tech.pg qemu-tech.toc qemu-tech.tp qemu-tech.vr
>> diff --git a/Makefile.target b/Makefile.target
>> index 2800f47..03fc486 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -1,6 +1,6 @@
>>   # -*- Mode: makefile -*-
>>
>> -GENERATED_HEADERS = config-target.h
>> +GENERATED_HEADERS = config-target.h config-devices.h
>>   CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
>>
>>   include ../config-host.mak
>> diff --git a/config.h b/config.h
>> index e20f786..07d79d4 100644
>> --- a/config.h
>> +++ b/config.h
>> @@ -1,2 +1,13 @@
>> +
>>   #include "config-host.h"
>>   #include "config-target.h"
>> +
>> +/* We want to include different config files for specific targets
>> +   And for the common library.  They need a different name because
>> +   we don't want to rely in paths */
>>    
>
> on paths?

yeap, sorry.