Hi
On Thu, Sep 29, 2022 at 6:29 PM Alexander Ivanov <
alexander.iva...@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.iva...@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.iva...@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
>
--
Marc-André Lureau
