diff mbox

Add optional dump of default config file paths..

Message ID 4C0F4B66.3010106@redhat.com
State New
Headers show

Commit Message

john cooper June 9, 2010, 8:05 a.m. UTC 
This patch adds the ability to determine the build-configured
runtime "config file" paths from the command line.  After
support for cpu model definitions were added to the default
runtime "target-" config file, testing of this feature has
tripped over an unintentionally mis-installed config file
enough to indicate some help is needed resolving such issues.

As no general "verbose" flag is currently available, specifying
"-readconfig ?" on the command line will maintain the default
(config file) disposition but additionally emit diagnostic info.
This mode is optional, otherwise the existing startup behavior
is identical.

Signed-off-by: john cooper <john.cooper@redhat.com>
---

Comments

Anthony Liguori June 14, 2010, 5:01 p.m. UTC | #1 
On 06/09/2010 03:05 AM, john cooper wrote:
> This patch adds the ability to determine the build-configured
> runtime "config file" paths from the command line.  After
> support for cpu model definitions were added to the default
> runtime "target-" config file, testing of this feature has
> tripped over an unintentionally mis-installed config file
> enough to indicate some help is needed resolving such issues.
>
> As no general "verbose" flag is currently available, specifying
> "-readconfig ?" on the command line will maintain the default
> (config file) disposition but additionally emit diagnostic info.
> This mode is optional, otherwise the existing startup behavior
> is identical.
>    

I assume this is something requested by libvirt?  I'd prefer we support 
this via Daniel's capabilities patchset instead of adding yet another 
hidden help output and having libvirt parse that output.

Regards,

Anthony Liguori

> Signed-off-by: john cooper<john.cooper@redhat.com>
> ---
>
> diff --git a/qemu-config.c b/qemu-config.c
> index 5a4e61b..a490603 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -518,21 +518,29 @@ out:
>       return res;
>   }
>
> -int qemu_read_config_file(const char *filename)
> +/* attempt to open and parse config file, report problems if vflag
> + */
> +int qemu_read_config_file(const char *filename, int vflag)
>   {
>       FILE *f = fopen(filename, "r");
> -    int ret;
> +    int rv = 0;
>
>       if (f == NULL) {
> -        return -errno;
> +        rv = -errno;
>       }
> -
> -    ret = qemu_config_parse(f, vm_config_groups, filename);
> -    fclose(f);
> -
> -    if (ret == 0) {
> -        return 0;
> -    } else {
> -        return -EINVAL;
> +    else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
> +        rv = -EINVAL;
> +    }
> +    else if (vflag) {
> +        fprintf(stderr, "read config file %s\n", filename);
>       }
> +    if (f) {
> +        fclose(f);
> +    }
> +    if (rv&&  vflag) {
> +        fprintf(stderr, "can't read config file %s: %s\n",
> +                filename, strerror(-rv));
> +    }
> +    return rv;
>   }
> +
> diff --git a/qemu-config.h b/qemu-config.h
> index dca69d4..2e15556 100644
> --- a/qemu-config.h
> +++ b/qemu-config.h
> @@ -23,6 +23,6 @@ void qemu_add_globals(void);
>   void qemu_config_write(FILE *fp);
>   int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
>
> -int qemu_read_config_file(const char *filename);
> +int qemu_read_config_file(const char *filename, int vflag);
>
>   #endif /* QEMU_CONFIG_H */
> diff --git a/vl.c b/vl.c
> index 7121cd0..23c7276 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2582,6 +2582,7 @@ int main(int argc, char **argv, char **envp)
>   #endif
>       int show_vnc_port = 0;
>       int defconfig = 1;
> +    int defconfig_verbose = 0;	
>
>       error_set_progname(argv[0]);
>
> @@ -2657,6 +2658,10 @@ int main(int argc, char **argv, char **envp)
>               case QEMU_OPTION_nodefconfig:
>                   defconfig=0;
>                   break;
> +            case QEMU_OPTION_readconfig:
> +                if (!strcmp(optarg, "?"))
> +                    defconfig_verbose = 1;
> +                break;
>               }
>           }
>       }
> @@ -2664,12 +2669,14 @@ int main(int argc, char **argv, char **envp)
>       if (defconfig) {
>           int ret;
>
> -        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
> +        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
> +                                    defconfig_verbose);
>           if (ret<  0&&  ret != -ENOENT) {
>               exit(1);
>           }
>
> -        ret = qemu_read_config_file(arch_config_name);
> +        ret = qemu_read_config_file(arch_config_name,
> +                                    defconfig_verbose);
>           if (ret<  0&&  ret != -ENOENT) {
>               exit(1);
>           }
> @@ -3386,15 +3393,9 @@ int main(int argc, char **argv, char **envp)
>                   xen_mode = XEN_ATTACH;
>                   break;
>               case QEMU_OPTION_readconfig:
> -                {
> -                    int ret = qemu_read_config_file(optarg);
> -                    if (ret<  0) {
> -                        fprintf(stderr, "read config %s: %s\n", optarg,
> -                            strerror(-ret));
> -                        exit(1);
> -                    }
> -                    break;
> -                }
> +                if (!defconfig_verbose&&  qemu_read_config_file(optarg, 1)<  0)
> +                    exit(1);
> +                break;
>               case QEMU_OPTION_writeconfig:
>                   {
>                       FILE *fp;
>
>
Daniel P. Berrangé June 14, 2010, 5:07 p.m. UTC | #2 
On Mon, Jun 14, 2010 at 12:01:42PM -0500, Anthony Liguori wrote:
> On 06/09/2010 03:05 AM, john cooper wrote:
> >This patch adds the ability to determine the build-configured
> >runtime "config file" paths from the command line.  After
> >support for cpu model definitions were added to the default
> >runtime "target-" config file, testing of this feature has
> >tripped over an unintentionally mis-installed config file
> >enough to indicate some help is needed resolving such issues.
> >
> >As no general "verbose" flag is currently available, specifying
> >"-readconfig ?" on the command line will maintain the default
> >(config file) disposition but additionally emit diagnostic info.
> >This mode is optional, otherwise the existing startup behavior
> >is identical.
> >   
> 
> I assume this is something requested by libvirt?  I'd prefer we support 
> this via Daniel's capabilities patchset instead of adding yet another 
> hidden help output and having libvirt parse that output.

No, libvirt has no need for this patch. We now forcably disable
all default configs with -nodefconfig.

Regards,
Daniel
john cooper June 14, 2010, 5:59 p.m. UTC | #3 
Anthony Liguori wrote:
> On 06/09/2010 03:05 AM, john cooper wrote:
>> This patch adds the ability to determine the build-configured
>> runtime "config file" paths from the command line.  After
>> support for cpu model definitions were added to the default
>> runtime "target-" config file, testing of this feature has
>> tripped over an unintentionally mis-installed config file
>> enough to indicate some help is needed resolving such issues.
>>
>> As no general "verbose" flag is currently available, specifying
>> "-readconfig ?" on the command line will maintain the default
>> (config file) disposition but additionally emit diagnostic info.
>> This mode is optional, otherwise the existing startup behavior
>> is identical.
>>    
> 
> I assume this is something requested by libvirt?

Not requested by libvirt but rather intended for the qemu
CLI facing user.  The alternatives of trying to puzzle
out built-in config file paths via strace or strings when
config problems surface is rather awkward.  This also
seems a general need for test of config file related
functionality.

Thanks,

-john

> I'd prefer we support
> this via Daniel's capabilities patchset instead of adding yet another
> hidden help output and having libvirt parse that output.
> 
> Regards,
> 
> Anthony Liguori
> 
>> Signed-off-by: john cooper<john.cooper@redhat.com>
>> ---
>>
>> diff --git a/qemu-config.c b/qemu-config.c
>> index 5a4e61b..a490603 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -518,21 +518,29 @@ out:
>>       return res;
>>   }
>>
>> -int qemu_read_config_file(const char *filename)
>> +/* attempt to open and parse config file, report problems if vflag
>> + */
>> +int qemu_read_config_file(const char *filename, int vflag)
>>   {
>>       FILE *f = fopen(filename, "r");
>> -    int ret;
>> +    int rv = 0;
>>
>>       if (f == NULL) {
>> -        return -errno;
>> +        rv = -errno;
>>       }
>> -
>> -    ret = qemu_config_parse(f, vm_config_groups, filename);
>> -    fclose(f);
>> -
>> -    if (ret == 0) {
>> -        return 0;
>> -    } else {
>> -        return -EINVAL;
>> +    else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
>> +        rv = -EINVAL;
>> +    }
>> +    else if (vflag) {
>> +        fprintf(stderr, "read config file %s\n", filename);
>>       }
>> +    if (f) {
>> +        fclose(f);
>> +    }
>> +    if (rv&&  vflag) {
>> +        fprintf(stderr, "can't read config file %s: %s\n",
>> +                filename, strerror(-rv));
>> +    }
>> +    return rv;
>>   }
>> +
>> diff --git a/qemu-config.h b/qemu-config.h
>> index dca69d4..2e15556 100644
>> --- a/qemu-config.h
>> +++ b/qemu-config.h
>> @@ -23,6 +23,6 @@ void qemu_add_globals(void);
>>   void qemu_config_write(FILE *fp);
>>   int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char
>> *fname);
>>
>> -int qemu_read_config_file(const char *filename);
>> +int qemu_read_config_file(const char *filename, int vflag);
>>
>>   #endif /* QEMU_CONFIG_H */
>> diff --git a/vl.c b/vl.c
>> index 7121cd0..23c7276 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2582,6 +2582,7 @@ int main(int argc, char **argv, char **envp)
>>   #endif
>>       int show_vnc_port = 0;
>>       int defconfig = 1;
>> +    int defconfig_verbose = 0;   
>>
>>       error_set_progname(argv[0]);
>>
>> @@ -2657,6 +2658,10 @@ int main(int argc, char **argv, char **envp)
>>               case QEMU_OPTION_nodefconfig:
>>                   defconfig=0;
>>                   break;
>> +            case QEMU_OPTION_readconfig:
>> +                if (!strcmp(optarg, "?"))
>> +                    defconfig_verbose = 1;
>> +                break;
>>               }
>>           }
>>       }
>> @@ -2664,12 +2669,14 @@ int main(int argc, char **argv, char **envp)
>>       if (defconfig) {
>>           int ret;
>>
>> -        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
>> +        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
>> +                                    defconfig_verbose);
>>           if (ret<  0&&  ret != -ENOENT) {
>>               exit(1);
>>           }
>>
>> -        ret = qemu_read_config_file(arch_config_name);
>> +        ret = qemu_read_config_file(arch_config_name,
>> +                                    defconfig_verbose);
>>           if (ret<  0&&  ret != -ENOENT) {
>>               exit(1);
>>           }
>> @@ -3386,15 +3393,9 @@ int main(int argc, char **argv, char **envp)
>>                   xen_mode = XEN_ATTACH;
>>                   break;
>>               case QEMU_OPTION_readconfig:
>> -                {
>> -                    int ret = qemu_read_config_file(optarg);
>> -                    if (ret<  0) {
>> -                        fprintf(stderr, "read config %s: %s\n", optarg,
>> -                            strerror(-ret));
>> -                        exit(1);
>> -                    }
>> -                    break;
>> -                }
>> +                if (!defconfig_verbose&& 
>> qemu_read_config_file(optarg, 1)<  0)
>> +                    exit(1);
>> +                break;
>>               case QEMU_OPTION_writeconfig:
>>                   {
>>                       FILE *fp;
>>
>>    
>
Anthony Liguori June 14, 2010, 7:13 p.m. UTC | #4 
On 06/14/2010 12:59 PM, john cooper wrote:
> Anthony Liguori wrote:
>    
>> On 06/09/2010 03:05 AM, john cooper wrote:
>>      
>>> This patch adds the ability to determine the build-configured
>>> runtime "config file" paths from the command line.  After
>>> support for cpu model definitions were added to the default
>>> runtime "target-" config file, testing of this feature has
>>> tripped over an unintentionally mis-installed config file
>>> enough to indicate some help is needed resolving such issues.
>>>
>>> As no general "verbose" flag is currently available, specifying
>>> "-readconfig ?" on the command line will maintain the default
>>> (config file) disposition but additionally emit diagnostic info.
>>> This mode is optional, otherwise the existing startup behavior
>>> is identical.
>>>
>>>        
>> I assume this is something requested by libvirt?
>>      
> Not requested by libvirt but rather intended for the qemu
> CLI facing user.  The alternatives of trying to puzzle
> out built-in config file paths via strace or strings when
> config problems surface is rather awkward.  This also
> seems a general need for test of config file related
> functionality.
>    

I wouldn't mind spitting out the ./configure line in qemu -version like 
gcc does.  That has the benefit of being a bit more obviously discoverable.

Regards,

Anthony Liguori

> Thanks,
>
> -john
>
>    
>> I'd prefer we support
>> this via Daniel's capabilities patchset instead of adding yet another
>> hidden help output and having libvirt parse that output.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>      
>>> Signed-off-by: john cooper<john.cooper@redhat.com>
>>> ---
>>>
>>> diff --git a/qemu-config.c b/qemu-config.c
>>> index 5a4e61b..a490603 100644
>>> --- a/qemu-config.c
>>> +++ b/qemu-config.c
>>> @@ -518,21 +518,29 @@ out:
>>>        return res;
>>>    }
>>>
>>> -int qemu_read_config_file(const char *filename)
>>> +/* attempt to open and parse config file, report problems if vflag
>>> + */
>>> +int qemu_read_config_file(const char *filename, int vflag)
>>>    {
>>>        FILE *f = fopen(filename, "r");
>>> -    int ret;
>>> +    int rv = 0;
>>>
>>>        if (f == NULL) {
>>> -        return -errno;
>>> +        rv = -errno;
>>>        }
>>> -
>>> -    ret = qemu_config_parse(f, vm_config_groups, filename);
>>> -    fclose(f);
>>> -
>>> -    if (ret == 0) {
>>> -        return 0;
>>> -    } else {
>>> -        return -EINVAL;
>>> +    else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
>>> +        rv = -EINVAL;
>>> +    }
>>> +    else if (vflag) {
>>> +        fprintf(stderr, "read config file %s\n", filename);
>>>        }
>>> +    if (f) {
>>> +        fclose(f);
>>> +    }
>>> +    if (rv&&   vflag) {
>>> +        fprintf(stderr, "can't read config file %s: %s\n",
>>> +                filename, strerror(-rv));
>>> +    }
>>> +    return rv;
>>>    }
>>> +
>>> diff --git a/qemu-config.h b/qemu-config.h
>>> index dca69d4..2e15556 100644
>>> --- a/qemu-config.h
>>> +++ b/qemu-config.h
>>> @@ -23,6 +23,6 @@ void qemu_add_globals(void);
>>>    void qemu_config_write(FILE *fp);
>>>    int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char
>>> *fname);
>>>
>>> -int qemu_read_config_file(const char *filename);
>>> +int qemu_read_config_file(const char *filename, int vflag);
>>>
>>>    #endif /* QEMU_CONFIG_H */
>>> diff --git a/vl.c b/vl.c
>>> index 7121cd0..23c7276 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -2582,6 +2582,7 @@ int main(int argc, char **argv, char **envp)
>>>    #endif
>>>        int show_vnc_port = 0;
>>>        int defconfig = 1;
>>> +    int defconfig_verbose = 0;
>>>
>>>        error_set_progname(argv[0]);
>>>
>>> @@ -2657,6 +2658,10 @@ int main(int argc, char **argv, char **envp)
>>>                case QEMU_OPTION_nodefconfig:
>>>                    defconfig=0;
>>>                    break;
>>> +            case QEMU_OPTION_readconfig:
>>> +                if (!strcmp(optarg, "?"))
>>> +                    defconfig_verbose = 1;
>>> +                break;
>>>                }
>>>            }
>>>        }
>>> @@ -2664,12 +2669,14 @@ int main(int argc, char **argv, char **envp)
>>>        if (defconfig) {
>>>            int ret;
>>>
>>> -        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
>>> +        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
>>> +                                    defconfig_verbose);
>>>            if (ret<   0&&   ret != -ENOENT) {
>>>                exit(1);
>>>            }
>>>
>>> -        ret = qemu_read_config_file(arch_config_name);
>>> +        ret = qemu_read_config_file(arch_config_name,
>>> +                                    defconfig_verbose);
>>>            if (ret<   0&&   ret != -ENOENT) {
>>>                exit(1);
>>>            }
>>> @@ -3386,15 +3393,9 @@ int main(int argc, char **argv, char **envp)
>>>                    xen_mode = XEN_ATTACH;
>>>                    break;
>>>                case QEMU_OPTION_readconfig:
>>> -                {
>>> -                    int ret = qemu_read_config_file(optarg);
>>> -                    if (ret<   0) {
>>> -                        fprintf(stderr, "read config %s: %s\n", optarg,
>>> -                            strerror(-ret));
>>> -                        exit(1);
>>> -                    }
>>> -                    break;
>>> -                }
>>> +                if (!defconfig_verbose&&
>>> qemu_read_config_file(optarg, 1)<   0)
>>> +                    exit(1);
>>> +                break;
>>>                case QEMU_OPTION_writeconfig:
>>>                    {
>>>                        FILE *fp;
>>>
>>>
>>>        
>>      
>
>
diff mbox

Patch

diff --git a/qemu-config.c b/qemu-config.c
index 5a4e61b..a490603 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -518,21 +518,29 @@  out:
     return res;
 }
 
-int qemu_read_config_file(const char *filename)
+/* attempt to open and parse config file, report problems if vflag
+ */
+int qemu_read_config_file(const char *filename, int vflag)
 {
     FILE *f = fopen(filename, "r");
-    int ret;
+    int rv = 0;
 
     if (f == NULL) {
-        return -errno;
+        rv = -errno;
     }
-
-    ret = qemu_config_parse(f, vm_config_groups, filename);
-    fclose(f);
-
-    if (ret == 0) {
-        return 0;
-    } else {
-        return -EINVAL;
+    else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
+        rv = -EINVAL;
+    }
+    else if (vflag) {
+        fprintf(stderr, "read config file %s\n", filename);
     }
+    if (f) {
+        fclose(f);
+    }
+    if (rv && vflag) {
+        fprintf(stderr, "can't read config file %s: %s\n",
+                filename, strerror(-rv));
+    }
+    return rv;
 }
+
diff --git a/qemu-config.h b/qemu-config.h
index dca69d4..2e15556 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -23,6 +23,6 @@  void qemu_add_globals(void);
 void qemu_config_write(FILE *fp);
 int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
 
-int qemu_read_config_file(const char *filename);
+int qemu_read_config_file(const char *filename, int vflag);
 
 #endif /* QEMU_CONFIG_H */
diff --git a/vl.c b/vl.c
index 7121cd0..23c7276 100644
--- a/vl.c
+++ b/vl.c
@@ -2582,6 +2582,7 @@  int main(int argc, char **argv, char **envp)
 #endif
     int show_vnc_port = 0;
     int defconfig = 1;
+    int defconfig_verbose = 0;	
 
     error_set_progname(argv[0]);
 
@@ -2657,6 +2658,10 @@  int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_nodefconfig:
                 defconfig=0;
                 break;
+            case QEMU_OPTION_readconfig:
+                if (!strcmp(optarg, "?"))
+                    defconfig_verbose = 1;
+                break;
             }
         }
     }
@@ -2664,12 +2669,14 @@  int main(int argc, char **argv, char **envp)
     if (defconfig) {
         int ret;
 
-        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
+        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
+                                    defconfig_verbose);
         if (ret < 0 && ret != -ENOENT) {
             exit(1);
         }
 
-        ret = qemu_read_config_file(arch_config_name);
+        ret = qemu_read_config_file(arch_config_name,
+                                    defconfig_verbose);
         if (ret < 0 && ret != -ENOENT) {
             exit(1);
         }
@@ -3386,15 +3393,9 @@  int main(int argc, char **argv, char **envp)
                 xen_mode = XEN_ATTACH;
                 break;
             case QEMU_OPTION_readconfig:
-                {
-                    int ret = qemu_read_config_file(optarg);
-                    if (ret < 0) {
-                        fprintf(stderr, "read config %s: %s\n", optarg,
-                            strerror(-ret));
-                        exit(1);
-                    }
-                    break;
-                }
+                if (!defconfig_verbose && qemu_read_config_file(optarg, 1) < 0)
+                    exit(1);
+                break;
             case QEMU_OPTION_writeconfig:
                 {
                     FILE *fp;