| Message ID | 1394462261-25581-2-git-send-email-wudxw@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Il 10/03/2014 15: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. > > To keep '-nodefaults' have the semantics of bare minimum, it adds a > check of 'has_defaults' in usb_enabled() to avoid that a USB controller > is added by '-nodefautls, -device VGA' implicitly. > > 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 +++- > vl.c | 3 ++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index bf46c38..5c9a154 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -781,13 +781,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/vl.c b/vl.c > index f8f7c00..e9d8baf 100644 > --- a/vl.c > +++ b/vl.c > @@ -932,7 +932,8 @@ static int parse_sandbox(QemuOpts *opts, void *opaque) > > bool usb_enabled(bool default_usb) > { > - return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", default_usb); > + return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", > + has_defaults && default_usb); > } > > #ifndef _WIN32 > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
On 03/10/2014 10:52 PM, Paolo Bonzini wrote: > Il 10/03/2014 15: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. >> >> To keep '-nodefaults' have the semantics of bare minimum, it adds a >> check of 'has_defaults' in usb_enabled() to avoid that a USB controller >> is added by '-nodefautls, -device VGA' implicitly. >> >> 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 +++- >> vl.c | 3 ++- >> 2 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index bf46c38..5c9a154 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -781,13 +781,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/vl.c b/vl.c >> index f8f7c00..e9d8baf 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -932,7 +932,8 @@ static int parse_sandbox(QemuOpts *opts, void >> *opaque) >> >> bool usb_enabled(bool default_usb) >> { >> - return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", >> default_usb); >> + return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", >> + has_defaults && default_usb); >> } >> >> #ifndef _WIN32 >> > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > Thanks a lot for the review. You contributed a lot on this patch, but I forgot to acknowledge it in commit message. I think re-sending a new patch to include it could cause some noise. May I ask the maintainer add the following line in the commit message when it's picked? Same on 1/2 in this series Thanks. Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Il 11/03/2014 10:52, Mark Wu ha scritto: >> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> > Thanks a lot for the review. You contributed a lot on this patch, but I > forgot to acknowledge it in commit message. > I think re-sending a new patch to include it could cause some noise. > May I ask the maintainer add the following line > in the commit message when it's picked? Same on 1/2 in this series Thanks. > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> No problem from me, I don't really care. It's kind of you anyway. :) Paolo
Am 11.03.2014 10:52, schrieb Mark Wu: > On 03/10/2014 10:52 PM, Paolo Bonzini wrote: >> Il 10/03/2014 15: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. >>> >>> To keep '-nodefaults' have the semantics of bare minimum, it adds a >>> check of 'has_defaults' in usb_enabled() to avoid that a USB controller >>> is added by '-nodefautls, -device VGA' implicitly. >>> >>> 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 +++- >>> vl.c | 3 ++- >>> 2 files changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>> index bf46c38..5c9a154 100644 >>> --- a/hw/ppc/spapr.c >>> +++ b/hw/ppc/spapr.c >>> @@ -781,13 +781,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/vl.c b/vl.c >>> index f8f7c00..e9d8baf 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -932,7 +932,8 @@ static int parse_sandbox(QemuOpts *opts, void >>> *opaque) >>> >>> bool usb_enabled(bool default_usb) >>> { >>> - return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", >>> default_usb); >>> + return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", >>> + has_defaults && default_usb); >>> } >>> >>> #ifndef _WIN32 >>> >> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> > Thanks a lot for the review. You contributed a lot on this patch, but I > forgot to acknowledge it in commit message. > I think re-sending a new patch to include it could cause some noise. > May I ask the maintainer add the following line > in the commit message when it's picked? Same on 1/2 in this series Thanks. > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Sorry, didn't spot this yesterday, applied to ppc-next (with typo fix): https://github.com/afaerber/qemu-cpu/commits/ppc-next Thanks, Andreas P.S. Mark, please remember to include a cover letter 0/2 next time.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index bf46c38..5c9a154 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -781,13 +781,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/vl.c b/vl.c index f8f7c00..e9d8baf 100644 --- a/vl.c +++ b/vl.c @@ -932,7 +932,8 @@ static int parse_sandbox(QemuOpts *opts, void *opaque) bool usb_enabled(bool default_usb) { - return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", default_usb); + return qemu_opt_get_bool(qemu_get_machine_opts(), "usb", + has_defaults && default_usb); } #ifndef _WIN32
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. To keep '-nodefaults' have the semantics of bare minimum, it adds a check of 'has_defaults' in usb_enabled() to avoid that a USB controller is added by '-nodefautls, -device VGA' implicitly. 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 +++- vl.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-)