Re: [PULL 09/12] qga: add ssh-{add,remove}-authorized-keys

[Michael Roth]

On Mon, Nov 02, 2020 at 04:49:27PM +0100, Markus Armbruster wrote:
> Michael Roth <michael.roth@amd.com> writes:
> 
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Add new commands to add and remove SSH public keys from
> > ~/.ssh/authorized_keys.
> >
> > I took a different approach for testing, including the unit tests right
> > with the code. I wanted to overwrite the function to get the user
> > details, I couldn't easily do that over QMP. Furthermore, I prefer
> > having unit tests very close to the code, and unit files that are domain
> > specific (commands-posix is too crowded already). FWIW, that
> > coding/testing style is Rust-style (where tests can or should even be
> > part of the documentation!).
> >
> > Fixes:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.redhat.com%2Fshow_bug.cgi%3Fid%3D1885332&amp;data=04%7C01%7Cmichael.roth%40amd.com%7C7302cfdd51b547a8c3de08d87f46e6cf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637399289788005891%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=IcYz5b1yBg3Q%2BPg82Z5VMdpf60vYHsLL518ENd0T1A4%3D&amp;reserved=0
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > *squashed in fix-ups for setting file ownership and use of QAPI
> >  conditionals for CONFIG_POSIX instead of stub definitions
> > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > ---
> >  qga/commands-posix-ssh.c | 407 +++++++++++++++++++++++++++++++++++++++
> >  qga/meson.build          |  20 +-
> >  qga/qapi-schema.json     |  35 ++++
> >  3 files changed, 461 insertions(+), 1 deletion(-)
> >  create mode 100644 qga/commands-posix-ssh.c
> >
> > diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
> > new file mode 100644
> > index 0000000000..a7bc9a1c24
> > --- /dev/null
> > +++ b/qga/commands-posix-ssh.c
> > @@ -0,0 +1,407 @@
> > + /*
> > +  * This work is licensed under the terms of the GNU GPL, version 2 or 
> > later.
> > +  * See the COPYING file in the top-level directory.
> > +  */
> > +#include "qemu/osdep.h"
> > +
> > +#include <glib-unix.h>
> > +#include <glib/gstdio.h>
> > +#include <locale.h>
> > +#include <pwd.h>
> > +
> > +#include "qapi/error.h"
> > +#include "qga-qapi-commands.h"
> > +
> > +#ifdef QGA_BUILD_UNIT_TEST
> > +static struct passwd *
> > +test_get_passwd_entry(const gchar *user_name, GError **error)
> > +{
> > +    struct passwd *p;
> > +    int ret;
> > +
> > +    if (!user_name || g_strcmp0(user_name, g_get_user_name())) {
> > +        g_set_error(error, G_UNIX_ERROR, 0, "Invalid user name");
> > +        return NULL;
> > +    }
> > +
> > +    p = g_new0(struct passwd, 1);
> > +    p->pw_dir = (char *)g_get_home_dir();
> > +    p->pw_uid = geteuid();
> > +    p->pw_gid = getegid();
> > +
> > +    ret = g_mkdir_with_parents(p->pw_dir, 0700);
> > +    g_assert_cmpint(ret, ==, 0);
> 
> checkpatch ERROR: Use g_assert or g_assert_not_reached
> 
> See commit 6e9389563e "checkpatch: Disallow glib asserts in main code"
> for rationale.

Thanks for the pointer, I couldn't figure out what the issue was and
assumed it was just noise. Wish I noticed this message before I sent out
v2... v3 incoming.

> 
> More below, and in PATCH 10+12.


> 
> [...]
>