Re: [Qemu-devel] [PATCH 1/2] create ga_run_program() helper for guest-set-user-password

2016-01-11 Thread Denis V. Lunev 

On 01/11/2016 03:08 PM, Yuriy Pudgorodskiy wrote:

On 1/6/2016 4:50 PM, Denis V. Lunev wrote:

hmmm

Yur,

it seems that you have re-invented the wheel with

gboolean 

g_spawn_sync (/|const gchar 
 
*working_directory|/,
  /|gchar 
 
**argv|/,
  /|gchar 
 
**envp|/,
  /|GSpawnFlags 
 
flags|/,
  /|GSpawnChildSetupFunc 
 
child_setup|/,
  /|gpointer 
 
user_data|/,
  /|gchar 
 
**standard_output|/,
  /|gchar 
 
**standard_error|/,
  /|gint 
 
*exit_status|/,
  /|GError 
 
**error|/);





Not exactly.

g_spawn_sync() wrapper cannot be used directly instead of my wrapper,
because it does not support redirected stdin which is required to send 
data to chpasswd.
g_spawn_async_with_pipes() can be used (with stdin), but it will not 
make code simpler.


Most of command-posix.c calls fork/execv directly, and so does the 
wrapper I wrote.




this seems that you are right :)

Then nothing needs to be changed and patches are OK.

Den

Re: [Qemu-devel] [PATCH 1/2] create ga_run_program() helper for guest-set-user-password

2016-01-11 Thread Yuriy Pudgorodskiy 

On 1/6/2016 4:50 PM, Denis V. Lunev wrote:

hmmm

Yur,

it seems that you have re-invented the wheel with

gboolean 

g_spawn_sync (/|const gchar 
 
*working_directory|/,
  /|gchar 
 
**argv|/,
  /|gchar 
 
**envp|/,
  /|GSpawnFlags 
 
flags|/,
  /|GSpawnChildSetupFunc 
 
child_setup|/,
  /|gpointer 
 
user_data|/,
  /|gchar 
 
**standard_output|/,
  /|gchar 
 
**standard_error|/,
  /|gint 
 
*exit_status|/,
  /|GError 
 
**error|/);





Not exactly.

g_spawn_sync() wrapper cannot be used directly instead of my wrapper,
because it does not support redirected stdin which is required to send 
data to chpasswd.
g_spawn_async_with_pipes() can be used (with stdin), but it will not 
make code simpler.


Most of command-posix.c calls fork/execv directly, and so does the 
wrapper I wrote.

[Qemu-devel] [PATCH 1/2] create ga_run_program() helper for guest-set-user-password

2016-01-06 Thread Denis V. Lunev 
From: Yuriy Pudgorodskiy 

This helper properly starts chpasswd and collects stdout/stderr of this
program to report it as error to the caller.

The code will be reused later to run useradd in addition to chpasswd.

This code is made specifically for Linux and is inside ifdef Linux braces.

Signed-off-by: Yuri Pudgorodskiy 
Signed-off-by: Denis V. Lunev 
CC: Eric Blake 
CC: Michael Roth 
---
 qga/commands-posix.c | 201 ---
 1 file changed, 141 insertions(+), 60 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 8fe708f..53e8d3b 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -75,6 +75,28 @@ static void ga_wait_child(pid_t pid, int *status, Error 
**errp)
 g_assert(rpid == pid);
 }
 
+static void ga_pipe_read_str(int fd[2], char **str, size_t *len)
+{
+ssize_t n;
+char buf[1024];
+close(fd[1]);
+fd[1] = -1;
+while ((n = read(fd[0], buf, sizeof(buf))) != 0) {
+if (n < 0) {
+if (errno == EINTR) {
+continue;
+} else {
+break;
+}
+}
+*str = g_realloc(*str, *len + n);
+memcpy(*str + *len, buf, n);
+*len += n;
+}
+close(fd[0]);
+fd[0] = -1;
+}
+
 void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
 {
 const char *shutdown_flag;
@@ -1952,20 +1974,128 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList 
*vcpus, Error **errp)
 return processed;
 }
 
+/* Helper to run command with input/output redirection,
+ * sending string to stdin and taking error message from
+ * stdout/err
+ */
+static void ga_run_program(const char *argv[],
+   const char *in_str,
+   const char *action, Error **errp)
+{
+pid_t pid;
+int status;
+int infd[2] = { -1, -1 };
+int outfd[2] = { -1, -1 };
+char *str = NULL;
+size_t len = 0;
+
+if (in_str) {
+if (pipe(infd) < 0) {
+error_setg(errp, "cannot create pipe FDs");
+goto out;
+}
+}
+
+if (pipe(outfd) < 0) {
+error_setg(errp, "cannot create pipe FDs");
+goto out;
+}
+
+pid = fork();
+if (pid == 0) {
+/* child */
+setsid();
+if (in_str) {
+/* redirect stdin to infd */
+close(infd[1]);
+dup2(infd[0], 0);
+close(infd[0]);
+} else {
+reopen_fd_to_null(0);
+}
+
+/* redirect stdout/stderr to outfd */
+close(outfd[0]);
+dup2(outfd[1], 1);
+dup2(outfd[1], 2);
+close(outfd[1]);
+
+execve(argv[0], (char *const *)argv, environ);
+_exit(EXIT_FAILURE);
+} else if (pid < 0) {
+error_setg_errno(errp, errno, "failed to create child process");
+goto out;
+}
+
+if (in_str) {
+close(infd[0]);
+infd[0] = -1;
+if (qemu_write_full(infd[1],
+in_str, strlen(in_str)) != strlen(in_str)) {
+error_setg_errno(errp, errno,
+"%s: cannot write to stdin pipe", action);
+goto out;
+}
+close(infd[1]);
+infd[1] = -1;
+}
+
+
+ga_pipe_read_str(outfd, , );
+
+ga_wait_child(pid, , errp);
+if (*errp) {
+goto out;
+}
+
+if (!WIFEXITED(status)) {
+if (len) {
+error_setg(errp, "child process has terminated abnormally: "
+"%s", str);
+} else {
+error_setg(errp, "child process has terminated abnormally");
+}
+goto out;
+}
+
+if (WEXITSTATUS(status)) {
+if (len) {
+error_setg(errp, "child process has failed to %s: %s",
+action, str);
+} else {
+error_setg(errp, "child process has failed to %s: exit status %d",
+action, WEXITSTATUS(status));
+}
+goto out;
+}
+
+out:
+g_free(str);
+
+if (infd[0] != -1) {
+close(infd[0]);
+}
+if (infd[1] != -1) {
+close(infd[1]);
+}
+if (outfd[0] != -1) {
+close(outfd[0]);
+}
+if (outfd[1] != -1) {
+close(outfd[1]);
+}
+}
+
 void qmp_guest_set_user_password(const char *username,
  const char *password,
  bool crypted,
  Error **errp)
 {
-Error *local_err = NULL;
 char *passwd_path = NULL;
-pid_t pid;
-int status;
-int datafd[2] = { -1, -1 };
 char *rawpasswddata = NULL;
 size_t rawpasswdlen;
 char *chpasswddata = NULL;
-size_t chpasswdlen;
+const char *chpasswd_argv[] = { NULL /*path*/, "chpasswd", "-e", NULL };
 
 rawpasswddata = (char *)qbase64_decode(password, -1, , errp);
 if

Re: [Qemu-devel] [PATCH 1/2] create ga_run_program() helper for guest-set-user-password

2016-01-06 Thread Denis V. Lunev 

On 01/06/2016 03:01 PM, Denis V. Lunev wrote:

From: Yuriy Pudgorodskiy 

This helper properly starts chpasswd and collects stdout/stderr of this
program to report it as error to the caller.

The code will be reused later to run useradd in addition to chpasswd.

This code is made specifically for Linux and is inside ifdef Linux braces.

Signed-off-by: Yuri Pudgorodskiy 
Signed-off-by: Denis V. Lunev 
CC: Eric Blake 
CC: Michael Roth 
---
  qga/commands-posix.c | 201 ---
  1 file changed, 141 insertions(+), 60 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 8fe708f..53e8d3b 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -75,6 +75,28 @@ static void ga_wait_child(pid_t pid, int *status, Error 
**errp)
  g_assert(rpid == pid);
  }
  
+static void ga_pipe_read_str(int fd[2], char **str, size_t *len)

+{
+ssize_t n;
+char buf[1024];
+close(fd[1]);
+fd[1] = -1;
+while ((n = read(fd[0], buf, sizeof(buf))) != 0) {
+if (n < 0) {
+if (errno == EINTR) {
+continue;
+} else {
+break;
+}
+}
+*str = g_realloc(*str, *len + n);
+memcpy(*str + *len, buf, n);
+*len += n;
+}
+close(fd[0]);
+fd[0] = -1;
+}
+
  void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
  {
  const char *shutdown_flag;
@@ -1952,20 +1974,128 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList 
*vcpus, Error **errp)
  return processed;
  }
  
+/* Helper to run command with input/output redirection,

+ * sending string to stdin and taking error message from
+ * stdout/err
+ */
+static void ga_run_program(const char *argv[],
+   const char *in_str,
+   const char *action, Error **errp)
+{
+pid_t pid;
+int status;
+int infd[2] = { -1, -1 };
+int outfd[2] = { -1, -1 };
+char *str = NULL;
+size_t len = 0;
+
+if (in_str) {
+if (pipe(infd) < 0) {
+error_setg(errp, "cannot create pipe FDs");
+goto out;
+}
+}
+
+if (pipe(outfd) < 0) {
+error_setg(errp, "cannot create pipe FDs");
+goto out;
+}
+
+pid = fork();
+if (pid == 0) {
+/* child */
+setsid();
+if (in_str) {
+/* redirect stdin to infd */
+close(infd[1]);
+dup2(infd[0], 0);
+close(infd[0]);
+} else {
+reopen_fd_to_null(0);
+}
+
+/* redirect stdout/stderr to outfd */
+close(outfd[0]);
+dup2(outfd[1], 1);
+dup2(outfd[1], 2);
+close(outfd[1]);
+
+execve(argv[0], (char *const *)argv, environ);
+_exit(EXIT_FAILURE);
+} else if (pid < 0) {
+error_setg_errno(errp, errno, "failed to create child process");
+goto out;
+}
+
+if (in_str) {
+close(infd[0]);
+infd[0] = -1;
+if (qemu_write_full(infd[1],
+in_str, strlen(in_str)) != strlen(in_str)) {
+error_setg_errno(errp, errno,
+"%s: cannot write to stdin pipe", action);
+goto out;
+}
+close(infd[1]);
+infd[1] = -1;
+}
+
+
+ga_pipe_read_str(outfd, , );
+
+ga_wait_child(pid, , errp);
+if (*errp) {
+goto out;
+}
+
+if (!WIFEXITED(status)) {
+if (len) {
+error_setg(errp, "child process has terminated abnormally: "
+"%s", str);
+} else {
+error_setg(errp, "child process has terminated abnormally");
+}
+goto out;
+}
+
+if (WEXITSTATUS(status)) {
+if (len) {
+error_setg(errp, "child process has failed to %s: %s",
+action, str);
+} else {
+error_setg(errp, "child process has failed to %s: exit status %d",
+action, WEXITSTATUS(status));
+}
+goto out;
+}
+
+out:
+g_free(str);
+
+if (infd[0] != -1) {
+close(infd[0]);
+}
+if (infd[1] != -1) {
+close(infd[1]);
+}
+if (outfd[0] != -1) {
+close(outfd[0]);
+}
+if (outfd[1] != -1) {
+close(outfd[1]);
+}
+}
+
  void qmp_guest_set_user_password(const char *username,
   const char *password,
   bool crypted,
   Error **errp)
  {
-Error *local_err = NULL;
  char *passwd_path = NULL;
-pid_t pid;
-int status;
-int datafd[2] = { -1, -1 };
  char *rawpasswddata = NULL;
  size_t rawpasswdlen;
  char *chpasswddata = NULL;
-size_t chpasswdlen;
+const char *chpasswd_argv[] = { NULL /*path*/, "chpasswd", "-e", NULL };