| Message ID | 1396544174-8904-1-git-send-email-nikunj@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 03.04.2014 18:56, schrieb Nikunj A Dadhania: > The following commit caused the regression in qemu-system-ppc64 > > 7effdaa3: spapr: Fix return value of vga initialization > d44229c5: Fix vga_interface_type for command line argument '-device VGA' > > Even when -nodefaults was provided, USB Keyboard and Mouse was added > to the machine. This breaks libvirt which uses -nodefaults and adds > the keyboard and mouse separately. The machine got 2 USB Keyboards > and 2 USB Mouses. > > CC: Paolo Bonzini <pbonzini@redhat.com> > CC: Mark Wu <wudxw@linux.vnet.ibm.com> > CC: Andreas Färber <afaerber@suse.de> > Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> > --- > hw/ppc/spapr.c | 6 +++++- > include/sysemu/sysemu.h | 1 + > vl.c | 10 ++++++++-- > 3 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index a11e121..3095626 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1328,7 +1328,11 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) > > if (usb_enabled(spapr->has_graphics)) { > pci_create_simple(phb->bus, -1, "pci-ohci"); > - if (spapr->has_graphics) { > + /* > + * For VGA/VNC, by default add usb keyboard/mouse, if -nodefaults > + * provided skip adding usb keyboard/mouse > + */ > + if (spapr->has_graphics && qemu_has_defaults()) { > usbdevice_create("keyboard"); > usbdevice_create("mouse"); > } > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index ba5c7f8..8e90ad0 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -200,6 +200,7 @@ DeviceState *get_boot_device(uint32_t position); > QemuOpts *qemu_get_machine_opts(void); > > bool usb_enabled(bool default_usb); > +bool qemu_has_defaults(void); > > extern QemuOptsList qemu_legacy_drive_opts; > extern QemuOptsList qemu_common_drive_opts; > diff --git a/vl.c b/vl.c > index 9975e5a..6bf37a2 100644 > --- a/vl.c > +++ b/vl.c > @@ -977,8 +977,14 @@ static void parse_name(QemuOpts *opts) > > bool usb_enabled(bool default_usb) > { > - return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", > - has_defaults && default_usb); > + return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", > + has_defaults && default_usb); > +} > + > +bool qemu_has_defaults(void) > +{ > + fprintf(stderr, "has_d %d\n", has_defaults); Debugging leftover surely? Cheers, Andreas > + return has_defaults; > } > > #ifndef _WIN32 >
Andreas Färber <afaerber@suse.de> writes: > Am 03.04.2014 18:56, schrieb Nikunj A Dadhania: >> The following commit caused the regression in qemu-system-ppc64 >> >> 7effdaa3: spapr: Fix return value of vga initialization >> d44229c5: Fix vga_interface_type for command line argument '-device VGA' >> >> Even when -nodefaults was provided, USB Keyboard and Mouse was added >> to the machine. This breaks libvirt which uses -nodefaults and adds >> the keyboard and mouse separately. The machine got 2 USB Keyboards >> and 2 USB Mouses. >> >> CC: Paolo Bonzini <pbonzini@redhat.com> >> CC: Mark Wu <wudxw@linux.vnet.ibm.com> >> CC: Andreas Färber <afaerber@suse.de> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr.c | 6 +++++- >> include/sysemu/sysemu.h | 1 + >> vl.c | 10 ++++++++-- >> 3 files changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index a11e121..3095626 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -1328,7 +1328,11 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) >> >> if (usb_enabled(spapr->has_graphics)) { >> pci_create_simple(phb->bus, -1, "pci-ohci"); >> - if (spapr->has_graphics) { >> + /* >> + * For VGA/VNC, by default add usb keyboard/mouse, if -nodefaults >> + * provided skip adding usb keyboard/mouse >> + */ >> + if (spapr->has_graphics && qemu_has_defaults()) { >> usbdevice_create("keyboard"); >> usbdevice_create("mouse"); >> } >> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h >> index ba5c7f8..8e90ad0 100644 >> --- a/include/sysemu/sysemu.h >> +++ b/include/sysemu/sysemu.h >> @@ -200,6 +200,7 @@ DeviceState *get_boot_device(uint32_t position); >> QemuOpts *qemu_get_machine_opts(void); >> >> bool usb_enabled(bool default_usb); >> +bool qemu_has_defaults(void); >> >> extern QemuOptsList qemu_legacy_drive_opts; >> extern QemuOptsList qemu_common_drive_opts; >> diff --git a/vl.c b/vl.c >> index 9975e5a..6bf37a2 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -977,8 +977,14 @@ static void parse_name(QemuOpts *opts) >> >> bool usb_enabled(bool default_usb) >> { >> - return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", >> - has_defaults && default_usb); >> + return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", >> + has_defaults && default_usb); >> +} >> + >> +bool qemu_has_defaults(void) >> +{ >> + fprintf(stderr, "has_d %d\n", has_defaults); > > Debugging leftover surely? Yes, you were too fast, i have sent updated mail just now. Thanks Nikunj
Il 03/04/2014 18:56, Nikunj A Dadhania ha scritto: > The following commit caused the regression in qemu-system-ppc64 > > 7effdaa3: spapr: Fix return value of vga initialization > d44229c5: Fix vga_interface_type for command line argument '-device VGA' > > Even when -nodefaults was provided, USB Keyboard and Mouse was added > to the machine. This breaks libvirt which uses -nodefaults and adds > the keyboard and mouse separately. The machine got 2 USB Keyboards > and 2 USB Mouses. Does libvirt use "-nodefaults -machine usb=true"? It should create the OHCI controller separately instead of using "-machine". Paolo
On 04/03/2014 12:01 PM, Paolo Bonzini wrote: > Il 03/04/2014 18:56, Nikunj A Dadhania ha scritto: >> The following commit caused the regression in qemu-system-ppc64 >> >> 7effdaa3: spapr: Fix return value of vga initialization >> d44229c5: Fix vga_interface_type for command line argument '-device VGA' >> >> Even when -nodefaults was provided, USB Keyboard and Mouse was added >> to the machine. This breaks libvirt which uses -nodefaults and adds >> the keyboard and mouse separately. The machine got 2 USB Keyboards >> and 2 USB Mouses. s/Mouses/Mice/ > > Does libvirt use "-nodefaults -machine usb=true"? It should create the > OHCI controller separately instead of using "-machine". At least with x86 emulation, libvirt prefers '-nodefaults -machine usb=off -device piix3-usb-uhci'; so I'm assuming libvirt knows how to directly create the USB devices it needs.
Paolo Bonzini <pbonzini@redhat.com> writes: > Il 03/04/2014 18:56, Nikunj A Dadhania ha scritto: >> The following commit caused the regression in qemu-system-ppc64 >> >> 7effdaa3: spapr: Fix return value of vga initialization >> d44229c5: Fix vga_interface_type for command line argument '-device VGA' >> >> Even when -nodefaults was provided, USB Keyboard and Mouse was added >> to the machine. This breaks libvirt which uses -nodefaults and adds >> the keyboard and mouse separately. The machine got 2 USB Keyboards >> and 2 USB Mouses. > > Does libvirt use "-nodefaults -machine usb=true"? It should create the > OHCI controller separately instead of using "-machine". I see it creating: -nodefaults -usb -device usb-kbd,id=input0 -device usb-mouse,id=input1 And -usb is translated to adding "pci-ohci" controller for spapr Regards, Nikunj
Il 03/04/2014 21:24, Nikunj A Dadhania ha scritto: >> > Does libvirt use "-nodefaults -machine usb=true"? It should create the >> > OHCI controller separately instead of using "-machine". > I see it creating: > > -nodefaults -usb -device usb-kbd,id=input0 -device usb-mouse,id=input1 > > And -usb is translated to adding "pci-ohci" controller for spapr Yeah, but with -nodefaults it's better to use -device directly. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > Il 03/04/2014 21:24, Nikunj A Dadhania ha scritto: >>> > Does libvirt use "-nodefaults -machine usb=true"? It should create the >>> > OHCI controller separately instead of using "-machine". >> I see it creating: >> >> -nodefaults -usb -device usb-kbd,id=input0 -device usb-mouse,id=input1 >> >> And -usb is translated to adding "pci-ohci" controller for spapr > > Yeah, but with -nodefaults it's better to use -device directly. I think there is special handling for this in vl.c bool usb_enabled(bool default_usb) { return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", has_defaults && default_usb); } And spapr.c uses this: if (usb_enabled(spapr->has_graphics)) { pci_create_simple(phb->bus, -1, "pci-ohci"); Regards Nikunj
Il 04/04/2014 07:28, Nikunj A Dadhania ha scritto: >>> >> >>> >> And -usb is translated to adding "pci-ohci" controller for spapr >> > >> > Yeah, but with -nodefaults it's better to use -device directly. > I think there is special handling for this in vl.c > > bool usb_enabled(bool default_usb) > { > return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", > has_defaults && default_usb); > } > > And spapr.c uses this: > > if (usb_enabled(spapr->has_graphics)) { > pci_create_simple(phb->bus, -1, "pci-ohci"); Sure. However, I'm saying that it's fine for spapr to make -usb mean "OHCI, and also keyboard & mouse if there is a VGA card in the system". If libvirt used "-device pci-ohci" unconditionally, it would fix the bug *and* it would ensure that the PCI slot of pci-ohci does not change due to some other unrelated reason. So I would rather have the fix in libvirt. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > Il 04/04/2014 07:28, Nikunj A Dadhania ha scritto: >>>> >> >>>> >> And -usb is translated to adding "pci-ohci" controller for spapr >>> > >>> > Yeah, but with -nodefaults it's better to use -device directly. >> I think there is special handling for this in vl.c >> >> bool usb_enabled(bool default_usb) >> { >> return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", >> has_defaults && default_usb); >> } >> >> And spapr.c uses this: >> >> if (usb_enabled(spapr->has_graphics)) { >> pci_create_simple(phb->bus, -1, "pci-ohci"); > > Sure. However, I'm saying that it's fine for spapr to make -usb mean > "OHCI, and also keyboard & mouse if there is a VGA card in the system". > > If libvirt used "-device pci-ohci" unconditionally, it would fix the bug > *and* it would ensure that the PCI slot of pci-ohci does not change due > to some other unrelated reason. So I would rather have the fix in libvirt. Sure, let me check this. Thanks, Nikunj
Il 04/04/2014 13:40, Nikunj A Dadhania ha scritto: >> > Sure. However, I'm saying that it's fine for spapr to make -usb mean >> > "OHCI, and also keyboard & mouse if there is a VGA card in the system". >> > >> > If libvirt used "-device pci-ohci" unconditionally, it would fix the bug >> > *and* it would ensure that the PCI slot of pci-ohci does not change due >> > to some other unrelated reason. So I would rather have the fix in libvirt. > Sure, let me check this. It is probably not that easy---libvirt needs to have an implicit USB <controller> element in the domain XML or something like that, and let the user's XML override the default USB controller. But it would be much better and much more consistent with x86. Paolo
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index a11e121..3095626 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1328,7 +1328,11 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) if (usb_enabled(spapr->has_graphics)) { pci_create_simple(phb->bus, -1, "pci-ohci"); - if (spapr->has_graphics) { + /* + * For VGA/VNC, by default add usb keyboard/mouse, if -nodefaults + * provided skip adding usb keyboard/mouse + */ + if (spapr->has_graphics && qemu_has_defaults()) { usbdevice_create("keyboard"); usbdevice_create("mouse"); } diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index ba5c7f8..8e90ad0 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -200,6 +200,7 @@ DeviceState *get_boot_device(uint32_t position); QemuOpts *qemu_get_machine_opts(void); bool usb_enabled(bool default_usb); +bool qemu_has_defaults(void); extern QemuOptsList qemu_legacy_drive_opts; extern QemuOptsList qemu_common_drive_opts; diff --git a/vl.c b/vl.c index 9975e5a..6bf37a2 100644 --- a/vl.c +++ b/vl.c @@ -977,8 +977,14 @@ static void parse_name(QemuOpts *opts) bool usb_enabled(bool default_usb) { - return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", - has_defaults && default_usb); + return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", + has_defaults && default_usb); +} + +bool qemu_has_defaults(void) +{ + fprintf(stderr, "has_d %d\n", has_defaults); + return has_defaults; } #ifndef _WIN32
The following commit caused the regression in qemu-system-ppc64 7effdaa3: spapr: Fix return value of vga initialization d44229c5: Fix vga_interface_type for command line argument '-device VGA' Even when -nodefaults was provided, USB Keyboard and Mouse was added to the machine. This breaks libvirt which uses -nodefaults and adds the keyboard and mouse separately. The machine got 2 USB Keyboards and 2 USB Mouses. CC: Paolo Bonzini <pbonzini@redhat.com> CC: Mark Wu <wudxw@linux.vnet.ibm.com> CC: Andreas Färber <afaerber@suse.de> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> --- hw/ppc/spapr.c | 6 +++++- include/sysemu/sysemu.h | 1 + vl.c | 10 ++++++++-- 3 files changed, 14 insertions(+), 3 deletions(-)