* [Qemu-devel] [PATCH v4 01/13] qga: misc spelling
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 02/13] qga: use exit() when parsing options marcandre.lureau
` (13 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/qapi-schema.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 18e3cc3..6b0bd16 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -793,7 +793,7 @@
# scheme. Refer to the documentation of the guest operating system
# in question to determine what is supported.
#
-# Note all guest operating systems will support use of the
+# Not all guest operating systems will support use of the
# @crypted flag, as they may require the clear-text password
#
# The @password parameter must always be base64 encoded before
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v4 02/13] qga: use exit() when parsing options
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 01/13] qga: misc spelling marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 03/13] qga: move string split in separate function marcandre.lureau
` (12 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The option parsing is going to be moved to a separate function,
use exit() consistently.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 791982e..10bb2f7 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -992,14 +992,14 @@ int main(int argc, char **argv)
break;
case 'V':
printf("QEMU Guest Agent %s\n", QEMU_VERSION);
- return 0;
+ exit(EXIT_SUCCESS);
case 'd':
daemonize = 1;
break;
case 'b': {
if (is_help_option(optarg)) {
qmp_for_each_command(ga_print_cmd, NULL);
- return 0;
+ exit(EXIT_SUCCESS);
}
for (j = 0, i = 0, len = strlen(optarg); i < len; i++) {
if (optarg[i] == ',') {
@@ -1027,36 +1027,36 @@ int main(int argc, char **argv)
NULL :
state_dir;
if (ga_install_vss_provider()) {
- return EXIT_FAILURE;
+ exit(EXIT_FAILURE);
}
if (ga_install_service(path, log_filepath, fixed_state_dir)) {
- return EXIT_FAILURE;
+ exit(EXIT_FAILURE);
}
- return 0;
+ exit(EXIT_SUCCESS);
} else if (strcmp(service, "uninstall") == 0) {
ga_uninstall_vss_provider();
- return ga_uninstall_service();
+ exit(ga_uninstall_service());
} else if (strcmp(service, "vss-install") == 0) {
if (ga_install_vss_provider()) {
- return EXIT_FAILURE;
+ exit(EXIT_FAILURE);
}
- return EXIT_SUCCESS;
+ exit(EXIT_SUCCESS);
} else if (strcmp(service, "vss-uninstall") == 0) {
ga_uninstall_vss_provider();
- return EXIT_SUCCESS;
+ exit(EXIT_SUCCESS);
} else {
printf("Unknown service command.\n");
- return EXIT_FAILURE;
+ exit(EXIT_FAILURE);
}
break;
#endif
case 'h':
usage(argv[0]);
- return 0;
+ exit(EXIT_SUCCESS);
case '?':
g_print("Unknown option, try '%s --help' for more information.\n",
argv[0]);
- return EXIT_FAILURE;
+ exit(EXIT_FAILURE);
}
}
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v4 03/13] qga: move string split in separate function
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 01/13] qga: misc spelling marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 02/13] qga: use exit() when parsing options marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-27 9:18 ` Denis V. Lunev
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 04/13] qga: make split_list() return allocated strings marcandre.lureau
` (11 subsequent siblings)
14 siblings, 1 reply; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The function is going to be reused in a later patch.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 10bb2f7..e75022c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -921,6 +921,26 @@ static void ga_print_cmd(QmpCommand *cmd, void *opaque)
printf("%s\n", qmp_command_name(cmd));
}
+static GList *split_list(gchar *str, const gchar separator)
+{
+ GList *list = NULL;
+ int i, j, len;
+
+ for (j = 0, i = 0, len = strlen(str); i < len; i++) {
+ if (str[i] == separator) {
+ str[i] = 0;
+ list = g_list_append(list, &str[j]);
+ j = i + 1;
+ }
+ }
+
+ if (j < i) {
+ list = g_list_append(list, &str[j]);
+ }
+
+ return list;
+}
+
int main(int argc, char **argv)
{
const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
@@ -953,7 +973,7 @@ int main(int argc, char **argv)
{ "statedir", 1, NULL, 't' },
{ NULL, 0, NULL, 0 }
};
- int opt_ind = 0, ch, daemonize = 0, i, j, len;
+ int opt_ind = 0, ch, daemonize = 0;
GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
GList *blacklist = NULL;
GAState *s;
@@ -1001,16 +1021,7 @@ int main(int argc, char **argv)
qmp_for_each_command(ga_print_cmd, NULL);
exit(EXIT_SUCCESS);
}
- for (j = 0, i = 0, len = strlen(optarg); i < len; i++) {
- if (optarg[i] == ',') {
- optarg[i] = 0;
- blacklist = g_list_append(blacklist, &optarg[j]);
- j = i + 1;
- }
- }
- if (j < i) {
- blacklist = g_list_append(blacklist, &optarg[j]);
- }
+ blacklist = g_list_concat(blacklist, split_list(optarg, ','));
break;
}
#ifdef _WIN32
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 03/13] qga: move string split in separate function
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 03/13] qga: move string split in separate function marcandre.lureau
@ 2015-08-27 9:18 ` Denis V. Lunev
0 siblings, 0 replies; 29+ messages in thread
From: Denis V. Lunev @ 2015-08-27 9:18 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: mdroth
On 08/27/2015 02:34 AM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The function is going to be reused in a later patch.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> qga/main.c | 33 ++++++++++++++++++++++-----------
> 1 file changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 10bb2f7..e75022c 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -921,6 +921,26 @@ static void ga_print_cmd(QmpCommand *cmd, void *opaque)
> printf("%s\n", qmp_command_name(cmd));
> }
>
> +static GList *split_list(gchar *str, const gchar separator)
> +{
> + GList *list = NULL;
> + int i, j, len;
> +
> + for (j = 0, i = 0, len = strlen(str); i < len; i++) {
> + if (str[i] == separator) {
> + str[i] = 0;
> + list = g_list_append(list, &str[j]);
> + j = i + 1;
> + }
> + }
> +
> + if (j < i) {
> + list = g_list_append(list, &str[j]);
> + }
> +
> + return list;
> +}
> +
> int main(int argc, char **argv)
> {
> const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
> @@ -953,7 +973,7 @@ int main(int argc, char **argv)
> { "statedir", 1, NULL, 't' },
> { NULL, 0, NULL, 0 }
> };
> - int opt_ind = 0, ch, daemonize = 0, i, j, len;
> + int opt_ind = 0, ch, daemonize = 0;
> GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
> GList *blacklist = NULL;
> GAState *s;
> @@ -1001,16 +1021,7 @@ int main(int argc, char **argv)
> qmp_for_each_command(ga_print_cmd, NULL);
> exit(EXIT_SUCCESS);
> }
> - for (j = 0, i = 0, len = strlen(optarg); i < len; i++) {
> - if (optarg[i] == ',') {
> - optarg[i] = 0;
> - blacklist = g_list_append(blacklist, &optarg[j]);
> - j = i + 1;
> - }
> - }
> - if (j < i) {
> - blacklist = g_list_append(blacklist, &optarg[j]);
> - }
> + blacklist = g_list_concat(blacklist, split_list(optarg, ','));
> break;
> }
> #ifdef _WIN32
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v4 04/13] qga: make split_list() return allocated strings
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (2 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 03/13] qga: move string split in separate function marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-27 9:13 ` Denis V. Lunev
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 05/13] qga: rename 'path' to 'channel_path' marcandre.lureau
` (10 subsequent siblings)
14 siblings, 1 reply; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
In order to avoid any confusion, let's allocate new strings when
splitting.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
qga/commands-posix.c | 6 +++---
qga/commands-win32.c | 4 ++--
qga/main.c | 22 +++++++++-------------
3 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 675f4b4..fc4fc72 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2454,7 +2454,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
char **p = (char **)list;
while (*p) {
- blacklist = g_list_append(blacklist, *p++);
+ blacklist = g_list_append(blacklist, g_strdup(*p++));
}
}
#endif
@@ -2468,13 +2468,13 @@ GList *ga_command_blacklist_init(GList *blacklist)
char **p = (char **)list;
while (*p) {
- blacklist = g_list_append(blacklist, *p++);
+ blacklist = g_list_append(blacklist, g_strdup(*p++));
}
}
#endif
#if !defined(CONFIG_FSTRIM)
- blacklist = g_list_append(blacklist, (char *)"guest-fstrim");
+ blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
#endif
return blacklist;
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 77d3c92..cbee186 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1306,7 +1306,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
char **p = (char **)list_unsupported;
while (*p) {
- blacklist = g_list_append(blacklist, *p++);
+ blacklist = g_list_append(blacklist, g_strdup(*p++));
}
if (!vss_init(true)) {
@@ -1317,7 +1317,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
p = (char **)list;
while (*p) {
- blacklist = g_list_append(blacklist, *p++);
+ blacklist = g_list_append(blacklist, g_strdup(*p++));
}
}
diff --git a/qga/main.c b/qga/main.c
index e75022c..a7df6c8 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -921,22 +921,17 @@ static void ga_print_cmd(QmpCommand *cmd, void *opaque)
printf("%s\n", qmp_command_name(cmd));
}
-static GList *split_list(gchar *str, const gchar separator)
+static GList *split_list(const gchar *str, const gchar *delim)
{
GList *list = NULL;
- int i, j, len;
+ int i;
+ gchar **strv;
- for (j = 0, i = 0, len = strlen(str); i < len; i++) {
- if (str[i] == separator) {
- str[i] = 0;
- list = g_list_append(list, &str[j]);
- j = i + 1;
- }
- }
-
- if (j < i) {
- list = g_list_append(list, &str[j]);
+ strv = g_strsplit(str, delim, -1);
+ for (i = 0; strv[i]; i++) {
+ list = g_list_prepend(list, strv[i]);
}
+ g_free(strv);
return list;
}
@@ -1021,7 +1016,7 @@ int main(int argc, char **argv)
qmp_for_each_command(ga_print_cmd, NULL);
exit(EXIT_SUCCESS);
}
- blacklist = g_list_concat(blacklist, split_list(optarg, ','));
+ blacklist = g_list_concat(blacklist, split_list(optarg, ","));
break;
}
#ifdef _WIN32
@@ -1201,6 +1196,7 @@ int main(int argc, char **argv)
}
#endif
+ g_list_free_full(ga_state->blacklist, g_free);
ga_command_state_cleanup_all(ga_state->command_state);
ga_channel_free(ga_state->channel);
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 04/13] qga: make split_list() return allocated strings
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 04/13] qga: make split_list() return allocated strings marcandre.lureau
@ 2015-08-27 9:13 ` Denis V. Lunev
2015-08-27 9:17 ` Denis V. Lunev
0 siblings, 1 reply; 29+ messages in thread
From: Denis V. Lunev @ 2015-08-27 9:13 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: mdroth
On 08/27/2015 02:34 AM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> In order to avoid any confusion, let's allocate new strings when
> splitting.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qga/commands-posix.c | 6 +++---
> qga/commands-win32.c | 4 ++--
> qga/main.c | 22 +++++++++-------------
> 3 files changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 675f4b4..fc4fc72 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2454,7 +2454,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
> char **p = (char **)list;
>
> while (*p) {
> - blacklist = g_list_append(blacklist, *p++);
> + blacklist = g_list_append(blacklist, g_strdup(*p++));
> }
> }
> #endif
> @@ -2468,13 +2468,13 @@ GList *ga_command_blacklist_init(GList *blacklist)
> char **p = (char **)list;
>
> while (*p) {
> - blacklist = g_list_append(blacklist, *p++);
> + blacklist = g_list_append(blacklist, g_strdup(*p++));
> }
> }
> #endif
>
> #if !defined(CONFIG_FSTRIM)
> - blacklist = g_list_append(blacklist, (char *)"guest-fstrim");
> + blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
> #endif
>
> return blacklist;
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 77d3c92..cbee186 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -1306,7 +1306,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
> char **p = (char **)list_unsupported;
>
> while (*p) {
> - blacklist = g_list_append(blacklist, *p++);
> + blacklist = g_list_append(blacklist, g_strdup(*p++));
> }
>
> if (!vss_init(true)) {
> @@ -1317,7 +1317,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
> p = (char **)list;
>
> while (*p) {
> - blacklist = g_list_append(blacklist, *p++);
> + blacklist = g_list_append(blacklist, g_strdup(*p++));
> }
> }
>
> diff --git a/qga/main.c b/qga/main.c
> index e75022c..a7df6c8 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -921,22 +921,17 @@ static void ga_print_cmd(QmpCommand *cmd, void *opaque)
> printf("%s\n", qmp_command_name(cmd));
> }
>
> -static GList *split_list(gchar *str, const gchar separator)
> +static GList *split_list(const gchar *str, const gchar *delim)
> {
> GList *list = NULL;
> - int i, j, len;
> + int i;
> + gchar **strv;
>
> - for (j = 0, i = 0, len = strlen(str); i < len; i++) {
> - if (str[i] == separator) {
> - str[i] = 0;
> - list = g_list_append(list, &str[j]);
> - j = i + 1;
> - }
> - }
> -
> - if (j < i) {
> - list = g_list_append(list, &str[j]);
> + strv = g_strsplit(str, delim, -1);
> + for (i = 0; strv[i]; i++) {
> + list = g_list_prepend(list, strv[i]);
> }
> + g_free(strv);
>
> return list;
> }
> @@ -1021,7 +1016,7 @@ int main(int argc, char **argv)
> qmp_for_each_command(ga_print_cmd, NULL);
> exit(EXIT_SUCCESS);
> }
> - blacklist = g_list_concat(blacklist, split_list(optarg, ','));
> + blacklist = g_list_concat(blacklist, split_list(optarg, ","));
> break;
> }
> #ifdef _WIN32
> @@ -1201,6 +1196,7 @@ int main(int argc, char **argv)
> }
> #endif
>
> + g_list_free_full(ga_state->blacklist, g_free);
> ga_command_state_cleanup_all(ga_state->command_state);
> ga_channel_free(ga_state->channel);
>
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 04/13] qga: make split_list() return allocated strings
2015-08-27 9:13 ` Denis V. Lunev
@ 2015-08-27 9:17 ` Denis V. Lunev
2015-08-27 14:30 ` Marc-André Lureau
0 siblings, 1 reply; 29+ messages in thread
From: Denis V. Lunev @ 2015-08-27 9:17 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: mdroth
On 08/27/2015 12:13 PM, Denis V. Lunev wrote:
> On 08/27/2015 02:34 AM, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> In order to avoid any confusion, let's allocate new strings when
>> splitting.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>> qga/commands-posix.c | 6 +++---
>> qga/commands-win32.c | 4 ++--
>> qga/main.c | 22 +++++++++-------------
>> 3 files changed, 14 insertions(+), 18 deletions(-)
>>
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index 675f4b4..fc4fc72 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -2454,7 +2454,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
>> char **p = (char **)list;
>> while (*p) {
>> - blacklist = g_list_append(blacklist, *p++);
>> + blacklist = g_list_append(blacklist, g_strdup(*p++));
>> }
>> }
>> #endif
>> @@ -2468,13 +2468,13 @@ GList *ga_command_blacklist_init(GList
>> *blacklist)
>> char **p = (char **)list;
>> while (*p) {
>> - blacklist = g_list_append(blacklist, *p++);
>> + blacklist = g_list_append(blacklist, g_strdup(*p++));
>> }
>> }
>> #endif
>> #if !defined(CONFIG_FSTRIM)
>> - blacklist = g_list_append(blacklist, (char *)"guest-fstrim");
>> + blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
>> #endif
>> return blacklist;
>> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
>> index 77d3c92..cbee186 100644
>> --- a/qga/commands-win32.c
>> +++ b/qga/commands-win32.c
>> @@ -1306,7 +1306,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
>> char **p = (char **)list_unsupported;
>> while (*p) {
>> - blacklist = g_list_append(blacklist, *p++);
>> + blacklist = g_list_append(blacklist, g_strdup(*p++));
>> }
>> if (!vss_init(true)) {
>> @@ -1317,7 +1317,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
>> p = (char **)list;
>> while (*p) {
>> - blacklist = g_list_append(blacklist, *p++);
>> + blacklist = g_list_append(blacklist, g_strdup(*p++));
>> }
>> }
>> diff --git a/qga/main.c b/qga/main.c
>> index e75022c..a7df6c8 100644
>> --- a/qga/main.c
>> +++ b/qga/main.c
>> @@ -921,22 +921,17 @@ static void ga_print_cmd(QmpCommand *cmd, void
>> *opaque)
>> printf("%s\n", qmp_command_name(cmd));
>> }
>> -static GList *split_list(gchar *str, const gchar separator)
>> +static GList *split_list(const gchar *str, const gchar *delim)
>> {
>> GList *list = NULL;
>> - int i, j, len;
>> + int i;
>> + gchar **strv;
>> - for (j = 0, i = 0, len = strlen(str); i < len; i++) {
>> - if (str[i] == separator) {
>> - str[i] = 0;
>> - list = g_list_append(list, &str[j]);
>> - j = i + 1;
>> - }
>> - }
>> -
>> - if (j < i) {
>> - list = g_list_append(list, &str[j]);
why not to
__str = g_strsplit(str, delim, -1);
>> + strv = g_strsplit(str, delim, -1);
>> + for (i = 0; strv[i]; i++) {
>> + list = g_list_prepend(list, strv[i]);
>> }
>> + g_free(strv);
g_free(__str);
This will remove all burden from callers.
You will be able to use const char * as argument too.
>> return list;
>> }
>> @@ -1021,7 +1016,7 @@ int main(int argc, char **argv)
>> qmp_for_each_command(ga_print_cmd, NULL);
>> exit(EXIT_SUCCESS);
>> }
>> - blacklist = g_list_concat(blacklist, split_list(optarg,
>> ','));
>> + blacklist = g_list_concat(blacklist, split_list(optarg,
>> ","));
>> break;
>> }
>> #ifdef _WIN32
>> @@ -1201,6 +1196,7 @@ int main(int argc, char **argv)
>> }
>> #endif
>> + g_list_free_full(ga_state->blacklist, g_free);
>> ga_command_state_cleanup_all(ga_state->command_state);
>> ga_channel_free(ga_state->channel);
> Reviewed-by: Denis V. Lunev <den@openvz.org>
sorry, this R-b was intended for previous patch.
Taken back :)
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 04/13] qga: make split_list() return allocated strings
2015-08-27 9:17 ` Denis V. Lunev
@ 2015-08-27 14:30 ` Marc-André Lureau
2015-08-27 16:15 ` Denis V. Lunev
0 siblings, 1 reply; 29+ messages in thread
From: Marc-André Lureau @ 2015-08-27 14:30 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: QEMU, Michael Roth
Hi
On Thu, Aug 27, 2015 at 11:17 AM, Denis V. Lunev <den@openvz.org> wrote:
> why not to
> __str = g_strsplit(str, delim, -1);
>>>
>>> + strv = g_strsplit(str, delim, -1);
>>> + for (i = 0; strv[i]; i++) {
>>> + list = g_list_prepend(list, strv[i]);
>>> }
>>> + g_free(strv);
>
> g_free(__str);
>
> This will remove all burden from callers.
> You will be able to use const char * as argument too.
Sorry, I don't understand, could you propose a different patch?
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 04/13] qga: make split_list() return allocated strings
2015-08-27 14:30 ` Marc-André Lureau
@ 2015-08-27 16:15 ` Denis V. Lunev
0 siblings, 0 replies; 29+ messages in thread
From: Denis V. Lunev @ 2015-08-27 16:15 UTC (permalink / raw)
To: Marc-André Lureau, Denis V. Lunev; +Cc: QEMU, Michael Roth
On 08/27/2015 05:30 PM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Aug 27, 2015 at 11:17 AM, Denis V. Lunev <den@openvz.org> wrote:
>> why not to
>> __str = g_strsplit(str, delim, -1);
>>>> + strv = g_strsplit(str, delim, -1);
>>>> + for (i = 0; strv[i]; i++) {
>>>> + list = g_list_prepend(list, strv[i]);
>>>> }
>>>> + g_free(strv);
>> g_free(__str);
>>
>> This will remove all burden from callers.
>> You will be able to use const char * as argument too.
>
> Sorry, I don't understand, could you propose a different patch?
>
you are right, me wrong :) I have mis-read the code.
This one is perfect.
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v4 05/13] qga: rename 'path' to 'channel_path'
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (3 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 04/13] qga: make split_list() return allocated strings marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 06/13] qga: copy argument strings marcandre.lureau
` (9 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
'path' is already a global function, rename the variable since it's
going to be in global scope in a later patch.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
qga/main.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index a7df6c8..7fa6dcc 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -939,7 +939,7 @@ static GList *split_list(const gchar *str, const gchar *delim)
int main(int argc, char **argv)
{
const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
- const char *method = NULL, *path = NULL;
+ const char *method = NULL, *channel_path = NULL;
const char *log_filepath = NULL;
const char *pid_filepath;
#ifdef CONFIG_FSFREEZE
@@ -985,7 +985,7 @@ int main(int argc, char **argv)
method = optarg;
break;
case 'p':
- path = optarg;
+ channel_path = optarg;
break;
case 'l':
log_filepath = optarg;
@@ -1035,7 +1035,8 @@ int main(int argc, char **argv)
if (ga_install_vss_provider()) {
exit(EXIT_FAILURE);
}
- if (ga_install_service(path, log_filepath, fixed_state_dir)) {
+ if (ga_install_service(channel_path, log_filepath,
+ fixed_state_dir)) {
exit(EXIT_FAILURE);
}
exit(EXIT_SUCCESS);
@@ -1180,7 +1181,7 @@ int main(int argc, char **argv)
#endif
s->main_loop = g_main_loop_new(NULL, false);
- if (!channel_init(ga_state, method, path)) {
+ if (!channel_init(ga_state, method, channel_path)) {
g_critical("failed to initialize guest agent channel");
goto out_bad;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v4 06/13] qga: copy argument strings
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (4 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 05/13] qga: rename 'path' to 'channel_path' marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-27 9:53 ` Denis V. Lunev
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 07/13] qga: move option parsing to separate function marcandre.lureau
` (8 subsequent siblings)
14 siblings, 1 reply; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Following patch will return allocated strings, so we must correctly
initialize alloc & free them. The nice side effect is that we no longer
have to check for "fixed_state_dir" to call ga_install_service() with a
NULL state dir. The default values are set after parsing the command
line options.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 57 +++++++++++++++++++++++++++++++--------------------------
1 file changed, 31 insertions(+), 26 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 7fa6dcc..766cb93 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -939,13 +939,13 @@ static GList *split_list(const gchar *str, const gchar *delim)
int main(int argc, char **argv)
{
const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
- const char *method = NULL, *channel_path = NULL;
- const char *log_filepath = NULL;
- const char *pid_filepath;
+ char *method = NULL, *channel_path = NULL;
+ char *log_filepath = NULL;
+ char *pid_filepath = NULL;
#ifdef CONFIG_FSFREEZE
- const char *fsfreeze_hook = NULL;
+ char *fsfreeze_hook = NULL;
#endif
- const char *state_dir;
+ char *state_dir = NULL;
#ifdef _WIN32
const char *service = NULL;
#endif
@@ -976,31 +976,28 @@ int main(int argc, char **argv)
module_call_init(MODULE_INIT_QAPI);
init_dfl_pathnames();
- pid_filepath = dfl_pathnames.pidfile;
- state_dir = dfl_pathnames.state_dir;
-
while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
switch (ch) {
case 'm':
- method = optarg;
+ method = g_strdup(optarg);
break;
case 'p':
- channel_path = optarg;
+ channel_path = g_strdup(optarg);
break;
case 'l':
- log_filepath = optarg;
+ log_filepath = g_strdup(optarg);
break;
case 'f':
- pid_filepath = optarg;
+ pid_filepath = g_strdup(optarg);
break;
#ifdef CONFIG_FSFREEZE
case 'F':
- fsfreeze_hook = optarg ? optarg : QGA_FSFREEZE_HOOK_DEFAULT;
+ fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
break;
#endif
case 't':
- state_dir = optarg;
- break;
+ state_dir = g_strdup(optarg);
+ break;
case 'v':
/* enable all log levels */
log_level = G_LOG_LEVEL_MASK;
@@ -1023,20 +1020,10 @@ int main(int argc, char **argv)
case 's':
service = optarg;
if (strcmp(service, "install") == 0) {
- const char *fixed_state_dir;
-
- /* If the user passed the "-t" option, we save that state dir
- * in the service. Otherwise we let the service fetch the state
- * dir from the environment when it starts.
- */
- fixed_state_dir = (state_dir == dfl_pathnames.state_dir) ?
- NULL :
- state_dir;
if (ga_install_vss_provider()) {
exit(EXIT_FAILURE);
}
- if (ga_install_service(channel_path, log_filepath,
- fixed_state_dir)) {
+ if (ga_install_service(channel_path, log_filepath, state_dir)) {
exit(EXIT_FAILURE);
}
exit(EXIT_SUCCESS);
@@ -1067,6 +1054,14 @@ int main(int argc, char **argv)
}
}
+ if (pid_filepath == NULL) {
+ pid_filepath = g_strdup(dfl_pathnames.pidfile);
+ }
+
+ if (state_dir == NULL) {
+ state_dir = g_strdup(dfl_pathnames.state_dir);
+ }
+
#ifdef _WIN32
/* On win32 the state directory is application specific (be it the default
* or a user override). We got past the command line parsing; let's create
@@ -1210,5 +1205,15 @@ out_bad:
if (daemonize) {
unlink(pid_filepath);
}
+
+ g_free(method);
+ g_free(log_filepath);
+ g_free(pid_filepath);
+ g_free(state_dir);
+ g_free(channel_path);
+#ifdef CONFIG_FSFREEZE
+ g_free(fsfreeze_hook);
+#endif
+
return EXIT_FAILURE;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 06/13] qga: copy argument strings
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 06/13] qga: copy argument strings marcandre.lureau
@ 2015-08-27 9:53 ` Denis V. Lunev
0 siblings, 0 replies; 29+ messages in thread
From: Denis V. Lunev @ 2015-08-27 9:53 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: mdroth
On 08/27/2015 02:34 AM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Following patch will return allocated strings, so we must correctly
> initialize alloc & free them. The nice side effect is that we no longer
> have to check for "fixed_state_dir" to call ga_install_service() with a
> NULL state dir. The default values are set after parsing the command
> line options.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> qga/main.c | 57 +++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 7fa6dcc..766cb93 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -939,13 +939,13 @@ static GList *split_list(const gchar *str, const gchar *delim)
> int main(int argc, char **argv)
> {
> const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
> - const char *method = NULL, *channel_path = NULL;
> - const char *log_filepath = NULL;
> - const char *pid_filepath;
> + char *method = NULL, *channel_path = NULL;
> + char *log_filepath = NULL;
> + char *pid_filepath = NULL;
> #ifdef CONFIG_FSFREEZE
> - const char *fsfreeze_hook = NULL;
> + char *fsfreeze_hook = NULL;
> #endif
> - const char *state_dir;
> + char *state_dir = NULL;
> #ifdef _WIN32
> const char *service = NULL;
> #endif
> @@ -976,31 +976,28 @@ int main(int argc, char **argv)
> module_call_init(MODULE_INIT_QAPI);
>
> init_dfl_pathnames();
> - pid_filepath = dfl_pathnames.pidfile;
> - state_dir = dfl_pathnames.state_dir;
> -
> while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
> switch (ch) {
> case 'm':
> - method = optarg;
> + method = g_strdup(optarg);
> break;
> case 'p':
> - channel_path = optarg;
> + channel_path = g_strdup(optarg);
> break;
> case 'l':
> - log_filepath = optarg;
> + log_filepath = g_strdup(optarg);
> break;
> case 'f':
> - pid_filepath = optarg;
> + pid_filepath = g_strdup(optarg);
> break;
> #ifdef CONFIG_FSFREEZE
> case 'F':
> - fsfreeze_hook = optarg ? optarg : QGA_FSFREEZE_HOOK_DEFAULT;
> + fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
> break;
> #endif
> case 't':
> - state_dir = optarg;
> - break;
> + state_dir = g_strdup(optarg);
> + break;
> case 'v':
> /* enable all log levels */
> log_level = G_LOG_LEVEL_MASK;
> @@ -1023,20 +1020,10 @@ int main(int argc, char **argv)
> case 's':
> service = optarg;
> if (strcmp(service, "install") == 0) {
> - const char *fixed_state_dir;
> -
> - /* If the user passed the "-t" option, we save that state dir
> - * in the service. Otherwise we let the service fetch the state
> - * dir from the environment when it starts.
> - */
> - fixed_state_dir = (state_dir == dfl_pathnames.state_dir) ?
> - NULL :
> - state_dir;
> if (ga_install_vss_provider()) {
> exit(EXIT_FAILURE);
> }
> - if (ga_install_service(channel_path, log_filepath,
> - fixed_state_dir)) {
> + if (ga_install_service(channel_path, log_filepath, state_dir)) {
> exit(EXIT_FAILURE);
> }
> exit(EXIT_SUCCESS);
> @@ -1067,6 +1054,14 @@ int main(int argc, char **argv)
> }
> }
>
> + if (pid_filepath == NULL) {
> + pid_filepath = g_strdup(dfl_pathnames.pidfile);
> + }
> +
> + if (state_dir == NULL) {
> + state_dir = g_strdup(dfl_pathnames.state_dir);
> + }
> +
> #ifdef _WIN32
> /* On win32 the state directory is application specific (be it the default
> * or a user override). We got past the command line parsing; let's create
> @@ -1210,5 +1205,15 @@ out_bad:
> if (daemonize) {
> unlink(pid_filepath);
> }
> +
> + g_free(method);
> + g_free(log_filepath);
> + g_free(pid_filepath);
> + g_free(state_dir);
> + g_free(channel_path);
> +#ifdef CONFIG_FSFREEZE
> + g_free(fsfreeze_hook);
> +#endif
> +
> return EXIT_FAILURE;
> }
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v4 07/13] qga: move option parsing to separate function
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (5 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 06/13] qga: copy argument strings marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 08/13] qga: fill default options in main() marcandre.lureau
` (7 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Move option parsing out of giant main().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
qga/main.c | 165 +++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 96 insertions(+), 69 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 766cb93..900b68c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -936,19 +936,28 @@ static GList *split_list(const gchar *str, const gchar *delim)
return list;
}
-int main(int argc, char **argv)
-{
- const char *sopt = "hVvdm:p:l:f:F::b:s:t:";
- char *method = NULL, *channel_path = NULL;
- char *log_filepath = NULL;
- char *pid_filepath = NULL;
+typedef struct GAConfig {
+ char *channel_path;
+ char *method;
+ char *log_filepath;
+ char *pid_filepath;
#ifdef CONFIG_FSFREEZE
- char *fsfreeze_hook = NULL;
+ char *fsfreeze_hook;
#endif
- char *state_dir = NULL;
+ char *state_dir;
#ifdef _WIN32
- const char *service = NULL;
+ const char *service;
#endif
+ GList *blacklist;
+ int daemonize;
+ GLogLevelFlags log_level;
+} GAConfig;
+
+static GAConfig *config_parse(int argc, char **argv)
+{
+ GAConfig *config = g_new0(GAConfig, 1);
+ const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
+ int opt_ind = 0, ch;
const struct option lopt[] = {
{ "help", 0, NULL, 'h' },
{ "version", 0, NULL, 'V' },
@@ -968,74 +977,71 @@ int main(int argc, char **argv)
{ "statedir", 1, NULL, 't' },
{ NULL, 0, NULL, 0 }
};
- int opt_ind = 0, ch, daemonize = 0;
- GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
- GList *blacklist = NULL;
- GAState *s;
- module_call_init(MODULE_INIT_QAPI);
+ config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
- init_dfl_pathnames();
while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
switch (ch) {
case 'm':
- method = g_strdup(optarg);
+ config->method = g_strdup(optarg);
break;
case 'p':
- channel_path = g_strdup(optarg);
+ config->channel_path = g_strdup(optarg);
break;
case 'l':
- log_filepath = g_strdup(optarg);
+ config->log_filepath = g_strdup(optarg);
break;
case 'f':
- pid_filepath = g_strdup(optarg);
+ config->pid_filepath = g_strdup(optarg);
break;
#ifdef CONFIG_FSFREEZE
case 'F':
- fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
+ config->fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
break;
#endif
case 't':
- state_dir = g_strdup(optarg);
+ config->state_dir = g_strdup(optarg);
break;
case 'v':
/* enable all log levels */
- log_level = G_LOG_LEVEL_MASK;
+ config->log_level = G_LOG_LEVEL_MASK;
break;
case 'V':
printf("QEMU Guest Agent %s\n", QEMU_VERSION);
exit(EXIT_SUCCESS);
case 'd':
- daemonize = 1;
+ config->daemonize = 1;
break;
case 'b': {
if (is_help_option(optarg)) {
qmp_for_each_command(ga_print_cmd, NULL);
exit(EXIT_SUCCESS);
}
- blacklist = g_list_concat(blacklist, split_list(optarg, ","));
+ config->blacklist = g_list_concat(config->blacklist,
+ split_list(optarg, ","));
break;
}
#ifdef _WIN32
case 's':
- service = optarg;
- if (strcmp(service, "install") == 0) {
+ config->service = optarg;
+ if (strcmp(config->service, "install") == 0) {
if (ga_install_vss_provider()) {
exit(EXIT_FAILURE);
}
- if (ga_install_service(channel_path, log_filepath, state_dir)) {
+ if (ga_install_service(config->channel_path,
+ config->log_filepath, config->state_dir)) {
exit(EXIT_FAILURE);
}
exit(EXIT_SUCCESS);
- } else if (strcmp(service, "uninstall") == 0) {
+ } else if (strcmp(config->service, "uninstall") == 0) {
ga_uninstall_vss_provider();
exit(ga_uninstall_service());
- } else if (strcmp(service, "vss-install") == 0) {
+ } else if (strcmp(config->service, "vss-install") == 0) {
if (ga_install_vss_provider()) {
exit(EXIT_FAILURE);
}
exit(EXIT_SUCCESS);
- } else if (strcmp(service, "vss-uninstall") == 0) {
+ } else if (strcmp(config->service, "vss-uninstall") == 0) {
ga_uninstall_vss_provider();
exit(EXIT_SUCCESS);
} else {
@@ -1054,12 +1060,39 @@ int main(int argc, char **argv)
}
}
- if (pid_filepath == NULL) {
- pid_filepath = g_strdup(dfl_pathnames.pidfile);
+ return config;
+}
+
+static void config_free(GAConfig *config)
+{
+ g_free(config->method);
+ g_free(config->log_filepath);
+ g_free(config->pid_filepath);
+ g_free(config->state_dir);
+ g_free(config->channel_path);
+#ifdef CONFIG_FSFREEZE
+ g_free(config->fsfreeze_hook);
+#endif
+ g_free(config);
+}
+
+int main(int argc, char **argv)
+{
+ GAState *s;
+ GAConfig *config;
+
+ module_call_init(MODULE_INIT_QAPI);
+
+ init_dfl_pathnames();
+
+ config = config_parse(argc, argv);
+
+ if (config->pid_filepath == NULL) {
+ config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
}
- if (state_dir == NULL) {
- state_dir = g_strdup(dfl_pathnames.state_dir);
+ if (config->state_dir == NULL) {
+ config->state_dir = g_strdup(dfl_pathnames.state_dir);
}
#ifdef _WIN32
@@ -1069,25 +1102,25 @@ int main(int argc, char **argv)
* error later on, we won't try to clean up the directory, it is considered
* persistent.
*/
- if (g_mkdir_with_parents(state_dir, S_IRWXU) == -1) {
+ if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
g_critical("unable to create (an ancestor of) the state directory"
- " '%s': %s", state_dir, strerror(errno));
+ " '%s': %s", config->state_dir, strerror(errno));
return EXIT_FAILURE;
}
#endif
s = g_malloc0(sizeof(GAState));
- s->log_level = log_level;
+ s->log_level = config->log_level;
s->log_file = stderr;
#ifdef CONFIG_FSFREEZE
- s->fsfreeze_hook = fsfreeze_hook;
+ s->fsfreeze_hook = config->fsfreeze_hook;
#endif
g_log_set_default_handler(ga_log, s);
g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
ga_enable_logging(s);
s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
- state_dir);
- s->pstate_filepath = g_strdup_printf("%s/qga.state", state_dir);
+ config->state_dir);
+ s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
s->frozen = false;
#ifndef _WIN32
@@ -1120,23 +1153,23 @@ int main(int argc, char **argv)
#endif
if (ga_is_frozen(s)) {
- if (daemonize) {
+ if (config->daemonize) {
/* delay opening/locking of pidfile till filesystems are unfrozen */
- s->deferred_options.pid_filepath = pid_filepath;
+ s->deferred_options.pid_filepath = config->pid_filepath;
become_daemon(NULL);
}
- if (log_filepath) {
+ if (config->log_filepath) {
/* delay opening the log file till filesystems are unfrozen */
- s->deferred_options.log_filepath = log_filepath;
+ s->deferred_options.log_filepath = config->log_filepath;
}
ga_disable_logging(s);
qmp_for_each_command(ga_disable_non_whitelisted, NULL);
} else {
- if (daemonize) {
- become_daemon(pid_filepath);
+ if (config->daemonize) {
+ become_daemon(config->pid_filepath);
}
- if (log_filepath) {
- FILE *log_file = ga_open_logfile(log_filepath);
+ if (config->log_filepath) {
+ FILE *log_file = ga_open_logfile(config->log_filepath);
if (!log_file) {
g_critical("unable to open specified log file: %s",
strerror(errno));
@@ -1154,14 +1187,15 @@ int main(int argc, char **argv)
goto out_bad;
}
- blacklist = ga_command_blacklist_init(blacklist);
- if (blacklist) {
- s->blacklist = blacklist;
+ config->blacklist = ga_command_blacklist_init(config->blacklist);
+ if (config->blacklist) {
+ GList *l = config->blacklist;
+ s->blacklist = config->blacklist;
do {
- g_debug("disabling command: %s", (char *)blacklist->data);
- qmp_disable_command(blacklist->data);
- blacklist = g_list_next(blacklist);
- } while (blacklist);
+ g_debug("disabling command: %s", (char *)l->data);
+ qmp_disable_command(l->data);
+ l = g_list_next(l);
+ } while (l);
}
s->command_state = ga_command_state_new();
ga_command_state_init(s, s->command_state);
@@ -1176,14 +1210,14 @@ int main(int argc, char **argv)
#endif
s->main_loop = g_main_loop_new(NULL, false);
- if (!channel_init(ga_state, method, channel_path)) {
+ if (!channel_init(ga_state, config->method, config->channel_path)) {
g_critical("failed to initialize guest agent channel");
goto out_bad;
}
#ifndef _WIN32
g_main_loop_run(ga_state->main_loop);
#else
- if (daemonize) {
+ if (config->daemonize) {
SERVICE_TABLE_ENTRY service_table[] = {
{ (char *)QGA_SERVICE_NAME, service_main }, { NULL, NULL } };
StartServiceCtrlDispatcher(service_table);
@@ -1196,24 +1230,17 @@ int main(int argc, char **argv)
ga_command_state_cleanup_all(ga_state->command_state);
ga_channel_free(ga_state->channel);
- if (daemonize) {
- unlink(pid_filepath);
+ if (config->daemonize) {
+ unlink(config->pid_filepath);
}
return 0;
out_bad:
- if (daemonize) {
- unlink(pid_filepath);
+ if (config->daemonize) {
+ unlink(config->pid_filepath);
}
- g_free(method);
- g_free(log_filepath);
- g_free(pid_filepath);
- g_free(state_dir);
- g_free(channel_path);
-#ifdef CONFIG_FSFREEZE
- g_free(fsfreeze_hook);
-#endif
+ config_free(config);
return EXIT_FAILURE;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v4 08/13] qga: fill default options in main()
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (6 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 07/13] qga: move option parsing to separate function marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 09/13] qga: move agent run in a separate function marcandre.lureau
` (6 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Fill all default options during main(). This is a preparation patch
to allow to dump the configuration.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
qga/main.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 900b68c..38ee196 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -658,23 +658,6 @@ static gboolean channel_init(GAState *s, const gchar *method, const gchar *path)
{
GAChannelMethod channel_method;
- if (method == NULL) {
- method = "virtio-serial";
- }
-
- if (path == NULL) {
- if (strcmp(method, "virtio-serial") == 0 ) {
- /* try the default path for the virtio-serial port */
- path = QGA_VIRTIO_PATH_DEFAULT;
- } else if (strcmp(method, "isa-serial") == 0){
- /* try the default path for the serial port - COM1 */
- path = QGA_SERIAL_PATH_DEFAULT;
- } else {
- g_critical("must specify a path for this channel");
- return false;
- }
- }
-
if (strcmp(method, "virtio-serial") == 0) {
s->virtio = true; /* virtio requires special handling in some cases */
channel_method = GA_CHANNEL_VIRTIO_SERIAL;
@@ -1095,6 +1078,23 @@ int main(int argc, char **argv)
config->state_dir = g_strdup(dfl_pathnames.state_dir);
}
+ if (config->method == NULL) {
+ config->method = g_strdup("virtio-serial");
+ }
+
+ if (config->channel_path == NULL) {
+ if (strcmp(config->method, "virtio-serial") == 0) {
+ /* try the default path for the virtio-serial port */
+ config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
+ } else if (strcmp(config->method, "isa-serial") == 0) {
+ /* try the default path for the serial port - COM1 */
+ config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
+ } else {
+ g_critical("must specify a path for this channel");
+ goto out_bad;
+ }
+ }
+
#ifdef _WIN32
/* On win32 the state directory is application specific (be it the default
* or a user override). We got past the command line parsing; let's create
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v4 09/13] qga: move agent run in a separate function
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (7 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 08/13] qga: fill default options in main() marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 10/13] qga: free a bit more marcandre.lureau
` (5 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Once the options are populated, move the running state to
a run_agent() function.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
qga/main.c | 166 +++++++++++++++++++++++++++++++++----------------------------
1 file changed, 90 insertions(+), 76 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 38ee196..6518e1f 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1059,70 +1059,8 @@ static void config_free(GAConfig *config)
g_free(config);
}
-int main(int argc, char **argv)
+static bool check_is_frozen(GAState *s)
{
- GAState *s;
- GAConfig *config;
-
- module_call_init(MODULE_INIT_QAPI);
-
- init_dfl_pathnames();
-
- config = config_parse(argc, argv);
-
- if (config->pid_filepath == NULL) {
- config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
- }
-
- if (config->state_dir == NULL) {
- config->state_dir = g_strdup(dfl_pathnames.state_dir);
- }
-
- if (config->method == NULL) {
- config->method = g_strdup("virtio-serial");
- }
-
- if (config->channel_path == NULL) {
- if (strcmp(config->method, "virtio-serial") == 0) {
- /* try the default path for the virtio-serial port */
- config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
- } else if (strcmp(config->method, "isa-serial") == 0) {
- /* try the default path for the serial port - COM1 */
- config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
- } else {
- g_critical("must specify a path for this channel");
- goto out_bad;
- }
- }
-
-#ifdef _WIN32
- /* On win32 the state directory is application specific (be it the default
- * or a user override). We got past the command line parsing; let's create
- * the directory (with any intermediate directories). If we run into an
- * error later on, we won't try to clean up the directory, it is considered
- * persistent.
- */
- if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
- g_critical("unable to create (an ancestor of) the state directory"
- " '%s': %s", config->state_dir, strerror(errno));
- return EXIT_FAILURE;
- }
-#endif
-
- s = g_malloc0(sizeof(GAState));
- s->log_level = config->log_level;
- s->log_file = stderr;
-#ifdef CONFIG_FSFREEZE
- s->fsfreeze_hook = config->fsfreeze_hook;
-#endif
- g_log_set_default_handler(ga_log, s);
- g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
- ga_enable_logging(s);
- s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
- config->state_dir);
- s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
- s->frozen = false;
-
#ifndef _WIN32
/* check if a previous instance of qemu-ga exited with filesystems' state
* marked as frozen. this could be a stale value (a non-qemu-ga process
@@ -1148,7 +1086,31 @@ int main(int argc, char **argv)
" guest-fsfreeze-thaw is issued, or filesystems are"
" manually unfrozen and the file %s is removed",
s->state_filepath_isfrozen);
- s->frozen = true;
+ return true;
+ }
+#endif
+ return false;
+}
+
+static int run_agent(GAState *s, GAConfig *config)
+{
+ ga_state = s;
+
+ g_log_set_default_handler(ga_log, s);
+ g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
+ ga_enable_logging(s);
+
+#ifdef _WIN32
+ /* On win32 the state directory is application specific (be it the default
+ * or a user override). We got past the command line parsing; let's create
+ * the directory (with any intermediate directories). If we run into an
+ * error later on, we won't try to clean up the directory, it is considered
+ * persistent.
+ */
+ if (g_mkdir_with_parents(config->state_dir, S_IRWXU) == -1) {
+ g_critical("unable to create (an ancestor of) the state directory"
+ " '%s': %s", config->state_dir, strerror(errno));
+ return EXIT_FAILURE;
}
#endif
@@ -1173,7 +1135,7 @@ int main(int argc, char **argv)
if (!log_file) {
g_critical("unable to open specified log file: %s",
strerror(errno));
- goto out_bad;
+ return EXIT_FAILURE;
}
s->log_file = log_file;
}
@@ -1184,7 +1146,7 @@ int main(int argc, char **argv)
s->pstate_filepath,
ga_is_frozen(s))) {
g_critical("failed to load persistent state");
- goto out_bad;
+ return EXIT_FAILURE;
}
config->blacklist = ga_command_blacklist_init(config->blacklist);
@@ -1205,14 +1167,14 @@ int main(int argc, char **argv)
#ifndef _WIN32
if (!register_signal_handlers()) {
g_critical("failed to register signal handlers");
- goto out_bad;
+ return EXIT_FAILURE;
}
#endif
s->main_loop = g_main_loop_new(NULL, false);
if (!channel_init(ga_state, config->method, config->channel_path)) {
g_critical("failed to initialize guest agent channel");
- goto out_bad;
+ return EXIT_FAILURE;
}
#ifndef _WIN32
g_main_loop_run(ga_state->main_loop);
@@ -1226,21 +1188,73 @@ int main(int argc, char **argv)
}
#endif
- g_list_free_full(ga_state->blacklist, g_free);
- ga_command_state_cleanup_all(ga_state->command_state);
- ga_channel_free(ga_state->channel);
+ return EXIT_SUCCESS;
+}
- if (config->daemonize) {
- unlink(config->pid_filepath);
+int main(int argc, char **argv)
+{
+ int ret = EXIT_SUCCESS;
+ GAState *s = g_new0(GAState, 1);
+ GAConfig *config;
+
+ module_call_init(MODULE_INIT_QAPI);
+
+ init_dfl_pathnames();
+
+ config = config_parse(argc, argv);
+
+ if (config->pid_filepath == NULL) {
+ config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
}
- return 0;
-out_bad:
+ if (config->state_dir == NULL) {
+ config->state_dir = g_strdup(dfl_pathnames.state_dir);
+ }
+
+ if (config->method == NULL) {
+ config->method = g_strdup("virtio-serial");
+ }
+
+ if (config->channel_path == NULL) {
+ if (strcmp(config->method, "virtio-serial") == 0) {
+ /* try the default path for the virtio-serial port */
+ config->channel_path = g_strdup(QGA_VIRTIO_PATH_DEFAULT);
+ } else if (strcmp(config->method, "isa-serial") == 0) {
+ /* try the default path for the serial port - COM1 */
+ config->channel_path = g_strdup(QGA_SERIAL_PATH_DEFAULT);
+ } else {
+ g_critical("must specify a path for this channel");
+ ret = EXIT_FAILURE;
+ goto end;
+ }
+ }
+
+ s->log_level = config->log_level;
+ s->log_file = stderr;
+#ifdef CONFIG_FSFREEZE
+ s->fsfreeze_hook = config->fsfreeze_hook;
+#endif
+ s->pstate_filepath = g_strdup_printf("%s/qga.state", config->state_dir);
+ s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen",
+ config->state_dir);
+ s->frozen = check_is_frozen(s);
+
+ ret = run_agent(s, config);
+
+end:
+ if (s->command_state) {
+ ga_command_state_cleanup_all(s->command_state);
+ }
+ if (s->channel) {
+ ga_channel_free(s->channel);
+ }
+ g_list_free_full(ga_state->blacklist, g_free);
+
if (config->daemonize) {
unlink(config->pid_filepath);
}
config_free(config);
- return EXIT_FAILURE;
+ return ret;
}
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v4 10/13] qga: free a bit more
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (8 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 09/13] qga: move agent run in a separate function marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 11/13] qga: add an optional qemu-ga.conf system configuration marcandre.lureau
` (4 subsequent siblings)
14 siblings, 0 replies; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Now that main() has a single exit point, we can free a few
more allocations.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
---
qga/main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 6518e1f..710dd47 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -82,7 +82,7 @@ struct GAState {
bool delimit_response;
bool frozen;
GList *blacklist;
- const char *state_filepath_isfrozen;
+ char *state_filepath_isfrozen;
struct {
const char *log_filepath;
const char *pid_filepath;
@@ -90,7 +90,7 @@ struct GAState {
#ifdef CONFIG_FSFREEZE
const char *fsfreeze_hook;
#endif
- const gchar *pstate_filepath;
+ gchar *pstate_filepath;
GAPersistentState pstate;
};
@@ -1249,6 +1249,8 @@ end:
ga_channel_free(s->channel);
}
g_list_free_full(ga_state->blacklist, g_free);
+ g_free(s->pstate_filepath);
+ g_free(s->state_filepath_isfrozen);
if (config->daemonize) {
unlink(config->pid_filepath);
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* [Qemu-devel] [PATCH v4 11/13] qga: add an optional qemu-ga.conf system configuration
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (9 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 10/13] qga: free a bit more marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-27 10:02 ` Denis V. Lunev
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 12/13] qga: add --dump-conf option marcandre.lureau
` (3 subsequent siblings)
14 siblings, 1 reply; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Learn to configure the agent with a system configuration.
This may simplify command-line handling, especially when the blacklist
is long.
Among the other benefits, this may standardize the configuration of an
init service (instead of distro-specific init keys/files)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 79 insertions(+), 7 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 710dd47..881a92b 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -56,6 +56,7 @@
#define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
#endif
#define QGA_SENTINEL_BYTE 0xFF
+#define QGA_CONF_DEFAULT CONFIG_QEMU_CONFDIR G_DIR_SEPARATOR_S "qemu-ga.conf"
static struct {
const char *state_dir;
@@ -931,14 +932,80 @@ typedef struct GAConfig {
#ifdef _WIN32
const char *service;
#endif
+ gchar *bliststr; /* blacklist may point to this string */
GList *blacklist;
int daemonize;
GLogLevelFlags log_level;
} GAConfig;
-static GAConfig *config_parse(int argc, char **argv)
+static void config_load(GAConfig *config)
+{
+ GError *gerr = NULL;
+ GKeyFile *keyfile;
+
+ /* read system config */
+ keyfile = g_key_file_new();
+ if (!g_key_file_load_from_file(keyfile, QGA_CONF_DEFAULT, 0, &gerr)) {
+ goto end;
+ }
+ if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
+ config->daemonize =
+ g_key_file_get_boolean(keyfile, "general", "daemon", &gerr);
+ }
+ if (g_key_file_has_key(keyfile, "general", "method", NULL)) {
+ config->method =
+ g_key_file_get_string(keyfile, "general", "method", &gerr);
+ }
+ if (g_key_file_has_key(keyfile, "general", "path", NULL)) {
+ config->channel_path =
+ g_key_file_get_string(keyfile, "general", "path", &gerr);
+ }
+ if (g_key_file_has_key(keyfile, "general", "logfile", NULL)) {
+ config->log_filepath =
+ g_key_file_get_string(keyfile, "general", "logfile", &gerr);
+ }
+ if (g_key_file_has_key(keyfile, "general", "pidfile", NULL)) {
+ config->pid_filepath =
+ g_key_file_get_string(keyfile, "general", "pidfile", &gerr);
+ }
+#ifdef CONFIG_FSFREEZE
+ if (g_key_file_has_key(keyfile, "general", "fsfreeze-hook", NULL)) {
+ config->fsfreeze_hook =
+ g_key_file_get_string(keyfile,
+ "general", "fsfreeze-hook", &gerr);
+ }
+#endif
+ if (g_key_file_has_key(keyfile, "general", "statedir", NULL)) {
+ config->state_dir =
+ g_key_file_get_string(keyfile, "general", "statedir", &gerr);
+ }
+ if (g_key_file_has_key(keyfile, "general", "verbose", NULL) &&
+ g_key_file_get_boolean(keyfile, "general", "verbose", &gerr)) {
+ /* enable all log levels */
+ config->log_level = G_LOG_LEVEL_MASK;
+ }
+ if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) {
+ config->bliststr =
+ g_key_file_get_string(keyfile, "general", "blacklist", &gerr);
+ config->blacklist = g_list_concat(config->blacklist,
+ split_list(config->bliststr, ","));
+ }
+
+end:
+ if (keyfile) {
+ g_key_file_free(keyfile);
+ }
+ if (gerr &&
+ !(gerr->domain == G_FILE_ERROR && gerr->code == G_FILE_ERROR_NOENT)) {
+ g_critical("error loading configuration from path: %s, %s",
+ QGA_CONF_DEFAULT, gerr->message);
+ exit(EXIT_FAILURE);
+ }
+ g_clear_error(&gerr);
+}
+
+static void config_parse(GAConfig *config, int argc, char **argv)
{
- GAConfig *config = g_new0(GAConfig, 1);
const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
int opt_ind = 0, ch;
const struct option lopt[] = {
@@ -966,23 +1033,29 @@ static GAConfig *config_parse(int argc, char **argv)
while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
switch (ch) {
case 'm':
+ g_free(config->method);
config->method = g_strdup(optarg);
break;
case 'p':
+ g_free(config->channel_path);
config->channel_path = g_strdup(optarg);
break;
case 'l':
+ g_free(config->log_filepath);
config->log_filepath = g_strdup(optarg);
break;
case 'f':
+ g_free(config->pid_filepath);
config->pid_filepath = g_strdup(optarg);
break;
#ifdef CONFIG_FSFREEZE
case 'F':
+ g_free(config->fsfreeze_hook);
config->fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
break;
#endif
case 't':
+ g_free(config->state_dir);
config->state_dir = g_strdup(optarg);
break;
case 'v':
@@ -1042,8 +1115,6 @@ static GAConfig *config_parse(int argc, char **argv)
exit(EXIT_FAILURE);
}
}
-
- return config;
}
static void config_free(GAConfig *config)
@@ -1053,6 +1124,7 @@ static void config_free(GAConfig *config)
g_free(config->pid_filepath);
g_free(config->state_dir);
g_free(config->channel_path);
+ g_free(config->bliststr);
#ifdef CONFIG_FSFREEZE
g_free(config->fsfreeze_hook);
#endif
@@ -1195,13 +1267,13 @@ int main(int argc, char **argv)
{
int ret = EXIT_SUCCESS;
GAState *s = g_new0(GAState, 1);
- GAConfig *config;
+ GAConfig *config = g_new0(GAConfig, 1);
module_call_init(MODULE_INIT_QAPI);
init_dfl_pathnames();
-
- config = config_parse(argc, argv);
+ config_load(config);
+ config_parse(config, argc, argv);
if (config->pid_filepath == NULL) {
config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 11/13] qga: add an optional qemu-ga.conf system configuration
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 11/13] qga: add an optional qemu-ga.conf system configuration marcandre.lureau
@ 2015-08-27 10:02 ` Denis V. Lunev
0 siblings, 0 replies; 29+ messages in thread
From: Denis V. Lunev @ 2015-08-27 10:02 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: mdroth
On 08/27/2015 02:34 AM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Learn to configure the agent with a system configuration.
>
> This may simplify command-line handling, especially when the blacklist
> is long.
>
> Among the other benefits, this may standardize the configuration of an
> init service (instead of distro-specific init keys/files)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> qga/main.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 79 insertions(+), 7 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 710dd47..881a92b 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -56,6 +56,7 @@
> #define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
> #endif
> #define QGA_SENTINEL_BYTE 0xFF
> +#define QGA_CONF_DEFAULT CONFIG_QEMU_CONFDIR G_DIR_SEPARATOR_S "qemu-ga.conf"
>
> static struct {
> const char *state_dir;
> @@ -931,14 +932,80 @@ typedef struct GAConfig {
> #ifdef _WIN32
> const char *service;
> #endif
> + gchar *bliststr; /* blacklist may point to this string */
> GList *blacklist;
> int daemonize;
> GLogLevelFlags log_level;
> } GAConfig;
>
> -static GAConfig *config_parse(int argc, char **argv)
> +static void config_load(GAConfig *config)
> +{
> + GError *gerr = NULL;
> + GKeyFile *keyfile;
> +
> + /* read system config */
> + keyfile = g_key_file_new();
> + if (!g_key_file_load_from_file(keyfile, QGA_CONF_DEFAULT, 0, &gerr)) {
> + goto end;
> + }
> + if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
> + config->daemonize =
> + g_key_file_get_boolean(keyfile, "general", "daemon", &gerr);
> + }
> + if (g_key_file_has_key(keyfile, "general", "method", NULL)) {
> + config->method =
> + g_key_file_get_string(keyfile, "general", "method", &gerr);
> + }
> + if (g_key_file_has_key(keyfile, "general", "path", NULL)) {
> + config->channel_path =
> + g_key_file_get_string(keyfile, "general", "path", &gerr);
> + }
> + if (g_key_file_has_key(keyfile, "general", "logfile", NULL)) {
> + config->log_filepath =
> + g_key_file_get_string(keyfile, "general", "logfile", &gerr);
> + }
> + if (g_key_file_has_key(keyfile, "general", "pidfile", NULL)) {
> + config->pid_filepath =
> + g_key_file_get_string(keyfile, "general", "pidfile", &gerr);
> + }
> +#ifdef CONFIG_FSFREEZE
> + if (g_key_file_has_key(keyfile, "general", "fsfreeze-hook", NULL)) {
> + config->fsfreeze_hook =
> + g_key_file_get_string(keyfile,
> + "general", "fsfreeze-hook", &gerr);
> + }
> +#endif
> + if (g_key_file_has_key(keyfile, "general", "statedir", NULL)) {
> + config->state_dir =
> + g_key_file_get_string(keyfile, "general", "statedir", &gerr);
> + }
> + if (g_key_file_has_key(keyfile, "general", "verbose", NULL) &&
> + g_key_file_get_boolean(keyfile, "general", "verbose", &gerr)) {
> + /* enable all log levels */
> + config->log_level = G_LOG_LEVEL_MASK;
> + }
> + if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) {
> + config->bliststr =
> + g_key_file_get_string(keyfile, "general", "blacklist", &gerr);
> + config->blacklist = g_list_concat(config->blacklist,
> + split_list(config->bliststr, ","));
> + }
> +
> +end:
> + if (keyfile) {
> + g_key_file_free(keyfile);
> + }
> + if (gerr &&
> + !(gerr->domain == G_FILE_ERROR && gerr->code == G_FILE_ERROR_NOENT)) {
> + g_critical("error loading configuration from path: %s, %s",
> + QGA_CONF_DEFAULT, gerr->message);
> + exit(EXIT_FAILURE);
> + }
> + g_clear_error(&gerr);
> +}
> +
> +static void config_parse(GAConfig *config, int argc, char **argv)
> {
> - GAConfig *config = g_new0(GAConfig, 1);
> const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
> int opt_ind = 0, ch;
> const struct option lopt[] = {
> @@ -966,23 +1033,29 @@ static GAConfig *config_parse(int argc, char **argv)
> while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
> switch (ch) {
> case 'm':
> + g_free(config->method);
> config->method = g_strdup(optarg);
> break;
> case 'p':
> + g_free(config->channel_path);
> config->channel_path = g_strdup(optarg);
> break;
> case 'l':
> + g_free(config->log_filepath);
> config->log_filepath = g_strdup(optarg);
> break;
> case 'f':
> + g_free(config->pid_filepath);
> config->pid_filepath = g_strdup(optarg);
> break;
> #ifdef CONFIG_FSFREEZE
> case 'F':
> + g_free(config->fsfreeze_hook);
> config->fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
> break;
> #endif
> case 't':
> + g_free(config->state_dir);
> config->state_dir = g_strdup(optarg);
> break;
> case 'v':
> @@ -1042,8 +1115,6 @@ static GAConfig *config_parse(int argc, char **argv)
> exit(EXIT_FAILURE);
> }
> }
> -
> - return config;
> }
>
> static void config_free(GAConfig *config)
> @@ -1053,6 +1124,7 @@ static void config_free(GAConfig *config)
> g_free(config->pid_filepath);
> g_free(config->state_dir);
> g_free(config->channel_path);
> + g_free(config->bliststr);
> #ifdef CONFIG_FSFREEZE
> g_free(config->fsfreeze_hook);
> #endif
> @@ -1195,13 +1267,13 @@ int main(int argc, char **argv)
> {
> int ret = EXIT_SUCCESS;
> GAState *s = g_new0(GAState, 1);
> - GAConfig *config;
> + GAConfig *config = g_new0(GAConfig, 1);
>
> module_call_init(MODULE_INIT_QAPI);
>
> init_dfl_pathnames();
> -
> - config = config_parse(argc, argv);
> + config_load(config);
> + config_parse(config, argc, argv);
>
> if (config->pid_filepath == NULL) {
> config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
Reviewed-by: Denis V. Lunev <den@openvz.org>
except same note as in patch 12 about key file.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v4 12/13] qga: add --dump-conf option
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (10 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 11/13] qga: add an optional qemu-ga.conf system configuration marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-26 23:38 ` Marc-André Lureau
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 13/13] qga: start a man page marcandre.lureau
` (2 subsequent siblings)
14 siblings, 1 reply; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This new option allows to review the agent configuration,
and ease the task of writing a configuration file.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
qga/main.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/qga/main.c b/qga/main.c
index 881a92b..74330aa 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -936,6 +936,7 @@ typedef struct GAConfig {
GList *blacklist;
int daemonize;
GLogLevelFlags log_level;
+ int dumpconf;
} GAConfig;
static void config_load(GAConfig *config)
@@ -1004,6 +1005,60 @@ end:
g_clear_error(&gerr);
}
+static gchar *list_join(GList *list, const gchar separator)
+{
+ GString *str = g_string_new("");
+
+ while (list) {
+ str = g_string_append(str, (gchar *)list->data);
+ list = g_list_next(list);
+ if (list) {
+ str = g_string_append_c(str, separator);
+ }
+ }
+
+ return g_string_free(str, FALSE);
+}
+
+static void config_dump(GAConfig *config)
+{
+ GError *error = NULL;
+ GKeyFile *keyfile;
+ gchar *tmp;
+
+ keyfile = g_key_file_new();
+ g_assert(keyfile);
+
+ g_key_file_set_boolean(keyfile, "general", "daemon", config->daemonize);
+ g_key_file_set_string(keyfile, "general", "method", config->method);
+ g_key_file_set_string(keyfile, "general", "path", config->channel_path);
+ if (config->log_filepath) {
+ g_key_file_set_string(keyfile, "general", "logfile",
+ config->log_filepath);
+ }
+ g_key_file_set_string(keyfile, "general", "pidfile", config->pid_filepath);
+#ifdef CONFIG_FSFREEZE
+ if (config->fsfreeze_hook) {
+ g_key_file_set_string(keyfile, "general", "fsfreeze-hook",
+ config->fsfreeze_hook);
+ }
+#endif
+ g_key_file_set_string(keyfile, "general", "statedir", config->state_dir);
+ g_key_file_set_boolean(keyfile, "general", "verbose",
+ config->log_level == G_LOG_LEVEL_MASK);
+ tmp = list_join(config->blacklist, ',');
+ g_key_file_set_string(keyfile, "general", "blacklist", tmp);
+ g_free(tmp);
+
+ tmp = g_key_file_to_data(keyfile, NULL, &error);
+ printf("%s", tmp);
+
+ g_free(tmp);
+ if (keyfile) {
+ g_key_file_free(keyfile);
+ }
+}
+
static void config_parse(GAConfig *config, int argc, char **argv)
{
const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
@@ -1011,6 +1066,7 @@ static void config_parse(GAConfig *config, int argc, char **argv)
const struct option lopt[] = {
{ "help", 0, NULL, 'h' },
{ "version", 0, NULL, 'V' },
+ { "dump-conf", 0, NULL, 'D' },
{ "logfile", 1, NULL, 'l' },
{ "pidfile", 1, NULL, 'f' },
#ifdef CONFIG_FSFREEZE
@@ -1068,6 +1124,9 @@ static void config_parse(GAConfig *config, int argc, char **argv)
case 'd':
config->daemonize = 1;
break;
+ case 'D':
+ config->dumpconf = 1;
+ break;
case 'b': {
if (is_help_option(optarg)) {
qmp_for_each_command(ga_print_cmd, NULL);
@@ -1311,6 +1370,11 @@ int main(int argc, char **argv)
config->state_dir);
s->frozen = check_is_frozen(s);
+ if (config->dumpconf) {
+ config_dump(config);
+ goto end;
+ }
+
ret = run_agent(s, config);
end:
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 12/13] qga: add --dump-conf option
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 12/13] qga: add --dump-conf option marcandre.lureau
@ 2015-08-26 23:38 ` Marc-André Lureau
2015-08-27 9:56 ` Denis V. Lunev
2015-08-27 9:57 ` Denis V. Lunev
0 siblings, 2 replies; 29+ messages in thread
From: Marc-André Lureau @ 2015-08-26 23:38 UTC (permalink / raw)
To: QEMU; +Cc: den, Michael Roth, Marc-André Lureau
Hi
On Thu, Aug 27, 2015 at 1:34 AM, <marcandre.lureau@redhat.com> wrote:
> + if (keyfile) {
> + g_key_file_free(keyfile);
> + }
> +}
I forgot the if () can be removed now.
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 12/13] qga: add --dump-conf option
2015-08-26 23:38 ` Marc-André Lureau
@ 2015-08-27 9:56 ` Denis V. Lunev
2015-08-27 9:57 ` Denis V. Lunev
1 sibling, 0 replies; 29+ messages in thread
From: Denis V. Lunev @ 2015-08-27 9:56 UTC (permalink / raw)
To: Marc-André Lureau, QEMU; +Cc: Marc-André Lureau, Michael Roth
On 08/27/2015 02:38 AM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Aug 27, 2015 at 1:34 AM, <marcandre.lureau@redhat.com> wrote:
>> + if (keyfile) {
>> + g_key_file_free(keyfile);
>> + }
>> +}
>
> I forgot the if () can be removed now.
>
yep...
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 12/13] qga: add --dump-conf option
2015-08-26 23:38 ` Marc-André Lureau
2015-08-27 9:56 ` Denis V. Lunev
@ 2015-08-27 9:57 ` Denis V. Lunev
1 sibling, 0 replies; 29+ messages in thread
From: Denis V. Lunev @ 2015-08-27 9:57 UTC (permalink / raw)
To: Marc-André Lureau, QEMU; +Cc: Marc-André Lureau, Michael Roth
On 08/27/2015 02:38 AM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Aug 27, 2015 at 1:34 AM, <marcandre.lureau@redhat.com> wrote:
>> + if (keyfile) {
>> + g_key_file_free(keyfile);
>> + }
>> +}
>
> I forgot the if () can be removed now.
>
yep...
The rest is
Reviewed-by: Denis V. Lunev <den@openvz.org>
Pls note that there is the same thing in patch 11
in
static void config_load(GAConfig *config)
^ permalink raw reply [flat|nested] 29+ messages in thread
* [Qemu-devel] [PATCH v4 13/13] qga: start a man page
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (11 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 12/13] qga: add --dump-conf option marcandre.lureau
@ 2015-08-26 23:34 ` marcandre.lureau
2015-08-28 21:44 ` Eric Blake
2015-08-27 16:32 ` [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file Denis V. Lunev
2015-08-27 23:47 ` Michael Roth
14 siblings, 1 reply; 29+ messages in thread
From: marcandre.lureau @ 2015-08-26 23:34 UTC (permalink / raw)
To: qemu-devel; +Cc: den, mdroth, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Add a simple man page for the qemu agent.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
Makefile | 14 +++++-
qemu-doc.texi | 6 +++
qemu-ga.texi | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 154 insertions(+), 2 deletions(-)
create mode 100644 qemu-ga.texi
diff --git a/Makefile b/Makefile
index 340d9c8..67d44b8 100644
--- a/Makefile
+++ b/Makefile
@@ -88,7 +88,8 @@ LIBS+=-lz $(LIBS_TOOLS)
HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
ifdef BUILD_DOCS
-DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 qmp-commands.txt
+DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
+DOCS+=qmp-commands.txt
ifdef CONFIG_LINUX
DOCS+=kvm_stat.1
endif
@@ -400,6 +401,9 @@ ifneq ($(TOOLS),)
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man8"
$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
endif
+ifneq (,$(findstring qemu-ga,$(TOOLS)))
+ $(INSTALL_DATA) qemu-ga.8 "$(DESTDIR)$(mandir)/man8"
+endif
endif
ifdef CONFIG_VIRTFS
$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
@@ -538,6 +542,12 @@ qemu-nbd.8: qemu-nbd.texi
$(POD2MAN) --section=8 --center=" " --release=" " qemu-nbd.pod > $@, \
" GEN $@")
+qemu-ga.8: qemu-ga.texi
+ $(call quiet-command, \
+ perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< qemu-ga.pod && \
+ $(POD2MAN) --section=8 --center=" " --release=" " qemu-ga.pod > $@, \
+ " GEN $@")
+
kvm_stat.1: scripts/kvm/kvm_stat.texi
$(call quiet-command, \
perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< kvm_stat.pod && \
@@ -551,7 +561,7 @@ pdf: qemu-doc.pdf qemu-tech.pdf
qemu-doc.dvi qemu-doc.html qemu-doc.info qemu-doc.pdf: \
qemu-img.texi qemu-nbd.texi qemu-options.texi \
- qemu-monitor.texi qemu-img-cmds.texi
+ qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi
ifdef CONFIG_WIN32
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 94af8c0..84d17d1 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -412,6 +412,7 @@ snapshots.
* vm_snapshots:: VM snapshots
* qemu_img_invocation:: qemu-img Invocation
* qemu_nbd_invocation:: qemu-nbd Invocation
+* qemu_ga_invocation:: qemu-ga Invocation
* disk_images_formats:: Disk image file formats
* host_drives:: Using host drives
* disk_images_fat_images:: Virtual FAT disk images
@@ -505,6 +506,11 @@ state is not saved or restored properly (in particular USB).
@include qemu-nbd.texi
+@node qemu_ga_invocation
+@subsection @code{qemu-ga} Invocation
+
+@include qemu-ga.texi
+
@node disk_images_formats
@subsection Disk image file formats
diff --git a/qemu-ga.texi b/qemu-ga.texi
new file mode 100644
index 0000000..cd1bc23
--- /dev/null
+++ b/qemu-ga.texi
@@ -0,0 +1,136 @@
+@example
+@c man begin SYNOPSIS
+usage: qemu-ga [-m <method> -p <path>] [OPTION]...
+@c man end
+@end example
+
+@c man begin DESCRIPTION
+
+The QEMU Guest Agent is a daemon that allows the host to perform
+various operations in the guest, such as:
+
+@itemize
+@item
+get information from the guest
+@item
+set the guest's system time
+@item
+read/write a file
+@item
+sync and freeze the filesystems
+@item
+suspend the guest
+@item
+reconfigure guest local processors
+@item
+set user's password
+@item
+...
+@end itemize
+
+qemu-ga will read a system configuration file on startup (located at
+q@file{/etc/qemu/qemu-ga.conf} by default), then parse remaining
+configuration options on the command line. For the same key, the last
+option wins, but the lists accumulate (see below for configuration
+file format).
+
+@c man end
+
+@c man begin OPTIONS
+@table @option
+@item -m, --method=@var{method}
+ Transport method: one of @samp{unix-listen}, @samp{virtio-serial}, or
+ @samp{isa-serial} (@samp{virtio-serial} is the default).
+
+@item -p, --path=@var{path}
+ Device/socket path (the default for virtio-serial is:
+ @samp{/dev/virtio-ports/org.qemu.guest_agent.0},
+ the default for isa-serial is: @samp{/dev/ttyS0})
+
+@item -l, --logfile=@var{path}
+ Set log file path, logs to stderr by default.
+
+@item -f, --pidfile=@var{path}
+ Specify pid file (default is @samp{/var/run/qemu-ga.pid}).
+
+@item -F, --fsfreeze-hook=@var{path}
+ Enable fsfreeze hook. Accepts an optional argument that specifies
+ script to run on freeze/thaw. Script will be called with
+ 'freeze'/'thaw' arguments accordingly. (default is
+ @samp{/etc/qemu/fsfreeze-hook}) If using -F with an argument, do
+ not follow -F with a space. (for example:
+ @samp{-F/var/run/fsfreezehook.sh})
+
+@item -t, --statedir=@var{path}
+ Specify the directory to store state information (absolute paths only,
+ default is @samp{/var/run}).
+
+@item -v, --verbose
+ Log extra debugging information.
+
+@item -V, --version
+ Print version information and exit.
+
+@item -d, --daemon
+ Daemonize after startup (detach from terminal).
+
+@item -b, --blacklist=@var{list}
+ Comma-separated list of RPCs to disable (no spaces, @samp{?} to list
+ available RPCs).
+
+@item -D, --dump-conf
+ Dump the configuration in a format compatible with @file{qemu-ga.conf}
+ and exit.
+
+@item -h, --help
+ Display this help and exit.
+@end table
+
+@c man end
+
+@c man begin FILES
+
+The syntax of the @file{qemu-ga.conf} configuration file follows the
+Desktop Entry Specification, here is a quick summary: it consists of
+groups of key-value pairs, interspersed with comments.
+
+@example
+# qemu-ga configuration sample
+[general]
+daemonize = 0
+pidfile = /var/run/qemu-ga.pid
+verbose = 0
+method = virtio-serial
+path = /dev/virtio-ports/org.qemu.guest_agent.0
+statedir = /var/run
+@end example
+
+The list of keys follows the command line options:
+@table @option
+@item daemon= boolean
+@item method= string
+@item path= string
+@item logfile= string
+@item pidfile= string
+@item fsfreeze-hook= string
+@item statedir= string
+@item verbose= boolean
+@item blacklist= string list
+@end table
+
+@c man end
+
+@ignore
+
+@setfilename qemu-ga
+@settitle QEMU Guest Agent
+
+@c man begin AUTHOR
+Michael Roth <mdroth@linux.vnet.ibm.com>
+@c man end
+
+@c man begin SEEALSO
+qemu(1)
+@c man end
+
+@end ignore
--
2.4.3
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 13/13] qga: start a man page
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 13/13] qga: start a man page marcandre.lureau
@ 2015-08-28 21:44 ` Eric Blake
2015-08-31 23:28 ` Michael Roth
0 siblings, 1 reply; 29+ messages in thread
From: Eric Blake @ 2015-08-28 21:44 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: den, mdroth
[-- Attachment #1: Type: text/plain, Size: 3904 bytes --]
On 08/26/2015 05:34 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Add a simple man page for the qemu agent.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
> Makefile | 14 +++++-
> qemu-doc.texi | 6 +++
> qemu-ga.texi | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 154 insertions(+), 2 deletions(-)
> create mode 100644 qemu-ga.texi
>
> +++ b/qemu-ga.texi
> @@ -0,0 +1,136 @@
> +@example
> +@c man begin SYNOPSIS
> +usage: qemu-ga [-m <method> -p <path>] [OPTION]...
This implies that -m/-p are linked (either both or neither must be
specified); and is a bit confusing since they are also OPTIONs (why are
they called out when the others are not). Might be simpler to just use:
usage: qemu-ga [OPTION]...
> +@c man end
> +@end example
> +
> +@c man begin DESCRIPTION
> +
> +The QEMU Guest Agent is a daemon that allows the host to perform
> +various operations in the guest, such as:
How about adding some words to make it obvious that it is not useful on
bare metal and is intended for virtualized guests:
The QEMU Guest Agent is a daemon intended to be run in virtual machines.
It allows the hypervisor host to perform various operations in the
guest, such as:
> +@c man begin OPTIONS
> +@table @option
> +@item -m, --method=@var{method}
> + Transport method: one of @samp{unix-listen}, @samp{virtio-serial}, or
> + @samp{isa-serial} (@samp{virtio-serial} is the default).
> +
> +@item -p, --path=@var{path}
> + Device/socket path (the default for virtio-serial is:
> + @samp{/dev/virtio-ports/org.qemu.guest_agent.0},
> + the default for isa-serial is: @samp{/dev/ttyS0})
s/is:/is/ in both places
> +
> +@item -l, --logfile=@var{path}
> + Set log file path, logs to stderr by default.
Also, 'path' is an annoying term; POSIX folks use it for any file name,
while GNU folks claim it should only be used for colon-separated lists
of directories as in PATH=. 'name' or 'location' might be better,
although I personally don't care strongly enough to reject on just that
choice of naming.
Elsewhere you've listed defaults in (); maybe:
Set log file (default is stderr).
> +
> +@item -f, --pidfile=@var{path}
> + Specify pid file (default is @samp{/var/run/qemu-ga.pid}).
Same bit about 'path', here and below (I'll quit pointing it out).
> +
> +@item -F, --fsfreeze-hook=@var{path}
> + Enable fsfreeze hook. Accepts an optional argument that specifies
> + script to run on freeze/thaw. Script will be called with
> + 'freeze'/'thaw' arguments accordingly. (default is
> + @samp{/etc/qemu/fsfreeze-hook}) If using -F with an argument, do
Place the '.' after the (), not before, since the 'If using' sentence
starts a new thought.
> + not follow -F with a space. (for example:
> + @samp{-F/var/run/fsfreezehook.sh})
And again.
[aside: short options that take an optional argument are wonky; POSIX
discourages their use in new programs, and GNU recommends that only long
options have optional arguments. But we've already shipped it that way,
so too late to change it now]
> +@item -b, --blacklist=@var{list}
> + Comma-separated list of RPCs to disable (no spaces, @samp{?} to list
> + available RPCs).
> +
'?' requires shell quoting to avoid unintended globbing based on the
contents of the current directory; in the past, we've updated 'qemu' to
take 'help' as a synonym anywhere that '?' was special, so that you can
query without worrying about quoting. We should probably do likewise
for the agent daemon; but as a separate patch.
Overall, fairly clean.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 13/13] qga: start a man page
2015-08-28 21:44 ` Eric Blake
@ 2015-08-31 23:28 ` Michael Roth
2015-09-01 15:11 ` Eric Blake
0 siblings, 1 reply; 29+ messages in thread
From: Michael Roth @ 2015-08-31 23:28 UTC (permalink / raw)
To: Eric Blake, marcandre.lureau, qemu-devel; +Cc: den
Quoting Eric Blake (2015-08-28 16:44:49)
> On 08/26/2015 05:34 PM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Add a simple man page for the qemu agent.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> > Makefile | 14 +++++-
> > qemu-doc.texi | 6 +++
> > qemu-ga.texi | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 154 insertions(+), 2 deletions(-)
> > create mode 100644 qemu-ga.texi
> >
>
> > +++ b/qemu-ga.texi
> > @@ -0,0 +1,136 @@
> > +@example
> > +@c man begin SYNOPSIS
> > +usage: qemu-ga [-m <method> -p <path>] [OPTION]...
>
> This implies that -m/-p are linked (either both or neither must be
> specified); and is a bit confusing since they are also OPTIONs (why are
> they called out when the others are not). Might be simpler to just use:
>
> usage: qemu-ga [OPTION]...
>
> > +@c man end
> > +@end example
> > +
> > +@c man begin DESCRIPTION
> > +
> > +The QEMU Guest Agent is a daemon that allows the host to perform
> > +various operations in the guest, such as:
>
> How about adding some words to make it obvious that it is not useful on
> bare metal and is intended for virtualized guests:
>
> The QEMU Guest Agent is a daemon intended to be run in virtual machines.
> It allows the hypervisor host to perform various operations in the
> guest, such as:
>
> > +@c man begin OPTIONS
> > +@table @option
> > +@item -m, --method=@var{method}
> > + Transport method: one of @samp{unix-listen}, @samp{virtio-serial}, or
> > + @samp{isa-serial} (@samp{virtio-serial} is the default).
> > +
> > +@item -p, --path=@var{path}
> > + Device/socket path (the default for virtio-serial is:
> > + @samp{/dev/virtio-ports/org.qemu.guest_agent.0},
> > + the default for isa-serial is: @samp{/dev/ttyS0})
>
> s/is:/is/ in both places
>
> > +
> > +@item -l, --logfile=@var{path}
> > + Set log file path, logs to stderr by default.
>
> Also, 'path' is an annoying term; POSIX folks use it for any file name,
> while GNU folks claim it should only be used for colon-separated lists
> of directories as in PATH=. 'name' or 'location' might be better,
> although I personally don't care strongly enough to reject on just that
> choice of naming.
>
> Elsewhere you've listed defaults in (); maybe:
>
> Set log file (default is stderr).
>
> > +
> > +@item -f, --pidfile=@var{path}
> > + Specify pid file (default is @samp{/var/run/qemu-ga.pid}).
>
> Same bit about 'path', here and below (I'll quit pointing it out).
>
> > +
> > +@item -F, --fsfreeze-hook=@var{path}
> > + Enable fsfreeze hook. Accepts an optional argument that specifies
> > + script to run on freeze/thaw. Script will be called with
> > + 'freeze'/'thaw' arguments accordingly. (default is
> > + @samp{/etc/qemu/fsfreeze-hook}) If using -F with an argument, do
>
> Place the '.' after the (), not before, since the 'If using' sentence
> starts a new thought.
>
> > + not follow -F with a space. (for example:
> > + @samp{-F/var/run/fsfreezehook.sh})
>
> And again.
>
> [aside: short options that take an optional argument are wonky; POSIX
> discourages their use in new programs, and GNU recommends that only long
> options have optional arguments. But we've already shipped it that way,
> so too late to change it now]
>
> > +@item -b, --blacklist=@var{list}
> > + Comma-separated list of RPCs to disable (no spaces, @samp{?} to list
> > + available RPCs).
> > +
>
> '?' requires shell quoting to avoid unintended globbing based on the
> contents of the current directory; in the past, we've updated 'qemu' to
> take 'help' as a synonym anywhere that '?' was special, so that you can
> query without worrying about quoting. We should probably do likewise
> for the agent daemon; but as a separate patch.
>
> Overall, fairly clean.
Thanks for the review. Since these are grammatical fixes I've gone ahead and
squashed these changes in, the only exception being suggestions regarding
'path' (the POSIX interpretation seems to make much more sense than associating
it with an unrelated environment variable's semantics), and the ones that are
outside the scope of this patch (short option args and 'help' as an alternative
to '?').
https://github.com/mdroth/qemu/commit/0e58c84d8618532bf6c3dcd3d13139fed5aaf3ca
Sending a pull later this week, can add your Reviewed-by if it looks okay.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 13/13] qga: start a man page
2015-08-31 23:28 ` Michael Roth
@ 2015-09-01 15:11 ` Eric Blake
0 siblings, 0 replies; 29+ messages in thread
From: Eric Blake @ 2015-09-01 15:11 UTC (permalink / raw)
To: Michael Roth, marcandre.lureau, qemu-devel; +Cc: den
[-- Attachment #1: Type: text/plain, Size: 887 bytes --]
On 08/31/2015 05:28 PM, Michael Roth wrote:
>> Overall, fairly clean.
>
> Thanks for the review. Since these are grammatical fixes I've gone ahead and
> squashed these changes in, the only exception being suggestions regarding
> 'path' (the POSIX interpretation seems to make much more sense than associating
> it with an unrelated environment variable's semantics), and the ones that are
> outside the scope of this patch (short option args and 'help' as an alternative
> to '?').
>
> https://github.com/mdroth/qemu/commit/0e58c84d8618532bf6c3dcd3d13139fed5aaf3ca
>
> Sending a pull later this week, can add your Reviewed-by if it looks okay.
I've looked that over, and I'm okay if you want to amend and add:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (12 preceding siblings ...)
2015-08-26 23:34 ` [Qemu-devel] [PATCH v4 13/13] qga: start a man page marcandre.lureau
@ 2015-08-27 16:32 ` Denis V. Lunev
2015-08-27 23:47 ` Michael Roth
14 siblings, 0 replies; 29+ messages in thread
From: Denis V. Lunev @ 2015-08-27 16:32 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: mdroth
On 08/27/2015 02:34 AM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> The following patches for the qemu agent add support for an optionnal
> configuration file, and a man page.
>
> v3->v4:
> - some spelling fixes
> - add a patch to allocate in split_list()
> - add more details in "copy argument strings" patch
> - add some g_free() in config_parse()
> - add an assert() after g_key_file_new()
> v2->v3:
> - fix compilation in intermediate patch
> - remove some extra space in intermediate patch
> - add some missing Reviewed-by tags
> v1->v2:
> - spelling fixes
> - change device_path to channel_path
> - moving config to GAConfig struct
> - do check_is_frozen() during main
> - use g_key_file_to_data() for the dump
>
> This is related to this RFE:
> https://bugzilla.redhat.com/show_bug.cgi?id=1101556
>
> Marc-André Lureau (13):
> qga: misc spelling
> qga: use exit() when parsing options
> qga: move string split in separate function
> qga: make split_list() return allocated strings
> qga: rename 'path' to 'channel_path'
> qga: copy argument strings
> qga: move option parsing to separate function
> qga: fill default options in main()
> qga: move agent run in a separate function
> qga: free a bit more
> qga: add an optional qemu-ga.conf system configuration
> qga: add --dump-conf option
> qga: start a man page
>
> Makefile | 14 +-
> qemu-doc.texi | 6 +
> qemu-ga.texi | 136 +++++++++++++++
> qga/commands-posix.c | 6 +-
> qga/commands-win32.c | 4 +-
> qga/main.c | 468 ++++++++++++++++++++++++++++++++++++---------------
> qga/qapi-schema.json | 2 +-
> 7 files changed, 490 insertions(+), 146 deletions(-)
> create mode 100644 qemu-ga.texi
>
There are 2 really small notes which are quite minor.
Thus for entire stuff except man page, which I am
not a right person to review, but you can add my
R-b at your taste.
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file
2015-08-26 23:34 [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file marcandre.lureau
` (13 preceding siblings ...)
2015-08-27 16:32 ` [Qemu-devel] [PATCH v4 00/13] qemu-ga: add a configuration file Denis V. Lunev
@ 2015-08-27 23:47 ` Michael Roth
14 siblings, 0 replies; 29+ messages in thread
From: Michael Roth @ 2015-08-27 23:47 UTC (permalink / raw)
To: marcandre.lureau, qemu-devel; +Cc: den
Quoting marcandre.lureau@redhat.com (2015-08-26 18:34:46)
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> The following patches for the qemu agent add support for an optionnal
> configuration file, and a man page.
>
> v3->v4:
> - some spelling fixes
> - add a patch to allocate in split_list()
> - add more details in "copy argument strings" patch
> - add some g_free() in config_parse()
> - add an assert() after g_key_file_new()
Thanks, applied to qga tree with fix-ups noted in 11+12:
https://github.com/mdroth/qemu/commits/qga
> v2->v3:
> - fix compilation in intermediate patch
> - remove some extra space in intermediate patch
> - add some missing Reviewed-by tags
> v1->v2:
> - spelling fixes
> - change device_path to channel_path
> - moving config to GAConfig struct
> - do check_is_frozen() during main
> - use g_key_file_to_data() for the dump
>
> This is related to this RFE:
> https://bugzilla.redhat.com/show_bug.cgi?id=1101556
>
> Marc-André Lureau (13):
> qga: misc spelling
> qga: use exit() when parsing options
> qga: move string split in separate function
> qga: make split_list() return allocated strings
> qga: rename 'path' to 'channel_path'
> qga: copy argument strings
> qga: move option parsing to separate function
> qga: fill default options in main()
> qga: move agent run in a separate function
> qga: free a bit more
> qga: add an optional qemu-ga.conf system configuration
> qga: add --dump-conf option
> qga: start a man page
>
> Makefile | 14 +-
> qemu-doc.texi | 6 +
> qemu-ga.texi | 136 +++++++++++++++
> qga/commands-posix.c | 6 +-
> qga/commands-win32.c | 4 +-
> qga/main.c | 468 ++++++++++++++++++++++++++++++++++++---------------
> qga/qapi-schema.json | 2 +-
> 7 files changed, 490 insertions(+), 146 deletions(-)
> create mode 100644 qemu-ga.texi
>
> --
> 2.4.3
>
^ permalink raw reply [flat|nested] 29+ messages in thread