| Message ID | 1394185045-24868-2-git-send-email-wudxw@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Il 07/03/2014 10:37, Mark Wu ha scritto: > Before spapr_vga_init will returned false if the vga is specified by > the command '-device VGA' because vga_interface_type was evaluated to > VGA_NONE. With the change in previous patch of this series, > spapr_vga_init should return true if it's told that the vga will be > initialized in flow of the generic devices initialization. > > This patch also makes two cleanups: > 1. skip initialization for VGA_NONE > 2. remove the useless 'break' I think that after this patch, "-nodefaults -device VGA" will get a USB controller that it didn't get before. Perhaps this in vl.c: bool usb_enabled(bool default_usb) { return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", default_usb); } should be bool usb_enabled(bool default_usb) { return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", !no_defaults && default_usb); } ? Thanks, Paolo > Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> > --- > hw/ppc/spapr.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 93d02c1..4d0ac56 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -765,13 +765,15 @@ static int spapr_vga_init(PCIBus *pci_bus) > { > switch (vga_interface_type) { > case VGA_NONE: > + return false; > + case VGA_DEVICE: > + return true; > case VGA_STD: > return pci_vga_init(pci_bus) != NULL; > default: > fprintf(stderr, "This vga model is not supported," > "currently it only supports -vga std\n"); > exit(0); > - break; > } > } > >
On 03/07/2014 08:43 PM, Paolo Bonzini wrote: > Il 07/03/2014 10:37, Mark Wu ha scritto: >> Before spapr_vga_init will returned false if the vga is specified by >> the command '-device VGA' because vga_interface_type was evaluated to >> VGA_NONE. With the change in previous patch of this series, >> spapr_vga_init should return true if it's told that the vga will be >> initialized in flow of the generic devices initialization. >> >> This patch also makes two cleanups: >> 1. skip initialization for VGA_NONE >> 2. remove the useless 'break' > > I think that after this patch, "-nodefaults -device VGA" will get a USB > controller that it didn't get before. I suspect what was meant by "the machine not aware of the graphics device" is that the guest won't work with VGA and without keyboard (default console will be vga + keyboard and not serial) which is USB and this is why the patch is trying to add USB. > > Perhaps this in vl.c: > > bool usb_enabled(bool default_usb) > { > return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", default_usb); > } > > should be > > bool usb_enabled(bool default_usb) > { > return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", > !no_defaults && default_usb); > } > > ? > > Thanks, > > Paolo > >> Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 93d02c1..4d0ac56 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -765,13 +765,15 @@ static int spapr_vga_init(PCIBus *pci_bus) >> { >> switch (vga_interface_type) { >> case VGA_NONE: >> + return false; >> + case VGA_DEVICE: >> + return true; >> case VGA_STD: >> return pci_vga_init(pci_bus) != NULL; >> default: >> fprintf(stderr, "This vga model is not supported," >> "currently it only supports -vga std\n"); >> exit(0); >> - break; >> } >> } >> >> > >
Il 08/03/2014 14:26, Alexey Kardashevskiy ha scritto: >> > I think that after this patch, "-nodefaults -device VGA" will get a USB >> > controller that it didn't get before. > > I suspect what was meant by "the machine not aware of the graphics device" > is that the guest won't work with VGA and without keyboard (default > console will be vga + keyboard and not serial) which is USB and this is why > the patch is trying to add USB. But with -nodefaults QEMU should never be adding USB. Paolo
On 03/10/2014 05:07 AM, Paolo Bonzini wrote: > Il 08/03/2014 14:26, Alexey Kardashevskiy ha scritto: >>> > I think that after this patch, "-nodefaults -device VGA" will get a USB >>> > controller that it didn't get before. >> >> I suspect what was meant by "the machine not aware of the graphics device" >> is that the guest won't work with VGA and without keyboard (default >> console will be vga + keyboard and not serial) which is USB and this is why >> the patch is trying to add USB. > > But with -nodefaults QEMU should never be adding USB. As I was told in this list before, even with -nodefaults, QEMU should not create a machine which is known for not working or not being supported. Having VGA and not having any input device is kind of such a config, no?
Il 10/03/2014 13:23, Alexey Kardashevskiy ha scritto: > On 03/10/2014 05:07 AM, Paolo Bonzini wrote: >> Il 08/03/2014 14:26, Alexey Kardashevskiy ha scritto: >>>>> I think that after this patch, "-nodefaults -device VGA" will get a USB >>>>> controller that it didn't get before. >>> >>> I suspect what was meant by "the machine not aware of the graphics device" >>> is that the guest won't work with VGA and without keyboard (default >>> console will be vga + keyboard and not serial) which is USB and this is why >>> the patch is trying to add USB. >> >> But with -nodefaults QEMU should never be adding USB. > > As I was told in this list before, even with -nodefaults, QEMU should not > create a machine which is known for not working or not being supported. > Having VGA and not having any input device is kind of such a config, no? -nodefaults is exactly the opposite of that: no magic whatsoever. No VGA, no serial, nothing. And especially, with -nodefaults adding a VGA means just that: adding a VGA. Paolo
On 03/10/2014 11:26 PM, Paolo Bonzini wrote: > Il 10/03/2014 13:23, Alexey Kardashevskiy ha scritto: >> On 03/10/2014 05:07 AM, Paolo Bonzini wrote: >>> Il 08/03/2014 14:26, Alexey Kardashevskiy ha scritto: >>>>>> I think that after this patch, "-nodefaults -device VGA" will get a USB >>>>>> controller that it didn't get before. >>>> >>>> I suspect what was meant by "the machine not aware of the graphics device" >>>> is that the guest won't work with VGA and without keyboard (default >>>> console will be vga + keyboard and not serial) which is USB and this is >>>> why >>>> the patch is trying to add USB. >>> >>> But with -nodefaults QEMU should never be adding USB. >> >> As I was told in this list before, even with -nodefaults, QEMU should not >> create a machine which is known for not working or not being supported. >> Having VGA and not having any input device is kind of such a config, no? > > -nodefaults is exactly the opposite of that: no magic whatsoever. No VGA, > no serial, nothing. qemu-system-x86_64 -enable-kvm -nographic -nodefaults -monitor stdio "info qtree" shows a whole bunch of devices like "i440FX-pcihost", "isa-fdc", "piix3-ide", "vmmouse", "vmport" (what are the last two?). I was told here that 8042 is to emulate A20, ok, but others - I do not really understand. "q35" is bit different than "pc" but not smaller. The point was made that there is no point in emulating a machine which does not exist in the real world. Has it changed recently? > And especially, with -nodefaults adding a VGA means just that: adding a VGA. Usual issue - libvirt expects keyboard with VGA and x86 provides this as it always has keyboard. PPC does not have such default.
Il 10/03/2014 13:45, Alexey Kardashevskiy ha scritto: >>> As I was told in this list before, even with -nodefaults, QEMU should not >>> create a machine which is known for not working or not being supported. >>> Having VGA and not having any input device is kind of such a config, no? >> >> -nodefaults is exactly the opposite of that: no magic whatsoever. No VGA, >> no serial, nothing. > > qemu-system-x86_64 -enable-kvm -nographic -nodefaults -monitor stdio > > "info qtree" shows a whole bunch of devices like "i440FX-pcihost", > "isa-fdc", "piix3-ide", "vmmouse", "vmport" (what are the last two?). I was > told here that 8042 is to emulate A20, ok, but others - I do not really > understand. "q35" is bit different than "pc" but not smaller. The point was > made that there is no point in emulating a machine which does not exist in > the real world. Has it changed recently? -nodefaults should be the bare minimum that is necessary for the VM firmware to work. On x86, isa-fdc/vmmouse/vmport are there only for backwards compatibility reasons. >> And especially, with -nodefaults adding a VGA means just that: adding a VGA. > > Usual issue - libvirt expects keyboard with VGA and x86 provides this as it > always has keyboard. PPC does not have such default. I don't think it's even a libvirt bug. Then in the libvirt XML you should have: <devices> <input type='keyboard' bus='usb'/> </devices> explicitly. If libvirt uses -nodefaults, it's up to virt-manager or oVirt or whatever management layer you're using to add all the necessary controllers (USB in this case) and devices (keyboard) for the machine to be usable. Paolo
On 03/07/2014 05:43 PM, Paolo Bonzini wrote: > Il 07/03/2014 10:37, Mark Wu ha scritto: >> Before spapr_vga_init will returned false if the vga is specified by >> the command '-device VGA' because vga_interface_type was evaluated to >> VGA_NONE. With the change in previous patch of this series, >> spapr_vga_init should return true if it's told that the vga will be >> initialized in flow of the generic devices initialization. >> >> This patch also makes two cleanups: >> 1. skip initialization for VGA_NONE >> 2. remove the useless 'break' > I think that after this patch, "-nodefaults -device VGA" will get a USB > controller that it didn't get before. > > Perhaps this in vl.c: > > bool usb_enabled(bool default_usb) > { > return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", default_usb); > } > > should be > > bool usb_enabled(bool default_usb) > { > return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", > !no_defaults && default_usb); > } Yes, it changes the semantics of 'nodefaults'. I will fix it in v3 and let the management app add usb controller. > ? > > Thanks, > > Paolo > >> Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 93d02c1..4d0ac56 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -765,13 +765,15 @@ static int spapr_vga_init(PCIBus *pci_bus) >> { >> switch (vga_interface_type) { >> case VGA_NONE: >> + return false; >> + case VGA_DEVICE: >> + return true; >> case VGA_STD: >> return pci_vga_init(pci_bus) != NULL; >> default: >> fprintf(stderr, "This vga model is not supported," >> "currently it only supports -vga std\n"); >> exit(0); >> - break; >> } >> } >> >>
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 93d02c1..4d0ac56 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -765,13 +765,15 @@ static int spapr_vga_init(PCIBus *pci_bus) { switch (vga_interface_type) { case VGA_NONE: + return false; + case VGA_DEVICE: + return true; case VGA_STD: return pci_vga_init(pci_bus) != NULL; default: fprintf(stderr, "This vga model is not supported," "currently it only supports -vga std\n"); exit(0); - break; } }
Before spapr_vga_init will returned false if the vga is specified by the command '-device VGA' because vga_interface_type was evaluated to VGA_NONE. With the change in previous patch of this series, spapr_vga_init should return true if it's told that the vga will be initialized in flow of the generic devices initialization. This patch also makes two cleanups: 1. skip initialization for VGA_NONE 2. remove the useless 'break' Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- hw/ppc/spapr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)