Patchwork [7/7] Only accept -no-hpet for i386 targets

login
register
mail settings
Submitter Jes Sorensen
Date June 15, 2010, 11:04 a.m.
Message ID <1276599879-22749-8-git-send-email-Jes.Sorensen@redhat.com>
Download mbox | patch
Permalink /patch/55642/
State New
Headers show

Comments

Jes Sorensen - June 15, 2010, 11:04 a.m.
From: Jes Sorensen <Jes.Sorensen@redhat.com>

Only accept -no-hpet for i386 targets

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 vl.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Jan Kiszka - June 15, 2010, 12:20 p.m.
Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Only accept -no-hpet for i386 targets
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  vl.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index e4a9aa9..1977409 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3328,9 +3328,11 @@ int main(int argc, char **argv, char **envp)
>              case QEMU_OPTION_no_acpi:
>                  acpi_enabled = 0;
>                  break;
> +#ifdef TARGET_I386
>              case QEMU_OPTION_no_hpet:
>                  no_hpet = 1;
>                  break;
> +#endif
>              case QEMU_OPTION_balloon:
>                  if (balloon_parse(optarg) < 0) {
>                      fprintf(stderr, "Unknown -balloon argument %s\n", optarg);

Upstream carries no-hpet unconditionally now. I guess you need to clean
up a different merge artifact.

Jan
Jes Sorensen - June 15, 2010, 12:28 p.m.
On 06/15/10 14:20, Jan Kiszka wrote:
>> @@ -3328,9 +3328,11 @@ int main(int argc, char **argv, char **envp)
>>              case QEMU_OPTION_no_acpi:
>>                  acpi_enabled = 0;
>>                  break;
>> +#ifdef TARGET_I386
>>              case QEMU_OPTION_no_hpet:
>>                  no_hpet = 1;
>>                  break;
>> +#endif
>>              case QEMU_OPTION_balloon:
>>                  if (balloon_parse(optarg) < 0) {
>>                      fprintf(stderr, "Unknown -balloon argument %s\n", optarg);
> 
> Upstream carries no-hpet unconditionally now. I guess you need to clean
> up a different merge artifact.

Upstream seems to have removed the TARGET_I386 around 'int no_hpet = 0;'
but to be honest, I think the correct solution is to not expose no_hpet
to non x86 targets in upstream.

Cheers,
Jes
Jan Kiszka - June 15, 2010, 12:37 p.m.
Jes Sorensen wrote:
> On 06/15/10 14:20, Jan Kiszka wrote:
>>> @@ -3328,9 +3328,11 @@ int main(int argc, char **argv, char **envp)
>>>              case QEMU_OPTION_no_acpi:
>>>                  acpi_enabled = 0;
>>>                  break;
>>> +#ifdef TARGET_I386
>>>              case QEMU_OPTION_no_hpet:
>>>                  no_hpet = 1;
>>>                  break;
>>> +#endif
>>>              case QEMU_OPTION_balloon:
>>>                  if (balloon_parse(optarg) < 0) {
>>>                      fprintf(stderr, "Unknown -balloon argument %s\n", optarg);
>> Upstream carries no-hpet unconditionally now. I guess you need to clean
>> up a different merge artifact.
> 
> Upstream seems to have removed the TARGET_I386 around 'int no_hpet = 0;'
> but to be honest, I think the correct solution is to not expose no_hpet
> to non x86 targets in upstream.

Then I would suggest to address this upstream first. The goal should be
to keep the diff low.

Jan
Jes Sorensen - June 15, 2010, 12:44 p.m.
On 06/15/10 14:37, Jan Kiszka wrote:
>> Upstream seems to have removed the TARGET_I386 around 'int no_hpet = 0;'
>> but to be honest, I think the correct solution is to not expose no_hpet
>> to non x86 targets in upstream.
> 
> Then I would suggest to address this upstream first. The goal should be
> to keep the diff low.

I agree, I'll send out an updated diff in a minute to make qemu-kvm
closer to qemu and then look at option exposure in qemu afterwards.

Cheers,
Jes
Paolo Bonzini - June 15, 2010, 1:38 p.m.
On 06/15/2010 01:04 PM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> Only accept -no-hpet for i386 targets
>
> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
> ---
>   vl.c |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index e4a9aa9..1977409 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3328,9 +3328,11 @@ int main(int argc, char **argv, char **envp)
>               case QEMU_OPTION_no_acpi:
>                   acpi_enabled = 0;
>                   break;
> +#ifdef TARGET_I386
>               case QEMU_OPTION_no_hpet:
>                   no_hpet = 1;
>                   break;
> +#endif
>               case QEMU_OPTION_balloon:
>                   if (balloon_parse(optarg)<  0) {
>                       fprintf(stderr, "Unknown -balloon argument %s\n", optarg);

This can alternatively be made to match upstream by just removing the 
#ifdef on no_hpet's declaration.

Not saying that -no-hpet makes any sense on non-i386 targets, mind, but 
it seems pointless to deviate from upstream.

Paolo
Jes Sorensen - June 15, 2010, 1:42 p.m.
On 06/15/10 15:38, Paolo Bonzini wrote:
> This can alternatively be made to match upstream by just removing the
> #ifdef on no_hpet's declaration.
> 
> Not saying that -no-hpet makes any sense on non-i386 targets, mind, but
> it seems pointless to deviate from upstream.
> 
> Paolo

Fixed in v2 of the patchset.

Jes

Patch

diff --git a/vl.c b/vl.c
index e4a9aa9..1977409 100644
--- a/vl.c
+++ b/vl.c
@@ -3328,9 +3328,11 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_no_acpi:
                 acpi_enabled = 0;
                 break;
+#ifdef TARGET_I386
             case QEMU_OPTION_no_hpet:
                 no_hpet = 1;
                 break;
+#endif
             case QEMU_OPTION_balloon:
                 if (balloon_parse(optarg) < 0) {
                     fprintf(stderr, "Unknown -balloon argument %s\n", optarg);