Patchwork [08/19] Convert RTC to use enumerations for configuration parameters

login
register
mail settings
Submitter Daniel P. Berrange
Date June 7, 2010, 2:42 p.m.
Message ID <1275921752-29420-9-git-send-email-berrange@redhat.com>
Download mbox | patch
Permalink /patch/54860/
State New
Headers show

Comments

Daniel P. Berrange - June 7, 2010, 2:42 p.m.
Convert the rtc clock and driftfix parameters to use enums for
configuration. This ensures strict validation at time of config
parsing.

Also fixes a bug in qemu-config.c where 'driftfix' was never
enabled because this is a target independant compliation unit,
thus cannot include config-target.h and thus never has the
TARGET_I386 symbol defined.

Ideally the 'base' parameter would be an enum too, but this
value is overloaded to also accept a pretty-printed start
date.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 hw/mc146818rtc.c |    8 ++++----
 qemu-config.c    |   27 +++++++++++++++++++++++----
 sysemu.h         |   20 +++++++++++++++++++-
 vl.c             |   34 ++++++++++------------------------
 4 files changed, 56 insertions(+), 33 deletions(-)
Luiz Capitulino - June 9, 2010, 7:54 p.m.
On Mon,  7 Jun 2010 15:42:21 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> Convert the rtc clock and driftfix parameters to use enums for
> configuration. This ensures strict validation at time of config
> parsing.
> 
> Also fixes a bug in qemu-config.c where 'driftfix' was never
> enabled because this is a target independant compliation unit,
> thus cannot include config-target.h and thus never has the
> TARGET_I386 symbol defined.
> 
> Ideally the 'base' parameter would be an enum too, but this
> value is overloaded to also accept a pretty-printed start
> date.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  hw/mc146818rtc.c |    8 ++++----
>  qemu-config.c    |   27 +++++++++++++++++++++++----
>  sysemu.h         |   20 +++++++++++++++++++-
>  vl.c             |   34 ++++++++++------------------------
>  4 files changed, 56 insertions(+), 33 deletions(-)
> 
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index c3e6a70..cbb072c 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -202,7 +202,7 @@ static void rtc_periodic_timer(void *opaque)
>      if (s->cmos_data[RTC_REG_B] & REG_B_PIE) {
>          s->cmos_data[RTC_REG_C] |= 0xc0;
>  #ifdef TARGET_I386
> -        if(rtc_td_hack) {
> +        if(rtc_driftfix == QEMU_RTC_DRIFTFIX_SLEW) {
>              if (s->irq_reinject_on_ack_count >= RTC_REINJECT_ON_ACK_COUNT)
>                  s->irq_reinject_on_ack_count = 0;		
>              apic_reset_irq_delivered();
> @@ -552,7 +552,7 @@ static int rtc_post_load(void *opaque, int version_id)
>      RTCState *s = opaque;
>  
>      if (version_id >= 2) {
> -        if (rtc_td_hack) {
> +        if (rtc_driftfix == QEMU_RTC_DRIFTFIX_SLEW) {
>              rtc_coalesced_timer_update(s);
>          }
>      }
> @@ -597,7 +597,7 @@ static void rtc_reset(void *opaque)
>      qemu_irq_lower(s->irq);
>  
>  #ifdef TARGET_I386
> -    if (rtc_td_hack)
> +    if (rtc_driftfix == QEMU_RTC_DRIFTFIX_SLEW)
>  	    s->irq_coalesced = 0;
>  #endif
>  }
> @@ -619,7 +619,7 @@ static int rtc_initfn(ISADevice *dev)
>  
>      s->periodic_timer = qemu_new_timer(rtc_clock, rtc_periodic_timer, s);
>  #ifdef TARGET_I386
> -    if (rtc_td_hack)
> +    if (rtc_driftfix == QEMU_RTC_DRIFTFIX_SLEW)
>          s->coalesced_timer =
>              qemu_new_timer(rtc_clock, rtc_coalesced_timer, s);
>  #endif
> diff --git a/qemu-config.c b/qemu-config.c
> index f656e6b..75cddc1 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -288,6 +288,11 @@ QemuOptsList qemu_net_opts = {
>      },
>  };
>  
> +QEMU_ENUM_IMPL(qemu_rtc_clock, QEMU_RTC_CLOCK_LAST,
> +	       "host", "guest");
> +QEMU_ENUM_IMPL(qemu_rtc_driftfix, QEMU_RTC_DRIFTFIX_LAST,
> +	       "none", "slew");
> +
>  QemuOptsList qemu_rtc_opts = {
>      .name = "rtc",
>      .head = QTAILQ_HEAD_INITIALIZER(qemu_rtc_opts.head),
> @@ -297,12 +302,26 @@ QemuOptsList qemu_rtc_opts = {
>              .type = QEMU_OPT_STRING,
>          },{
>              .name = "clock",
> -            .type = QEMU_OPT_STRING,
> -#ifdef TARGET_I386
> +            .type = QEMU_OPT_ENUM,
> +            .validate = {
> +                .optEnum = {
> +                    .to_string = qemu_rtc_clock_to_string,
> +                    .to_string_list = qemu_rtc_clock_to_string_list,
> +                    .from_string = qemu_rtc_clock_from_string,
> +                    .last = QEMU_RTC_CLOCK_LAST,
> +                },
> +            },
>          },{
>              .name = "driftfix",
> -            .type = QEMU_OPT_STRING,
> -#endif
> +            .type = QEMU_OPT_ENUM,
> +            .validate = {
> +                .optEnum = {
> +                    .to_string = qemu_rtc_driftfix_to_string,
> +                    .to_string_list = qemu_rtc_driftfix_to_string_list,
> +                    .from_string = qemu_rtc_driftfix_from_string,
> +                    .last = QEMU_RTC_DRIFTFIX_LAST,
> +                },
> +            },
>          },
>          { /* end if list */ }
>      },
> diff --git a/sysemu.h b/sysemu.h
> index f0c5eb8..4d39566 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -120,7 +120,6 @@ extern uint8_t irq0override;
>  extern DisplayType display_type;
>  extern const char *keyboard_layout;
>  extern int win2k_install_hack;
> -extern int rtc_td_hack;
>  extern int alt_grab;
>  extern int ctrl_grab;
>  extern int usb_enabled;
> @@ -133,7 +132,26 @@ extern int no_shutdown;
>  extern int semihosting_enabled;
>  extern int old_param;
>  extern int boot_menu;
> +
>  extern QEMUClock *rtc_clock;
> +extern int rtc_driftfix;
> +
> +enum {
> +    QEMU_RTC_CLOCK_HOST,
> +    QEMU_RTC_CLOCK_GUEST,
> +
> +    QEMU_RTC_CLOCK_LAST
> +};
> +QEMU_ENUM_DECL(qemu_rtc_clock);
> +
> +enum {
> +    QEMU_RTC_DRIFTFIX_NONE,
> +    QEMU_RTC_DRIFTFIX_SLEW,
> +
> +    QEMU_RTC_DRIFTFIX_LAST
> +};
> +QEMU_ENUM_DECL(qemu_rtc_driftfix);
> +
>  
>  #define MAX_NODES 64
>  extern int nb_numa_nodes;
> diff --git a/vl.c b/vl.c
> index 16491c4..0b38d62 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -201,7 +201,7 @@ CharDriverState *serial_hds[MAX_SERIAL_PORTS];
>  CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
>  CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
>  int win2k_install_hack = 0;
> -int rtc_td_hack = 0;
> +int rtc_driftfix = QEMU_RTC_DRIFTFIX_NONE;
>  int usb_enabled = 0;
>  int singlestep = 0;
>  int smp_cpus = 1;
> @@ -418,6 +418,9 @@ static void configure_rtc_date_offset(const char *startdate, int legacy)
>  static void configure_rtc(QemuOpts *opts)
>  {
>      const char *value;
> +    QEMUClock *clocks[] = { host_clock, vm_clock };
> +    verify_true(ARRAY_SIZE(clocks) == QEMU_RTC_CLOCK_LAST);

 This and the other verify_true() usage are generating the following
build error for me when debug is enabled:

cc1: warnings being treated as errors
/home/lcapitulino/src/qmp-unstable/vl.c: In function ‘configure_rtc’:
/home/lcapitulino/src/qmp-unstable/vl.c:422: error: statement with no effect
/home/lcapitulino/src/qmp-unstable/vl.c: In function ‘drive_init’:
/home/lcapitulino/src/qmp-unstable/vl.c:847: error: statement with no effect
make[1]: *** [vl.o] Error 1
make: *** [subdir-libhw64] Error 2


> +    int clock;
>  
>      value = qemu_opt_get(opts, "base");
>      if (value) {
> @@ -429,28 +432,11 @@ static void configure_rtc(QemuOpts *opts)
>              configure_rtc_date_offset(value, 0);
>          }
>      }
> -    value = qemu_opt_get(opts, "clock");
> -    if (value) {
> -        if (!strcmp(value, "host")) {
> -            rtc_clock = host_clock;
> -        } else if (!strcmp(value, "vm")) {
> -            rtc_clock = vm_clock;
> -        } else {
> -            fprintf(stderr, "qemu: invalid option value '%s'\n", value);
> -            exit(1);
> -        }
> -    }
> -    value = qemu_opt_get(opts, "driftfix");
> -    if (value) {
> -        if (!strcmp(value, "slew")) {
> -            rtc_td_hack = 1;
> -        } else if (!strcmp(value, "none")) {
> -            rtc_td_hack = 0;
> -        } else {
> -            fprintf(stderr, "qemu: invalid option value '%s'\n", value);
> -            exit(1);
> -        }
> -    }
> +
> +    clock = qemu_opt_get_enum(opts, "clock", QEMU_RTC_CLOCK_HOST);
> +    rtc_clock = clocks[clock];
> +
> +    rtc_driftfix = qemu_opt_get_enum(opts, "driftfix", QEMU_RTC_DRIFTFIX_NONE);
>  }
>  
>  /***********************************************************/
> @@ -3149,7 +3135,7 @@ int main(int argc, char **argv, char **envp)
>                  win2k_install_hack = 1;
>                  break;
>              case QEMU_OPTION_rtc_td_hack:
> -                rtc_td_hack = 1;
> +                rtc_driftfix = QEMU_RTC_DRIFTFIX_SLEW;
>                  break;
>              case QEMU_OPTION_acpitable:
>                  do_acpitable_option(optarg);

Patch

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index c3e6a70..cbb072c 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -202,7 +202,7 @@  static void rtc_periodic_timer(void *opaque)
     if (s->cmos_data[RTC_REG_B] & REG_B_PIE) {
         s->cmos_data[RTC_REG_C] |= 0xc0;
 #ifdef TARGET_I386
-        if(rtc_td_hack) {
+        if(rtc_driftfix == QEMU_RTC_DRIFTFIX_SLEW) {
             if (s->irq_reinject_on_ack_count >= RTC_REINJECT_ON_ACK_COUNT)
                 s->irq_reinject_on_ack_count = 0;		
             apic_reset_irq_delivered();
@@ -552,7 +552,7 @@  static int rtc_post_load(void *opaque, int version_id)
     RTCState *s = opaque;
 
     if (version_id >= 2) {
-        if (rtc_td_hack) {
+        if (rtc_driftfix == QEMU_RTC_DRIFTFIX_SLEW) {
             rtc_coalesced_timer_update(s);
         }
     }
@@ -597,7 +597,7 @@  static void rtc_reset(void *opaque)
     qemu_irq_lower(s->irq);
 
 #ifdef TARGET_I386
-    if (rtc_td_hack)
+    if (rtc_driftfix == QEMU_RTC_DRIFTFIX_SLEW)
 	    s->irq_coalesced = 0;
 #endif
 }
@@ -619,7 +619,7 @@  static int rtc_initfn(ISADevice *dev)
 
     s->periodic_timer = qemu_new_timer(rtc_clock, rtc_periodic_timer, s);
 #ifdef TARGET_I386
-    if (rtc_td_hack)
+    if (rtc_driftfix == QEMU_RTC_DRIFTFIX_SLEW)
         s->coalesced_timer =
             qemu_new_timer(rtc_clock, rtc_coalesced_timer, s);
 #endif
diff --git a/qemu-config.c b/qemu-config.c
index f656e6b..75cddc1 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -288,6 +288,11 @@  QemuOptsList qemu_net_opts = {
     },
 };
 
+QEMU_ENUM_IMPL(qemu_rtc_clock, QEMU_RTC_CLOCK_LAST,
+	       "host", "guest");
+QEMU_ENUM_IMPL(qemu_rtc_driftfix, QEMU_RTC_DRIFTFIX_LAST,
+	       "none", "slew");
+
 QemuOptsList qemu_rtc_opts = {
     .name = "rtc",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_rtc_opts.head),
@@ -297,12 +302,26 @@  QemuOptsList qemu_rtc_opts = {
             .type = QEMU_OPT_STRING,
         },{
             .name = "clock",
-            .type = QEMU_OPT_STRING,
-#ifdef TARGET_I386
+            .type = QEMU_OPT_ENUM,
+            .validate = {
+                .optEnum = {
+                    .to_string = qemu_rtc_clock_to_string,
+                    .to_string_list = qemu_rtc_clock_to_string_list,
+                    .from_string = qemu_rtc_clock_from_string,
+                    .last = QEMU_RTC_CLOCK_LAST,
+                },
+            },
         },{
             .name = "driftfix",
-            .type = QEMU_OPT_STRING,
-#endif
+            .type = QEMU_OPT_ENUM,
+            .validate = {
+                .optEnum = {
+                    .to_string = qemu_rtc_driftfix_to_string,
+                    .to_string_list = qemu_rtc_driftfix_to_string_list,
+                    .from_string = qemu_rtc_driftfix_from_string,
+                    .last = QEMU_RTC_DRIFTFIX_LAST,
+                },
+            },
         },
         { /* end if list */ }
     },
diff --git a/sysemu.h b/sysemu.h
index f0c5eb8..4d39566 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -120,7 +120,6 @@  extern uint8_t irq0override;
 extern DisplayType display_type;
 extern const char *keyboard_layout;
 extern int win2k_install_hack;
-extern int rtc_td_hack;
 extern int alt_grab;
 extern int ctrl_grab;
 extern int usb_enabled;
@@ -133,7 +132,26 @@  extern int no_shutdown;
 extern int semihosting_enabled;
 extern int old_param;
 extern int boot_menu;
+
 extern QEMUClock *rtc_clock;
+extern int rtc_driftfix;
+
+enum {
+    QEMU_RTC_CLOCK_HOST,
+    QEMU_RTC_CLOCK_GUEST,
+
+    QEMU_RTC_CLOCK_LAST
+};
+QEMU_ENUM_DECL(qemu_rtc_clock);
+
+enum {
+    QEMU_RTC_DRIFTFIX_NONE,
+    QEMU_RTC_DRIFTFIX_SLEW,
+
+    QEMU_RTC_DRIFTFIX_LAST
+};
+QEMU_ENUM_DECL(qemu_rtc_driftfix);
+
 
 #define MAX_NODES 64
 extern int nb_numa_nodes;
diff --git a/vl.c b/vl.c
index 16491c4..0b38d62 100644
--- a/vl.c
+++ b/vl.c
@@ -201,7 +201,7 @@  CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
 int win2k_install_hack = 0;
-int rtc_td_hack = 0;
+int rtc_driftfix = QEMU_RTC_DRIFTFIX_NONE;
 int usb_enabled = 0;
 int singlestep = 0;
 int smp_cpus = 1;
@@ -418,6 +418,9 @@  static void configure_rtc_date_offset(const char *startdate, int legacy)
 static void configure_rtc(QemuOpts *opts)
 {
     const char *value;
+    QEMUClock *clocks[] = { host_clock, vm_clock };
+    verify_true(ARRAY_SIZE(clocks) == QEMU_RTC_CLOCK_LAST);
+    int clock;
 
     value = qemu_opt_get(opts, "base");
     if (value) {
@@ -429,28 +432,11 @@  static void configure_rtc(QemuOpts *opts)
             configure_rtc_date_offset(value, 0);
         }
     }
-    value = qemu_opt_get(opts, "clock");
-    if (value) {
-        if (!strcmp(value, "host")) {
-            rtc_clock = host_clock;
-        } else if (!strcmp(value, "vm")) {
-            rtc_clock = vm_clock;
-        } else {
-            fprintf(stderr, "qemu: invalid option value '%s'\n", value);
-            exit(1);
-        }
-    }
-    value = qemu_opt_get(opts, "driftfix");
-    if (value) {
-        if (!strcmp(value, "slew")) {
-            rtc_td_hack = 1;
-        } else if (!strcmp(value, "none")) {
-            rtc_td_hack = 0;
-        } else {
-            fprintf(stderr, "qemu: invalid option value '%s'\n", value);
-            exit(1);
-        }
-    }
+
+    clock = qemu_opt_get_enum(opts, "clock", QEMU_RTC_CLOCK_HOST);
+    rtc_clock = clocks[clock];
+
+    rtc_driftfix = qemu_opt_get_enum(opts, "driftfix", QEMU_RTC_DRIFTFIX_NONE);
 }
 
 /***********************************************************/
@@ -3149,7 +3135,7 @@  int main(int argc, char **argv, char **envp)
                 win2k_install_hack = 1;
                 break;
             case QEMU_OPTION_rtc_td_hack:
-                rtc_td_hack = 1;
+                rtc_driftfix = QEMU_RTC_DRIFTFIX_SLEW;
                 break;
             case QEMU_OPTION_acpitable:
                 do_acpitable_option(optarg);