From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55480) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8Yuz-0007Sq-11 for qemu-devel@nongnu.org; Thu, 02 Jun 2016 16:08:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8Yuu-0004J3-L4 for qemu-devel@nongnu.org; Thu, 02 Jun 2016 16:08:35 -0400 Received: from e38.co.us.ibm.com ([32.97.110.159]:41232) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8Yuu-0004IM-Bi for qemu-devel@nongnu.org; Thu, 02 Jun 2016 16:08:32 -0400 Received: from localhost by e38.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 2 Jun 2016 14:08:30 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1464097721-18101-1-git-send-email-marcandre.lureau@redhat.com> References: <1464097721-18101-1-git-send-email-marcandre.lureau@redhat.com> Message-ID: <20160602200814.21893.48923@loki> Date: Thu, 02 Jun 2016 15:08:14 -0500 Subject: Re: [Qemu-devel] [PATCH] tests: start a /qga/guest-exec test List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org Cc: yur@virtuozzo.com Quoting marcandre.lureau@redhat.com (2016-05-24 08:48:41) > From: Marc-Andr=C3=A9 Lureau > = > Test a few guest-exec guest agent commands, added in qemu 2.5. > = > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > tests/test-qga.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++= ++++++ > 1 file changed, 70 insertions(+) > = > diff --git a/tests/test-qga.c b/tests/test-qga.c > index 72a89de..42c9a89 100644 > --- a/tests/test-qga.c > +++ b/tests/test-qga.c > @@ -823,6 +823,75 @@ static void test_qga_fsfreeze_and_thaw(gconstpointer= fix) > QDECREF(ret); > } > = > +static void test_qga_guest_exec(gconstpointer fix) > +{ > + const TestFixture *fixture =3D fix; > + QDict *ret, *val, *error; > + const gchar *class, *desc, *out; > + gchar *decoded; > + int64_t pid, now, exitcode; > + gsize len; > + bool exited; > + > + /* exec 'echo foo bar' */ > + ret =3D qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': = {" > + " 'path': '/bin/echo', 'arg': [ 'foo', 'bar' ]," Maybe 'arg': [ '-n', '" test_str "' ]," to make the check below a little more robust as well, as exercising that arguments are being passed appropriately (since `echo "foo bar"` and `echo foo bar` are identical as far as the output. > + " 'capture-output': true } }"); > + g_assert_nonnull(ret); > + qmp_assert_no_error(ret); > + val =3D qdict_get_qdict(ret, "return"); > + pid =3D qdict_get_int(val, "pid"); > + g_assert_cmpint(pid, >, 0); > + QDECREF(ret); > + > + /* wait for completion */ > + now =3D g_get_monotonic_time(); > + do { > + ret =3D qmp_fd(fixture->fd, "{'execute': 'guest-exec-status'," > + " 'arguments': { 'pid': %" PRId64 " } }", pid); > + g_assert_nonnull(ret); > + val =3D qdict_get_qdict(ret, "return"); > + exited =3D qdict_get_bool(val, "exited"); > + if (!exited) { > + QDECREF(ret); > + } > + } while (!exited && > + g_get_monotonic_time() < now + 5 * G_TIME_SPAN_SECOND); > + g_assert(exited); > + > + /* check stdout */ > + exitcode =3D qdict_get_int(val, "exitcode"); > + g_assert_cmpint(exitcode, =3D=3D, 0); > + out =3D qdict_get_str(val, "out-data"); > + decoded =3D g_base64_decode(out, &len); > + g_assert_cmpint(len, =3D=3D, 8); > + g_assert_cmpint(strncmp(decoded, "foo bar\n", 8), =3D=3D, 0); > + g_free(decoded); > + QDECREF(ret); > + > + /* invalid command */ > + ret =3D qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': = {" > + " 'path': '/bin/invalid-cmd42' } }"); Since there's no shared state between this and the above command I think I'd rather this be in a separate test function. I know we have somewhat of a precedence for this with test_qga_blacklist, but there's probably a number of other checks worth adding that would eventually make this test function a bit unwieldly. > + g_assert_nonnull(ret); > + error =3D qdict_get_qdict(ret, "error"); > + class =3D qdict_get_try_str(error, "class"); > + desc =3D qdict_get_try_str(error, "desc"); > + g_assert_cmpstr(class, =3D=3D, "GenericError"); > + g_assert_cmpint(strlen(desc), >, 0); > + QDECREF(ret); > + > + /* invalid pid */ > + ret =3D qmp_fd(fixture->fd, "{'execute': 'guest-exec-status'," > + " 'arguments': { 'pid': 0 } }"); > + g_assert_nonnull(ret); > + error =3D qdict_get_qdict(ret, "error"); g_assert_nonnull(error) (qdict_get_try_str doesn't seem to check for null qdict) > + class =3D qdict_get_try_str(error, "class"); > + desc =3D qdict_get_try_str(error, "desc"); > + g_assert_cmpstr(class, =3D=3D, "GenericError"); > + g_assert_cmpint(strlen(desc), >, 0); I might be missing something in the error_setg path, but it doesn't seem like we can rely on desc not being an empty string. guest-exec-status doesn't use it in that fashion, but that's an internal detail rather than something we can reliably test for. Looks good otherwise. > + QDECREF(ret); > +} > + > int main(int argc, char **argv) > { > TestFixture fix; > @@ -853,6 +922,7 @@ int main(int argc, char **argv) > = > g_test_add_data_func("/qga/blacklist", NULL, test_qga_blacklist); > g_test_add_data_func("/qga/config", NULL, test_qga_config); > + g_test_add_data_func("/qga/guest-exec", &fix, test_qga_guest_exec); > = > if (g_getenv("QGA_TEST_SIDE_EFFECTING")) { > g_test_add_data_func("/qga/fsfreeze-and-thaw", &fix, > -- = > 2.7.4 >=20