| Message ID | 20220929075239.1675374-6-alexander.ivanov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qga: Add FreeBSD support | expand |
Hi Alexander On Thu, Sep 29, 2022 at 12:52 PM Alexander Ivanov < alexander.ivanov@virtuozzo.com> wrote: > Move qmp_guest_set_user_password() from __linux__ condition to > (__linux__ || __FreeBSD__) condition. Add command and arguments > for password setting in FreeBSD. > > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > Could you explain why you need to move the code? You could instead have a top declaration? If it's really required, please split the patch in move + additions. > --- > qga/commands-posix.c | 223 +++++++++++++++++++++++-------------------- > 1 file changed, 118 insertions(+), 105 deletions(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 88e0d0fe24..6ce894ca6e 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -2123,111 +2123,6 @@ int64_t > qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) > return processed; > } > > -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; > - > - rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, > errp); > - if (!rawpasswddata) { > - return; > - } > - rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1); > - rawpasswddata[rawpasswdlen] = '\0'; > - > - if (strchr(rawpasswddata, '\n')) { > - error_setg(errp, "forbidden characters in raw password"); > - goto out; > - } > - > - if (strchr(username, '\n') || > - strchr(username, ':')) { > - error_setg(errp, "forbidden characters in username"); > - goto out; > - } > - > - chpasswddata = g_strdup_printf("%s:%s\n", username, rawpasswddata); > - chpasswdlen = strlen(chpasswddata); > - > - passwd_path = g_find_program_in_path("chpasswd"); > - > - if (!passwd_path) { > - error_setg(errp, "cannot find 'passwd' program in PATH"); > - goto out; > - } > - > - if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { > - error_setg(errp, "cannot create pipe FDs"); > - goto out; > - } > - > - pid = fork(); > - if (pid == 0) { > - close(datafd[1]); > - /* child */ > - setsid(); > - dup2(datafd[0], 0); > - reopen_fd_to_null(1); > - reopen_fd_to_null(2); > - > - if (crypted) { > - execl(passwd_path, "chpasswd", "-e", NULL); > - } else { > - execl(passwd_path, "chpasswd", NULL); > - } > - _exit(EXIT_FAILURE); > - } else if (pid < 0) { > - error_setg_errno(errp, errno, "failed to create child process"); > - goto out; > - } > - close(datafd[0]); > - datafd[0] = -1; > - > - if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != > chpasswdlen) { > - error_setg_errno(errp, errno, "cannot write new account > password"); > - goto out; > - } > - close(datafd[1]); > - datafd[1] = -1; > - > - ga_wait_child(pid, &status, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > - goto out; > - } > - > - if (!WIFEXITED(status)) { > - error_setg(errp, "child process has terminated abnormally"); > - goto out; > - } > - > - if (WEXITSTATUS(status)) { > - error_setg(errp, "child process has failed to set user password"); > - goto out; > - } > - > -out: > - g_free(chpasswddata); > - g_free(rawpasswddata); > - g_free(passwd_path); > - if (datafd[0] != -1) { > - close(datafd[0]); > - } > - if (datafd[1] != -1) { > - close(datafd[1]); > - } > -} > - > static void ga_read_sysfs_file(int dirfd, const char *pathname, char *buf, > int size, Error **errp) > { > @@ -2793,6 +2688,124 @@ GuestMemoryBlockInfo > *qmp_guest_get_memory_block_info(Error **errp) > > #endif > > +#if defined(__linux__) || defined(__FreeBSD__) > +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; > + > + rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, > errp); > + if (!rawpasswddata) { > + return; > + } > + rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1); > + rawpasswddata[rawpasswdlen] = '\0'; > + > + if (strchr(rawpasswddata, '\n')) { > + error_setg(errp, "forbidden characters in raw password"); > + goto out; > + } > + > + if (strchr(username, '\n') || > + strchr(username, ':')) { > + error_setg(errp, "forbidden characters in username"); > + goto out; > + } > + > +#ifdef __FreeBSD__ > + chpasswddata = g_strdup(rawpasswddata); > + passwd_path = g_find_program_in_path("pw"); > +#else > + chpasswddata = g_strdup_printf("%s:%s\n", username, rawpasswddata); > + passwd_path = g_find_program_in_path("chpasswd"); > +#endif > + > + chpasswdlen = strlen(chpasswddata); > + > + if (!passwd_path) { > + error_setg(errp, "cannot find 'passwd' program in PATH"); > + goto out; > + } > + > + if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { > + error_setg(errp, "cannot create pipe FDs"); > + goto out; > + } > + > + pid = fork(); > + if (pid == 0) { > + close(datafd[1]); > + /* child */ > + setsid(); > + dup2(datafd[0], 0); > + reopen_fd_to_null(1); > + reopen_fd_to_null(2); > + > +#ifdef __FreeBSD__ > + const char *h_arg; > + h_arg = (crypted) ? "-H" : "-h"; > + execl(passwd_path, "pw", "usermod", "-n", username, h_arg, "0", > NULL); > +#else > + if (crypted) { > + execl(passwd_path, "chpasswd", "-e", NULL); > + } else { > + execl(passwd_path, "chpasswd", NULL); > + } > +#endif > + _exit(EXIT_FAILURE); > + } else if (pid < 0) { > + error_setg_errno(errp, errno, "failed to create child process"); > + goto out; > + } > + close(datafd[0]); > + datafd[0] = -1; > + > + if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != > chpasswdlen) { > + error_setg_errno(errp, errno, "cannot write new account > password"); > + goto out; > + } > + close(datafd[1]); > + datafd[1] = -1; > + > + ga_wait_child(pid, &status, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + goto out; > + } > + > + if (!WIFEXITED(status)) { > + error_setg(errp, "child process has terminated abnormally"); > + goto out; > + } > + > + if (WEXITSTATUS(status)) { > + error_setg(errp, "child process has failed to set user password"); > + goto out; > + } > + > +out: > + g_free(chpasswddata); > + g_free(rawpasswddata); > + g_free(passwd_path); > + if (datafd[0] != -1) { > + close(datafd[0]); > + } > + if (datafd[1] != -1) { > + close(datafd[1]); > + } > +} > +#endif > + > #ifdef HAVE_GETIFADDRS > static GuestNetworkInterface * > guest_find_interface(GuestNetworkInterfaceList *head, > -- > 2.34.1 > > >
On 29.09.2022 13:22, Marc-André Lureau wrote: > > > Caution: This is an external email and has a suspicious subject or > content. Please take care when clicking links or opening attachments. > When in doubt, contact your IT Department > > > Hi Alexander > > On Thu, Sep 29, 2022 at 12:52 PM Alexander Ivanov > <alexander.ivanov@virtuozzo.com> wrote: > > Move qmp_guest_set_user_password() from __linux__ condition to > (__linux__ || __FreeBSD__) condition. Add command and arguments > for password setting in FreeBSD. > > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > > > Could you explain why you need to move the code? > > You could instead have a top declaration? Now qmp_guest_set_user_password()is under #ifdef __linux__. I want to use this function for FreeBSD too, so I moved it under (__linux__ || __FreeBSD__) condition. I could create another function for FreeBSD but it would lead to code duplication. Unfortunately I don't see another way to do it except to add (__linux__ || __FreeBSD__) condition, cut the function and paste in this condition. If you could suggest another way I would happy. > If it's really required, please split the patch in move + additions. Just moving the function without other changes leads to unbuildable code in FreeBSD. I thought it's worse than moving and changing at the same time. I split the patch if you prefer. > > --- > qga/commands-posix.c | 223 > +++++++++++++++++++++++-------------------- > 1 file changed, 118 insertions(+), 105 deletions(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 88e0d0fe24..6ce894ca6e 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -2123,111 +2123,6 @@ int64_t > qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) > return processed; > } > > -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; > - > - rawpasswddata = (char *)qbase64_decode(password, -1, > &rawpasswdlen, errp); > - if (!rawpasswddata) { > - return; > - } > - rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1); > - rawpasswddata[rawpasswdlen] = '\0'; > - > - if (strchr(rawpasswddata, '\n')) { > - error_setg(errp, "forbidden characters in raw password"); > - goto out; > - } > - > - if (strchr(username, '\n') || > - strchr(username, ':')) { > - error_setg(errp, "forbidden characters in username"); > - goto out; > - } > - > - chpasswddata = g_strdup_printf("%s:%s\n", username, > rawpasswddata); > - chpasswdlen = strlen(chpasswddata); > - > - passwd_path = g_find_program_in_path("chpasswd"); > - > - if (!passwd_path) { > - error_setg(errp, "cannot find 'passwd' program in PATH"); > - goto out; > - } > - > - if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { > - error_setg(errp, "cannot create pipe FDs"); > - goto out; > - } > - > - pid = fork(); > - if (pid == 0) { > - close(datafd[1]); > - /* child */ > - setsid(); > - dup2(datafd[0], 0); > - reopen_fd_to_null(1); > - reopen_fd_to_null(2); > - > - if (crypted) { > - execl(passwd_path, "chpasswd", "-e", NULL); > - } else { > - execl(passwd_path, "chpasswd", NULL); > - } > - _exit(EXIT_FAILURE); > - } else if (pid < 0) { > - error_setg_errno(errp, errno, "failed to create child > process"); > - goto out; > - } > - close(datafd[0]); > - datafd[0] = -1; > - > - if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != > chpasswdlen) { > - error_setg_errno(errp, errno, "cannot write new account > password"); > - goto out; > - } > - close(datafd[1]); > - datafd[1] = -1; > - > - ga_wait_child(pid, &status, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > - goto out; > - } > - > - if (!WIFEXITED(status)) { > - error_setg(errp, "child process has terminated abnormally"); > - goto out; > - } > - > - if (WEXITSTATUS(status)) { > - error_setg(errp, "child process has failed to set user > password"); > - goto out; > - } > - > -out: > - g_free(chpasswddata); > - g_free(rawpasswddata); > - g_free(passwd_path); > - if (datafd[0] != -1) { > - close(datafd[0]); > - } > - if (datafd[1] != -1) { > - close(datafd[1]); > - } > -} > - > static void ga_read_sysfs_file(int dirfd, const char *pathname, > char *buf, > int size, Error **errp) > { > @@ -2793,6 +2688,124 @@ GuestMemoryBlockInfo > *qmp_guest_get_memory_block_info(Error **errp) > > #endif > > +#if defined(__linux__) || defined(__FreeBSD__) > +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; > + > + rawpasswddata = (char *)qbase64_decode(password, -1, > &rawpasswdlen, errp); > + if (!rawpasswddata) { > + return; > + } > + rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1); > + rawpasswddata[rawpasswdlen] = '\0'; > + > + if (strchr(rawpasswddata, '\n')) { > + error_setg(errp, "forbidden characters in raw password"); > + goto out; > + } > + > + if (strchr(username, '\n') || > + strchr(username, ':')) { > + error_setg(errp, "forbidden characters in username"); > + goto out; > + } > + > +#ifdef __FreeBSD__ > + chpasswddata = g_strdup(rawpasswddata); > + passwd_path = g_find_program_in_path("pw"); > +#else > + chpasswddata = g_strdup_printf("%s:%s\n", username, > rawpasswddata); > + passwd_path = g_find_program_in_path("chpasswd"); > +#endif > + > + chpasswdlen = strlen(chpasswddata); > + > + if (!passwd_path) { > + error_setg(errp, "cannot find 'passwd' program in PATH"); > + goto out; > + } > + > + if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { > + error_setg(errp, "cannot create pipe FDs"); > + goto out; > + } > + > + pid = fork(); > + if (pid == 0) { > + close(datafd[1]); > + /* child */ > + setsid(); > + dup2(datafd[0], 0); > + reopen_fd_to_null(1); > + reopen_fd_to_null(2); > + > +#ifdef __FreeBSD__ > + const char *h_arg; > + h_arg = (crypted) ? "-H" : "-h"; > + execl(passwd_path, "pw", "usermod", "-n", username, > h_arg, "0", NULL); > +#else > + if (crypted) { > + execl(passwd_path, "chpasswd", "-e", NULL); > + } else { > + execl(passwd_path, "chpasswd", NULL); > + } > +#endif > + _exit(EXIT_FAILURE); > + } else if (pid < 0) { > + error_setg_errno(errp, errno, "failed to create child > process"); > + goto out; > + } > + close(datafd[0]); > + datafd[0] = -1; > + > + if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != > chpasswdlen) { > + error_setg_errno(errp, errno, "cannot write new account > password"); > + goto out; > + } > + close(datafd[1]); > + datafd[1] = -1; > + > + ga_wait_child(pid, &status, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + goto out; > + } > + > + if (!WIFEXITED(status)) { > + error_setg(errp, "child process has terminated abnormally"); > + goto out; > + } > + > + if (WEXITSTATUS(status)) { > + error_setg(errp, "child process has failed to set user > password"); > + goto out; > + } > + > +out: > + g_free(chpasswddata); > + g_free(rawpasswddata); > + g_free(passwd_path); > + if (datafd[0] != -1) { > + close(datafd[0]); > + } > + if (datafd[1] != -1) { > + close(datafd[1]); > + } > +} > +#endif > + > #ifdef HAVE_GETIFADDRS > static GuestNetworkInterface * > guest_find_interface(GuestNetworkInterfaceList *head, > -- > 2.34.1 > > > > > -- > Marc-André Lureau
Hi On Thu, Sep 29, 2022 at 6:29 PM Alexander Ivanov < alexander.ivanov@virtuozzo.com> wrote: > > On 29.09.2022 13:22, Marc-André Lureau wrote: > > > > > > Caution: This is an external email and has a suspicious subject or > > content. Please take care when clicking links or opening attachments. > > When in doubt, contact your IT Department > > > > > > Hi Alexander > > > > On Thu, Sep 29, 2022 at 12:52 PM Alexander Ivanov > > <alexander.ivanov@virtuozzo.com> wrote: > > > > Move qmp_guest_set_user_password() from __linux__ condition to > > (__linux__ || __FreeBSD__) condition. Add command and arguments > > for password setting in FreeBSD. > > > > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > > > > > > Could you explain why you need to move the code? > > > > You could instead have a top declaration? > Now qmp_guest_set_user_password()is under #ifdef __linux__. I want to > use this function for FreeBSD too, so I moved it under (__linux__ || > __FreeBSD__) condition. I could create another function for FreeBSD but > Why not make the following change? diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 88e0d0fe24..78a345c9f3 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -2122,7 +2122,9 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) return processed; } +#endif /* linux */ +#if defined(__linux__) || defined(__FreeBSD__) void qmp_guest_set_user_password(const char *username, const char *password, bool crypted, @@ -2227,7 +2229,9 @@ out: close(datafd[1]); } } +#endif /* linux || freebsd */ +#if defined(__linux__) static void ga_read_sysfs_file(int dirfd, const char *pathname, char *buf, int size, Error **errp) { > it would lead to code duplication. Unfortunately I don't see another way > to do it except to add (__linux__ || __FreeBSD__) condition, cut the > function and paste in this condition. If you could suggest another way I > would happy. > > If it's really required, please split the patch in move + additions. > Just moving the function without other changes leads to unbuildable code > in FreeBSD. I thought it's worse than moving and changing at the same > time. I split the patch if you prefer. > > > > --- > > qga/commands-posix.c | 223 > > +++++++++++++++++++++++-------------------- > > 1 file changed, 118 insertions(+), 105 deletions(-) > > > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > > index 88e0d0fe24..6ce894ca6e 100644 > > --- a/qga/commands-posix.c > > +++ b/qga/commands-posix.c > > @@ -2123,111 +2123,6 @@ int64_t > > qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) > > return processed; > > } > > > > -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; > > - > > - rawpasswddata = (char *)qbase64_decode(password, -1, > > &rawpasswdlen, errp); > > - if (!rawpasswddata) { > > - return; > > - } > > - rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1); > > - rawpasswddata[rawpasswdlen] = '\0'; > > - > > - if (strchr(rawpasswddata, '\n')) { > > - error_setg(errp, "forbidden characters in raw password"); > > - goto out; > > - } > > - > > - if (strchr(username, '\n') || > > - strchr(username, ':')) { > > - error_setg(errp, "forbidden characters in username"); > > - goto out; > > - } > > - > > - chpasswddata = g_strdup_printf("%s:%s\n", username, > > rawpasswddata); > > - chpasswdlen = strlen(chpasswddata); > > - > > - passwd_path = g_find_program_in_path("chpasswd"); > > - > > - if (!passwd_path) { > > - error_setg(errp, "cannot find 'passwd' program in PATH"); > > - goto out; > > - } > > - > > - if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { > > - error_setg(errp, "cannot create pipe FDs"); > > - goto out; > > - } > > - > > - pid = fork(); > > - if (pid == 0) { > > - close(datafd[1]); > > - /* child */ > > - setsid(); > > - dup2(datafd[0], 0); > > - reopen_fd_to_null(1); > > - reopen_fd_to_null(2); > > - > > - if (crypted) { > > - execl(passwd_path, "chpasswd", "-e", NULL); > > - } else { > > - execl(passwd_path, "chpasswd", NULL); > > - } > > - _exit(EXIT_FAILURE); > > - } else if (pid < 0) { > > - error_setg_errno(errp, errno, "failed to create child > > process"); > > - goto out; > > - } > > - close(datafd[0]); > > - datafd[0] = -1; > > - > > - if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != > > chpasswdlen) { > > - error_setg_errno(errp, errno, "cannot write new account > > password"); > > - goto out; > > - } > > - close(datafd[1]); > > - datafd[1] = -1; > > - > > - ga_wait_child(pid, &status, &local_err); > > - if (local_err) { > > - error_propagate(errp, local_err); > > - goto out; > > - } > > - > > - if (!WIFEXITED(status)) { > > - error_setg(errp, "child process has terminated abnormally"); > > - goto out; > > - } > > - > > - if (WEXITSTATUS(status)) { > > - error_setg(errp, "child process has failed to set user > > password"); > > - goto out; > > - } > > - > > -out: > > - g_free(chpasswddata); > > - g_free(rawpasswddata); > > - g_free(passwd_path); > > - if (datafd[0] != -1) { > > - close(datafd[0]); > > - } > > - if (datafd[1] != -1) { > > - close(datafd[1]); > > - } > > -} > > - > > static void ga_read_sysfs_file(int dirfd, const char *pathname, > > char *buf, > > int size, Error **errp) > > { > > @@ -2793,6 +2688,124 @@ GuestMemoryBlockInfo > > *qmp_guest_get_memory_block_info(Error **errp) > > > > #endif > > > > +#if defined(__linux__) || defined(__FreeBSD__) > > +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; > > + > > + rawpasswddata = (char *)qbase64_decode(password, -1, > > &rawpasswdlen, errp); > > + if (!rawpasswddata) { > > + return; > > + } > > + rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1); > > + rawpasswddata[rawpasswdlen] = '\0'; > > + > > + if (strchr(rawpasswddata, '\n')) { > > + error_setg(errp, "forbidden characters in raw password"); > > + goto out; > > + } > > + > > + if (strchr(username, '\n') || > > + strchr(username, ':')) { > > + error_setg(errp, "forbidden characters in username"); > > + goto out; > > + } > > + > > +#ifdef __FreeBSD__ > > + chpasswddata = g_strdup(rawpasswddata); > > + passwd_path = g_find_program_in_path("pw"); > > +#else > > + chpasswddata = g_strdup_printf("%s:%s\n", username, > > rawpasswddata); > > + passwd_path = g_find_program_in_path("chpasswd"); > > +#endif > > + > > + chpasswdlen = strlen(chpasswddata); > > + > > + if (!passwd_path) { > > + error_setg(errp, "cannot find 'passwd' program in PATH"); > > + goto out; > > + } > > + > > + if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { > > + error_setg(errp, "cannot create pipe FDs"); > > + goto out; > > + } > > + > > + pid = fork(); > > + if (pid == 0) { > > + close(datafd[1]); > > + /* child */ > > + setsid(); > > + dup2(datafd[0], 0); > > + reopen_fd_to_null(1); > > + reopen_fd_to_null(2); > > + > > +#ifdef __FreeBSD__ > > + const char *h_arg; > > + h_arg = (crypted) ? "-H" : "-h"; > > + execl(passwd_path, "pw", "usermod", "-n", username, > > h_arg, "0", NULL); > > +#else > > + if (crypted) { > > + execl(passwd_path, "chpasswd", "-e", NULL); > > + } else { > > + execl(passwd_path, "chpasswd", NULL); > > + } > > +#endif > > + _exit(EXIT_FAILURE); > > + } else if (pid < 0) { > > + error_setg_errno(errp, errno, "failed to create child > > process"); > > + goto out; > > + } > > + close(datafd[0]); > > + datafd[0] = -1; > > + > > + if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != > > chpasswdlen) { > > + error_setg_errno(errp, errno, "cannot write new account > > password"); > > + goto out; > > + } > > + close(datafd[1]); > > + datafd[1] = -1; > > + > > + ga_wait_child(pid, &status, &local_err); > > + if (local_err) { > > + error_propagate(errp, local_err); > > + goto out; > > + } > > + > > + if (!WIFEXITED(status)) { > > + error_setg(errp, "child process has terminated abnormally"); > > + goto out; > > + } > > + > > + if (WEXITSTATUS(status)) { > > + error_setg(errp, "child process has failed to set user > > password"); > > + goto out; > > + } > > + > > +out: > > + g_free(chpasswddata); > > + g_free(rawpasswddata); > > + g_free(passwd_path); > > + if (datafd[0] != -1) { > > + close(datafd[0]); > > + } > > + if (datafd[1] != -1) { > > + close(datafd[1]); > > + } > > +} > > +#endif > > + > > #ifdef HAVE_GETIFADDRS > > static GuestNetworkInterface * > > guest_find_interface(GuestNetworkInterfaceList *head, > > -- > > 2.34.1 > > > > > > > > > > -- > > Marc-André Lureau >
Hi On 30.09.2022 10:19, Marc-André Lureau wrote: > > > Caution: This is an external email and has a suspicious subject or > content. Please take care when clicking links or opening attachments. > When in doubt, contact your IT Department > > > Hi > > On Thu, Sep 29, 2022 at 6:29 PM Alexander Ivanov > <alexander.ivanov@virtuozzo.com> wrote: > > > On 29.09.2022 13:22, Marc-André Lureau wrote: > > > > > > Caution: This is an external email and has a suspicious subject or > > content. Please take care when clicking links or opening > attachments. > > When in doubt, contact your IT Department > > > > > > Hi Alexander > > > > On Thu, Sep 29, 2022 at 12:52 PM Alexander Ivanov > > <alexander.ivanov@virtuozzo.com> wrote: > > > > Move qmp_guest_set_user_password() from __linux__ condition to > > (__linux__ || __FreeBSD__) condition. Add command and arguments > > for password setting in FreeBSD. > > > > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > > > > > > Could you explain why you need to move the code? > > > > You could instead have a top declaration? > Now qmp_guest_set_user_password()is under #ifdef __linux__. I want to > use this function for FreeBSD too, so I moved it under (__linux__ || > __FreeBSD__) condition. I could create another function for > FreeBSD but > > > Why not make the following change? I thought it is a bad idea to multiply pieces of code under the same arch conditions. OK, I will change the code in this way. Thank you. > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 88e0d0fe24..78a345c9f3 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -2122,7 +2122,9 @@ int64_t > qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) > > return processed; > } > +#endif /* linux */ > > +#if defined(__linux__) || defined(__FreeBSD__) > void qmp_guest_set_user_password(const char *username, > const char *password, > bool crypted, > @@ -2227,7 +2229,9 @@ out: > close(datafd[1]); > } > } > +#endif /* linux || freebsd */ > > +#if defined(__linux__) > static void ga_read_sysfs_file(int dirfd, const char *pathname, char > *buf, > int size, Error **errp) > { > > it would lead to code duplication. Unfortunately I don't see > another way > to do it except to add (__linux__ || __FreeBSD__) condition, cut the > function and paste in this condition. If you could suggest another > way I > would happy. > > If it's really required, please split the patch in move + additions. > Just moving the function without other changes leads to > unbuildable code > in FreeBSD. I thought it's worse than moving and changing at the same > time. I split the patch if you prefer. > > > > > --- > > qga/commands-posix.c | 223 > > +++++++++++++++++++++++-------------------- > > 1 file changed, 118 insertions(+), 105 deletions(-) > > > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > > index 88e0d0fe24..6ce894ca6e 100644 > > --- a/qga/commands-posix.c > > +++ b/qga/commands-posix.c > > @@ -2123,111 +2123,6 @@ int64_t > > qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error > **errp) > > return processed; > > } > > > > -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; > > - > > - rawpasswddata = (char *)qbase64_decode(password, -1, > > &rawpasswdlen, errp); > > - if (!rawpasswddata) { > > - return; > > - } > > - rawpasswddata = g_renew(char, rawpasswddata, > rawpasswdlen + 1); > > - rawpasswddata[rawpasswdlen] = '\0'; > > - > > - if (strchr(rawpasswddata, '\n')) { > > - error_setg(errp, "forbidden characters in raw > password"); > > - goto out; > > - } > > - > > - if (strchr(username, '\n') || > > - strchr(username, ':')) { > > - error_setg(errp, "forbidden characters in username"); > > - goto out; > > - } > > - > > - chpasswddata = g_strdup_printf("%s:%s\n", username, > > rawpasswddata); > > - chpasswdlen = strlen(chpasswddata); > > - > > - passwd_path = g_find_program_in_path("chpasswd"); > > - > > - if (!passwd_path) { > > - error_setg(errp, "cannot find 'passwd' program in > PATH"); > > - goto out; > > - } > > - > > - if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { > > - error_setg(errp, "cannot create pipe FDs"); > > - goto out; > > - } > > - > > - pid = fork(); > > - if (pid == 0) { > > - close(datafd[1]); > > - /* child */ > > - setsid(); > > - dup2(datafd[0], 0); > > - reopen_fd_to_null(1); > > - reopen_fd_to_null(2); > > - > > - if (crypted) { > > - execl(passwd_path, "chpasswd", "-e", NULL); > > - } else { > > - execl(passwd_path, "chpasswd", NULL); > > - } > > - _exit(EXIT_FAILURE); > > - } else if (pid < 0) { > > - error_setg_errno(errp, errno, "failed to create child > > process"); > > - goto out; > > - } > > - close(datafd[0]); > > - datafd[0] = -1; > > - > > - if (qemu_write_full(datafd[1], chpasswddata, > chpasswdlen) != > > chpasswdlen) { > > - error_setg_errno(errp, errno, "cannot write new account > > password"); > > - goto out; > > - } > > - close(datafd[1]); > > - datafd[1] = -1; > > - > > - ga_wait_child(pid, &status, &local_err); > > - if (local_err) { > > - error_propagate(errp, local_err); > > - goto out; > > - } > > - > > - if (!WIFEXITED(status)) { > > - error_setg(errp, "child process has terminated > abnormally"); > > - goto out; > > - } > > - > > - if (WEXITSTATUS(status)) { > > - error_setg(errp, "child process has failed to set user > > password"); > > - goto out; > > - } > > - > > -out: > > - g_free(chpasswddata); > > - g_free(rawpasswddata); > > - g_free(passwd_path); > > - if (datafd[0] != -1) { > > - close(datafd[0]); > > - } > > - if (datafd[1] != -1) { > > - close(datafd[1]); > > - } > > -} > > - > > static void ga_read_sysfs_file(int dirfd, const char *pathname, > > char *buf, > > int size, Error **errp) > > { > > @@ -2793,6 +2688,124 @@ GuestMemoryBlockInfo > > *qmp_guest_get_memory_block_info(Error **errp) > > > > #endif > > > > +#if defined(__linux__) || defined(__FreeBSD__) > > +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; > > + > > + rawpasswddata = (char *)qbase64_decode(password, -1, > > &rawpasswdlen, errp); > > + if (!rawpasswddata) { > > + return; > > + } > > + rawpasswddata = g_renew(char, rawpasswddata, > rawpasswdlen + 1); > > + rawpasswddata[rawpasswdlen] = '\0'; > > + > > + if (strchr(rawpasswddata, '\n')) { > > + error_setg(errp, "forbidden characters in raw > password"); > > + goto out; > > + } > > + > > + if (strchr(username, '\n') || > > + strchr(username, ':')) { > > + error_setg(errp, "forbidden characters in username"); > > + goto out; > > + } > > + > > +#ifdef __FreeBSD__ > > + chpasswddata = g_strdup(rawpasswddata); > > + passwd_path = g_find_program_in_path("pw"); > > +#else > > + chpasswddata = g_strdup_printf("%s:%s\n", username, > > rawpasswddata); > > + passwd_path = g_find_program_in_path("chpasswd"); > > +#endif > > + > > + chpasswdlen = strlen(chpasswddata); > > + > > + if (!passwd_path) { > > + error_setg(errp, "cannot find 'passwd' program in > PATH"); > > + goto out; > > + } > > + > > + if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { > > + error_setg(errp, "cannot create pipe FDs"); > > + goto out; > > + } > > + > > + pid = fork(); > > + if (pid == 0) { > > + close(datafd[1]); > > + /* child */ > > + setsid(); > > + dup2(datafd[0], 0); > > + reopen_fd_to_null(1); > > + reopen_fd_to_null(2); > > + > > +#ifdef __FreeBSD__ > > + const char *h_arg; > > + h_arg = (crypted) ? "-H" : "-h"; > > + execl(passwd_path, "pw", "usermod", "-n", username, > > h_arg, "0", NULL); > > +#else > > + if (crypted) { > > + execl(passwd_path, "chpasswd", "-e", NULL); > > + } else { > > + execl(passwd_path, "chpasswd", NULL); > > + } > > +#endif > > + _exit(EXIT_FAILURE); > > + } else if (pid < 0) { > > + error_setg_errno(errp, errno, "failed to create child > > process"); > > + goto out; > > + } > > + close(datafd[0]); > > + datafd[0] = -1; > > + > > + if (qemu_write_full(datafd[1], chpasswddata, > chpasswdlen) != > > chpasswdlen) { > > + error_setg_errno(errp, errno, "cannot write new account > > password"); > > + goto out; > > + } > > + close(datafd[1]); > > + datafd[1] = -1; > > + > > + ga_wait_child(pid, &status, &local_err); > > + if (local_err) { > > + error_propagate(errp, local_err); > > + goto out; > > + } > > + > > + if (!WIFEXITED(status)) { > > + error_setg(errp, "child process has terminated > abnormally"); > > + goto out; > > + } > > + > > + if (WEXITSTATUS(status)) { > > + error_setg(errp, "child process has failed to set user > > password"); > > + goto out; > > + } > > + > > +out: > > + g_free(chpasswddata); > > + g_free(rawpasswddata); > > + g_free(passwd_path); > > + if (datafd[0] != -1) { > > + close(datafd[0]); > > + } > > + if (datafd[1] != -1) { > > + close(datafd[1]); > > + } > > +} > > +#endif > > + > > #ifdef HAVE_GETIFADDRS > > static GuestNetworkInterface * > > guest_find_interface(GuestNetworkInterfaceList *head, > > -- > > 2.34.1 > > > > > > > > > > -- > > Marc-André Lureau > > > > -- > Marc-André Lureau
diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 88e0d0fe24..6ce894ca6e 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -2123,111 +2123,6 @@ int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) return processed; } -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; - - rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, errp); - if (!rawpasswddata) { - return; - } - rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1); - rawpasswddata[rawpasswdlen] = '\0'; - - if (strchr(rawpasswddata, '\n')) { - error_setg(errp, "forbidden characters in raw password"); - goto out; - } - - if (strchr(username, '\n') || - strchr(username, ':')) { - error_setg(errp, "forbidden characters in username"); - goto out; - } - - chpasswddata = g_strdup_printf("%s:%s\n", username, rawpasswddata); - chpasswdlen = strlen(chpasswddata); - - passwd_path = g_find_program_in_path("chpasswd"); - - if (!passwd_path) { - error_setg(errp, "cannot find 'passwd' program in PATH"); - goto out; - } - - if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { - error_setg(errp, "cannot create pipe FDs"); - goto out; - } - - pid = fork(); - if (pid == 0) { - close(datafd[1]); - /* child */ - setsid(); - dup2(datafd[0], 0); - reopen_fd_to_null(1); - reopen_fd_to_null(2); - - if (crypted) { - execl(passwd_path, "chpasswd", "-e", NULL); - } else { - execl(passwd_path, "chpasswd", NULL); - } - _exit(EXIT_FAILURE); - } else if (pid < 0) { - error_setg_errno(errp, errno, "failed to create child process"); - goto out; - } - close(datafd[0]); - datafd[0] = -1; - - if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != chpasswdlen) { - error_setg_errno(errp, errno, "cannot write new account password"); - goto out; - } - close(datafd[1]); - datafd[1] = -1; - - ga_wait_child(pid, &status, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto out; - } - - if (!WIFEXITED(status)) { - error_setg(errp, "child process has terminated abnormally"); - goto out; - } - - if (WEXITSTATUS(status)) { - error_setg(errp, "child process has failed to set user password"); - goto out; - } - -out: - g_free(chpasswddata); - g_free(rawpasswddata); - g_free(passwd_path); - if (datafd[0] != -1) { - close(datafd[0]); - } - if (datafd[1] != -1) { - close(datafd[1]); - } -} - static void ga_read_sysfs_file(int dirfd, const char *pathname, char *buf, int size, Error **errp) { @@ -2793,6 +2688,124 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp) #endif +#if defined(__linux__) || defined(__FreeBSD__) +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; + + rawpasswddata = (char *)qbase64_decode(password, -1, &rawpasswdlen, errp); + if (!rawpasswddata) { + return; + } + rawpasswddata = g_renew(char, rawpasswddata, rawpasswdlen + 1); + rawpasswddata[rawpasswdlen] = '\0'; + + if (strchr(rawpasswddata, '\n')) { + error_setg(errp, "forbidden characters in raw password"); + goto out; + } + + if (strchr(username, '\n') || + strchr(username, ':')) { + error_setg(errp, "forbidden characters in username"); + goto out; + } + +#ifdef __FreeBSD__ + chpasswddata = g_strdup(rawpasswddata); + passwd_path = g_find_program_in_path("pw"); +#else + chpasswddata = g_strdup_printf("%s:%s\n", username, rawpasswddata); + passwd_path = g_find_program_in_path("chpasswd"); +#endif + + chpasswdlen = strlen(chpasswddata); + + if (!passwd_path) { + error_setg(errp, "cannot find 'passwd' program in PATH"); + goto out; + } + + if (!g_unix_open_pipe(datafd, FD_CLOEXEC, NULL)) { + error_setg(errp, "cannot create pipe FDs"); + goto out; + } + + pid = fork(); + if (pid == 0) { + close(datafd[1]); + /* child */ + setsid(); + dup2(datafd[0], 0); + reopen_fd_to_null(1); + reopen_fd_to_null(2); + +#ifdef __FreeBSD__ + const char *h_arg; + h_arg = (crypted) ? "-H" : "-h"; + execl(passwd_path, "pw", "usermod", "-n", username, h_arg, "0", NULL); +#else + if (crypted) { + execl(passwd_path, "chpasswd", "-e", NULL); + } else { + execl(passwd_path, "chpasswd", NULL); + } +#endif + _exit(EXIT_FAILURE); + } else if (pid < 0) { + error_setg_errno(errp, errno, "failed to create child process"); + goto out; + } + close(datafd[0]); + datafd[0] = -1; + + if (qemu_write_full(datafd[1], chpasswddata, chpasswdlen) != chpasswdlen) { + error_setg_errno(errp, errno, "cannot write new account password"); + goto out; + } + close(datafd[1]); + datafd[1] = -1; + + ga_wait_child(pid, &status, &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto out; + } + + if (!WIFEXITED(status)) { + error_setg(errp, "child process has terminated abnormally"); + goto out; + } + + if (WEXITSTATUS(status)) { + error_setg(errp, "child process has failed to set user password"); + goto out; + } + +out: + g_free(chpasswddata); + g_free(rawpasswddata); + g_free(passwd_path); + if (datafd[0] != -1) { + close(datafd[0]); + } + if (datafd[1] != -1) { + close(datafd[1]); + } +} +#endif + #ifdef HAVE_GETIFADDRS static GuestNetworkInterface * guest_find_interface(GuestNetworkInterfaceList *head,
Move qmp_guest_set_user_password() from __linux__ condition to (__linux__ || __FreeBSD__) condition. Add command and arguments for password setting in FreeBSD. Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- qga/commands-posix.c | 223 +++++++++++++++++++++++-------------------- 1 file changed, 118 insertions(+), 105 deletions(-)