* [Qemu-devel] [PATCH] pr-helper: Rework socket path handling
@ 2018-07-03 9:51 Michal Privoznik
2018-07-03 10:01 ` Paolo Bonzini
0 siblings, 1 reply; 2+ messages in thread
From: Michal Privoznik @ 2018-07-03 9:51 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, famz, qemu-block
When reviewing Paolo's pr-helper patches I've noticed couple of
problems:
1) socket_path needs to be calculated at two different places
(one for printing out help, the other if socket activation is NOT
used),
2) even though the default socket_path is allocated in
compute_default_paths() it is the only default path the function
handles. For instance, pidfile is allocated outside of this
function. And yet again, at different places than 1)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
scsi/qemu-pr-helper.c | 36 ++++++++++--------------------------
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 0218d65bbf..59df3fd608 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -76,14 +76,12 @@ static int gid = -1;
static void compute_default_paths(void)
{
- if (!socket_path) {
- socket_path = qemu_get_local_state_pathname("run/qemu-pr-helper.sock");
- }
+ socket_path = qemu_get_local_state_pathname("run/qemu-pr-helper.sock");
+ pidfile = qemu_get_local_state_pathname("run/qemu-pr-helper.pid");
}
static void usage(const char *name)
{
- compute_default_paths();
(printf) (
"Usage: %s [OPTIONS] FILE\n"
"Persistent Reservation helper program for QEMU\n"
@@ -844,19 +842,6 @@ static gboolean accept_client(QIOChannel *ioc, GIOCondition cond, gpointer opaqu
return TRUE;
}
-
-/*
- * Check socket parameters compatibility when socket activation is used.
- */
-static const char *socket_activation_validate_opts(void)
-{
- if (socket_path != NULL) {
- return "Unix socket can't be set when using socket activation";
- }
-
- return NULL;
-}
-
static void termsig_handler(int signum)
{
atomic_cmpxchg(&state, RUNNING, TERMINATE);
@@ -930,6 +915,7 @@ int main(int argc, char **argv)
char *trace_file = NULL;
bool daemonize = false;
bool pidfile_specified = false;
+ bool socket_path_specified = false;
unsigned socket_activation;
struct sigaction sa_sigterm;
@@ -946,12 +932,14 @@ int main(int argc, char **argv)
qemu_add_opts(&qemu_trace_opts);
qemu_init_exec_dir(argv[0]);
- pidfile = qemu_get_local_state_pathname("run/qemu-pr-helper.pid");
+ compute_default_paths();
while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
switch (ch) {
case 'k':
- socket_path = optarg;
+ g_free(socket_path);
+ socket_path = g_strdup(optarg);
+ socket_path_specified = true;
if (socket_path[0] != '/') {
error_report("socket path must be absolute");
exit(EXIT_FAILURE);
@@ -1042,10 +1030,9 @@ int main(int argc, char **argv)
socket_activation = check_socket_activation();
if (socket_activation == 0) {
SocketAddress saddr;
- compute_default_paths();
saddr = (SocketAddress){
.type = SOCKET_ADDRESS_TYPE_UNIX,
- .u.q_unix.path = g_strdup(socket_path)
+ .u.q_unix.path = socket_path,
};
server_ioc = qio_channel_socket_new();
if (qio_channel_socket_listen_sync(server_ioc, &saddr, &local_err) < 0) {
@@ -1053,12 +1040,10 @@ int main(int argc, char **argv)
error_report_err(local_err);
return 1;
}
- g_free(saddr.u.q_unix.path);
} else {
/* Using socket activation - check user didn't use -p etc. */
- const char *err_msg = socket_activation_validate_opts();
- if (err_msg != NULL) {
- error_report("%s", err_msg);
+ if (socket_path_specified) {
+ error_report("Unix socket can't be set when using socket activation");
exit(EXIT_FAILURE);
}
@@ -1075,7 +1060,6 @@ int main(int argc, char **argv)
error_get_pretty(local_err));
exit(EXIT_FAILURE);
}
- socket_path = NULL;
}
if (qemu_init_main_loop(&local_err)) {
--
2.16.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] pr-helper: Rework socket path handling
2018-07-03 9:51 [Qemu-devel] [PATCH] pr-helper: Rework socket path handling Michal Privoznik
@ 2018-07-03 10:01 ` Paolo Bonzini
0 siblings, 0 replies; 2+ messages in thread
From: Paolo Bonzini @ 2018-07-03 10:01 UTC (permalink / raw)
To: Michal Privoznik, qemu-devel; +Cc: famz, qemu-block
On 03/07/2018 11:51, Michal Privoznik wrote:
> When reviewing Paolo's pr-helper patches I've noticed couple of
> problems:
>
> 1) socket_path needs to be calculated at two different places
> (one for printing out help, the other if socket activation is NOT
> used),
>
> 2) even though the default socket_path is allocated in
> compute_default_paths() it is the only default path the function
> handles. For instance, pidfile is allocated outside of this
> function. And yet again, at different places than 1)
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Queued, thanks.
Paolo
> ---
> scsi/qemu-pr-helper.c | 36 ++++++++++--------------------------
> 1 file changed, 10 insertions(+), 26 deletions(-)
>
> diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
> index 0218d65bbf..59df3fd608 100644
> --- a/scsi/qemu-pr-helper.c
> +++ b/scsi/qemu-pr-helper.c
> @@ -76,14 +76,12 @@ static int gid = -1;
>
> static void compute_default_paths(void)
> {
> - if (!socket_path) {
> - socket_path = qemu_get_local_state_pathname("run/qemu-pr-helper.sock");
> - }
> + socket_path = qemu_get_local_state_pathname("run/qemu-pr-helper.sock");
> + pidfile = qemu_get_local_state_pathname("run/qemu-pr-helper.pid");
> }
>
> static void usage(const char *name)
> {
> - compute_default_paths();
> (printf) (
> "Usage: %s [OPTIONS] FILE\n"
> "Persistent Reservation helper program for QEMU\n"
> @@ -844,19 +842,6 @@ static gboolean accept_client(QIOChannel *ioc, GIOCondition cond, gpointer opaqu
> return TRUE;
> }
>
> -
> -/*
> - * Check socket parameters compatibility when socket activation is used.
> - */
> -static const char *socket_activation_validate_opts(void)
> -{
> - if (socket_path != NULL) {
> - return "Unix socket can't be set when using socket activation";
> - }
> -
> - return NULL;
> -}
> -
> static void termsig_handler(int signum)
> {
> atomic_cmpxchg(&state, RUNNING, TERMINATE);
> @@ -930,6 +915,7 @@ int main(int argc, char **argv)
> char *trace_file = NULL;
> bool daemonize = false;
> bool pidfile_specified = false;
> + bool socket_path_specified = false;
> unsigned socket_activation;
>
> struct sigaction sa_sigterm;
> @@ -946,12 +932,14 @@ int main(int argc, char **argv)
> qemu_add_opts(&qemu_trace_opts);
> qemu_init_exec_dir(argv[0]);
>
> - pidfile = qemu_get_local_state_pathname("run/qemu-pr-helper.pid");
> + compute_default_paths();
>
> while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
> switch (ch) {
> case 'k':
> - socket_path = optarg;
> + g_free(socket_path);
> + socket_path = g_strdup(optarg);
> + socket_path_specified = true;
> if (socket_path[0] != '/') {
> error_report("socket path must be absolute");
> exit(EXIT_FAILURE);
> @@ -1042,10 +1030,9 @@ int main(int argc, char **argv)
> socket_activation = check_socket_activation();
> if (socket_activation == 0) {
> SocketAddress saddr;
> - compute_default_paths();
> saddr = (SocketAddress){
> .type = SOCKET_ADDRESS_TYPE_UNIX,
> - .u.q_unix.path = g_strdup(socket_path)
> + .u.q_unix.path = socket_path,
> };
> server_ioc = qio_channel_socket_new();
> if (qio_channel_socket_listen_sync(server_ioc, &saddr, &local_err) < 0) {
> @@ -1053,12 +1040,10 @@ int main(int argc, char **argv)
> error_report_err(local_err);
> return 1;
> }
> - g_free(saddr.u.q_unix.path);
> } else {
> /* Using socket activation - check user didn't use -p etc. */
> - const char *err_msg = socket_activation_validate_opts();
> - if (err_msg != NULL) {
> - error_report("%s", err_msg);
> + if (socket_path_specified) {
> + error_report("Unix socket can't be set when using socket activation");
> exit(EXIT_FAILURE);
> }
>
> @@ -1075,7 +1060,6 @@ int main(int argc, char **argv)
> error_get_pretty(local_err));
> exit(EXIT_FAILURE);
> }
> - socket_path = NULL;
> }
>
> if (qemu_init_main_loop(&local_err)) {
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-07-03 10:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-03 9:51 [Qemu-devel] [PATCH] pr-helper: Rework socket path handling Michal Privoznik
2018-07-03 10:01 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).