mbox

[PULL,v2,00/12] qemu-ga patch queue for soft-freeze

Message ID 20201103011134.887744-1-michael.roth@amd.com
State New
Headers show

Pull-request

git://github.com/mdroth/qemu.git tags/qga-pull-2020-10-27-v2-tag

Message

Michael Roth Nov. 3, 2020, 1:11 a.m. UTC 
Hi Peter,

Sorry to get these out so late, for some inexplicable reason my email
client decided to flag all responses v1 as spam and I didn't notice
until I double-checked the archives this morning.

I've fixed the gcc-on-BSD and clang-on-linux issues you pointed out 
(PATCH 6) and added proper test coverage for both.

Also, the qga-ssh-test unit test introduced in this series triggers a
failure in Gitlab CI build-oss-fuzz test. This seems to be due to a
memory leak in GLib itself when G_TEST_OPTION_ISOLATE_DIRS option is
passed to g_test_init(). I verified the unit test doesn't introduce any
leaks when run without g_test* harness. Since G_TEST_OPTION_ISOLATE_DIRS
is currently needed to safely run the qga-ssh-test, I've disabled it for
now (PATCH 9 and 12) until we figure out solution.

Thanks,

Mike

The following changes since commit 2c6605389c1f76973d92b69b85d40d94b8f1092c:

  Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20201101.0' into staging (2020-11-02 09:54:00 +0000)

are available in the Git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2020-10-27-v2-tag

for you to fetch changes up to b457991dfb801bf9227e8823534de5dbb3c276c1:

  qga: add ssh-get-authorized-keys (2020-11-02 18:30:42 -0600)

----------------------------------------------------------------
qemu-ga patch queue for soft-freeze

* add guest-get-disks for w32/linux
* add guest-{add,remove,get}-authorized-keys
* fix API violations and schema documentation inconsistencies with
  recently-added guest-get-devices

v2:
- fix BSD build error due to missing stub for guest_get_disks
- fix clang build error on linux due to unused variable
- disable qga-ssh-test for now due to a memory leak within GLib when
  G_TEST_OPTION_ISOLATE_DIRS is passed to g_test_init() since it
  break Gitlab CI build-oss-fuzz test
- rebased and re-tested on master

----------------------------------------------------------------
Marc-André Lureau (5):
      glib-compat: add g_unix_get_passwd_entry_qemu()
      qga: add ssh-{add,remove}-authorized-keys
      qga: add *reset argument to ssh-add-authorized-keys
      meson: minor simplification
      qga: add ssh-get-authorized-keys

Markus Armbruster (4):
      qga: Rename guest-get-devices return member 'address' to 'id'
      qga: Use common time encoding for guest-get-devices 'driver-date'
      qga-win: Fix guest-get-devices error API violations
      qga: Flatten simple union GuestDeviceId

Tomáš Golembiovský (3):
      qga: add command guest-get-disks
      qga: add implementation of guest-get-disks for Linux
      qga: add implementation of guest-get-disks for Windows

 include/glib-compat.h    |  26 +++
 qga/commands-posix-ssh.c | 516 +++++++++++++++++++++++++++++++++++++++++++++++
 qga/commands-posix.c     | 297 ++++++++++++++++++++++++++-
 qga/commands-win32.c     | 140 +++++++++++--
 qga/meson.build          |  39 +++-
 qga/qapi-schema.json     | 127 +++++++++++-
 6 files changed, 1104 insertions(+), 41 deletions(-)
 create mode 100644 qga/commands-posix-ssh.c

Comments

no-reply@patchew.org Nov. 3, 2020, 1:39 a.m. UTC | #1 
Patchew URL: https://patchew.org/QEMU/20201103011134.887744-1-michael.roth@amd.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201103011134.887744-1-michael.roth@amd.com
Subject: [PULL v2 00/12] qemu-ga patch queue for soft-freeze

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20201102202710.2224691-1-f4bug@amsat.org -> patchew/20201102202710.2224691-1-f4bug@amsat.org
 - [tag update]      patchew/20201102203609.2228309-1-philmd@redhat.com -> patchew/20201102203609.2228309-1-philmd@redhat.com
 * [new tag]         patchew/20201103011134.887744-1-michael.roth@amd.com -> patchew/20201103011134.887744-1-michael.roth@amd.com
Switched to a new branch 'test'
134f664 qga: add ssh-get-authorized-keys
9f5e642 meson: minor simplification
445e4c4 qga: add *reset argument to ssh-add-authorized-keys
7ab2def qga: add ssh-{add,remove}-authorized-keys
b137836 glib-compat: add g_unix_get_passwd_entry_qemu()
bf1041a qga: add implementation of guest-get-disks for Windows
78bc690 qga: add implementation of guest-get-disks for Linux
de60426 qga: add command guest-get-disks
e8dc031 qga: Flatten simple union GuestDeviceId
a78863f qga-win: Fix guest-get-devices error API violations
3d3253f qga: Use common time encoding for guest-get-devices 'driver-date'
1965fac qga: Rename guest-get-devices return member 'address' to 'id'

=== OUTPUT BEGIN ===
1/12 Checking commit 1965fac569d1 (qga: Rename guest-get-devices return member 'address' to 'id')
2/12 Checking commit 3d3253f3695c (qga: Use common time encoding for guest-get-devices 'driver-date')
3/12 Checking commit a78863f8a833 (qga-win: Fix guest-get-devices error API violations)
4/12 Checking commit e8dc03167d53 (qga: Flatten simple union GuestDeviceId)
5/12 Checking commit de60426a744f (qga: add command guest-get-disks)
6/12 Checking commit 78bc69001515 (qga: add implementation of guest-get-disks for Linux)
7/12 Checking commit bf1041a868af (qga: add implementation of guest-get-disks for Windows)
8/12 Checking commit b1378362971f (glib-compat: add g_unix_get_passwd_entry_qemu())
WARNING: Block comments use a leading /* on a separate line
#42: FILE: include/glib-compat.h:81:
+/* Note: The fallback implementation is not MT-safe, and it returns a copy of

WARNING: Block comments use a trailing */ on a separate line
#45: FILE: include/glib-compat.h:84:
+ * GLib API substitution. */

total: 0 errors, 2 warnings, 38 lines checked

Patch 8/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/12 Checking commit 7ab2def768e8 (qga: add ssh-{add,remove}-authorized-keys)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

ERROR: Use g_assert or g_assert_not_reached
#69: FILE: qga/commands-posix-ssh.c:33:
+    g_assert_cmpint(ret, ==, 0);

ERROR: Use g_assert or g_assert_not_reached
#330: FILE: qga/commands-posix-ssh.c:294:
+    g_assert_cmpint(ret, ==, 0);

ERROR: Use g_assert or g_assert_not_reached
#335: FILE: qga/commands-posix-ssh.c:299:
+    g_assert_no_error(err);

ERROR: Use g_assert or g_assert_not_reached
#347: FILE: qga/commands-posix-ssh.c:311:
+    g_assert_no_error(err);

ERROR: Use g_assert or g_assert_not_reached
#349: FILE: qga/commands-posix-ssh.c:313:
+    g_assert_cmpstr(contents, ==, expected);

ERROR: Use g_assert or g_assert_not_reached
#386: FILE: qga/commands-posix-ssh.c:350:
+    g_assert_null(err);

ERROR: Use g_assert or g_assert_not_reached
#392: FILE: qga/commands-posix-ssh.c:356:
+    g_assert_null(err);

ERROR: Use g_assert or g_assert_not_reached
#413: FILE: qga/commands-posix-ssh.c:377:
+    g_assert_null(err);

ERROR: Use g_assert or g_assert_not_reached
#418: FILE: qga/commands-posix-ssh.c:382:
+    g_assert_null(err);

total: 9 errors, 1 warnings, 479 lines checked

Patch 9/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

10/12 Checking commit 445e4c4ff87d (qga: add *reset argument to ssh-add-authorized-keys)
ERROR: Use g_assert or g_assert_not_reached
#96: FILE: qga/commands-posix-ssh.c:381:
+    g_assert_null(err);

ERROR: Use g_assert or g_assert_not_reached
#106: FILE: qga/commands-posix-ssh.c:391:
+    g_assert_null(err);

ERROR: Use g_assert or g_assert_not_reached
#115: FILE: qga/commands-posix-ssh.c:400:
+    g_assert_null(err);

total: 3 errors, 0 warnings, 121 lines checked

Patch 10/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

11/12 Checking commit 9f5e6424dba4 (meson: minor simplification)
12/12 Checking commit 134f66443ae0 (qga: add ssh-get-authorized-keys)
ERROR: space prohibited between function name and open parenthesis '('
#60: FILE: qga/commands-posix-ssh.c:309:
+    return g_steal_pointer (&ret);

ERROR: Use g_assert or g_assert_not_reached
#84: FILE: qga/commands-posix-ssh.c:484:
+    g_assert_null(err);

ERROR: Use g_assert or g_assert_not_reached
#91: FILE: qga/commands-posix-ssh.c:491:
+    g_assert_cmpint(len, ==, 2);

total: 3 errors, 0 warnings, 138 lines checked

Patch 12/12 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201103011134.887744-1-michael.roth@amd.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Michael Roth Nov. 3, 2020, 2:37 a.m. UTC | #2 
On Mon, Nov 02, 2020 at 07:11:22PM -0600, Michael Roth wrote:
> Hi Peter,
> 
> Sorry to get these out so late, for some inexplicable reason my email
> client decided to flag all responses v1 as spam and I didn't notice
> until I double-checked the archives this morning.
> 
> I've fixed the gcc-on-BSD and clang-on-linux issues you pointed out 
> (PATCH 6) and added proper test coverage for both.
> 
> Also, the qga-ssh-test unit test introduced in this series triggers a
> failure in Gitlab CI build-oss-fuzz test. This seems to be due to a
> memory leak in GLib itself when G_TEST_OPTION_ISOLATE_DIRS option is
> passed to g_test_init(). I verified the unit test doesn't introduce any
> leaks when run without g_test* harness. Since G_TEST_OPTION_ISOLATE_DIRS
> is currently needed to safely run the qga-ssh-test, I've disabled it for
> now (PATCH 9 and 12) until we figure out solution.
> 
> Thanks,
> 
> Mike

...And I just noticed Markus email noting that checkpatch complaints about
g_assert_* aren't just noise. Re-spinning a v3 to address.

> 
> The following changes since commit 2c6605389c1f76973d92b69b85d40d94b8f1092c:
> 
>   Merge remote-tracking branch 'remotes/awilliam/tags/vfio-update-20201101.0' into staging (2020-11-02 09:54:00 +0000)
> 
> are available in the Git repository at:
> 
>   git://github.com/mdroth/qemu.git tags/qga-pull-2020-10-27-v2-tag
> 
> for you to fetch changes up to b457991dfb801bf9227e8823534de5dbb3c276c1:
> 
>   qga: add ssh-get-authorized-keys (2020-11-02 18:30:42 -0600)
> 
> ----------------------------------------------------------------
> qemu-ga patch queue for soft-freeze
> 
> * add guest-get-disks for w32/linux
> * add guest-{add,remove,get}-authorized-keys
> * fix API violations and schema documentation inconsistencies with
>   recently-added guest-get-devices
> 
> v2:
> - fix BSD build error due to missing stub for guest_get_disks
> - fix clang build error on linux due to unused variable
> - disable qga-ssh-test for now due to a memory leak within GLib when
>   G_TEST_OPTION_ISOLATE_DIRS is passed to g_test_init() since it
>   break Gitlab CI build-oss-fuzz test
> - rebased and re-tested on master
> 
> ----------------------------------------------------------------
> Marc-André Lureau (5):
>       glib-compat: add g_unix_get_passwd_entry_qemu()
>       qga: add ssh-{add,remove}-authorized-keys
>       qga: add *reset argument to ssh-add-authorized-keys
>       meson: minor simplification
>       qga: add ssh-get-authorized-keys
> 
> Markus Armbruster (4):
>       qga: Rename guest-get-devices return member 'address' to 'id'
>       qga: Use common time encoding for guest-get-devices 'driver-date'
>       qga-win: Fix guest-get-devices error API violations
>       qga: Flatten simple union GuestDeviceId
> 
> Tomáš Golembiovský (3):
>       qga: add command guest-get-disks
>       qga: add implementation of guest-get-disks for Linux
>       qga: add implementation of guest-get-disks for Windows
> 
>  include/glib-compat.h    |  26 +++
>  qga/commands-posix-ssh.c | 516 +++++++++++++++++++++++++++++++++++++++++++++++
>  qga/commands-posix.c     | 297 ++++++++++++++++++++++++++-
>  qga/commands-win32.c     | 140 +++++++++++--
>  qga/meson.build          |  39 +++-
>  qga/qapi-schema.json     | 127 +++++++++++-
>  6 files changed, 1104 insertions(+), 41 deletions(-)
>  create mode 100644 qga/commands-posix-ssh.c
> 
>