[PATCH v2 0/7] vhost: replace master/slave with more accurate wording

Paolo Bonzini posted 7 patches 3 years, 1 month ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210311103250.532191-1-pbonzini@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
docs/interop/vhost-user-gpu.rst           |  10 +-
docs/interop/vhost-user.rst               | 532 ++++++++++++----------
hw/virtio/vhost-user.c                    |  96 ++--
subprojects/libvhost-user/libvhost-user.c |  66 +--
subprojects/libvhost-user/libvhost-user.h |  45 +-
5 files changed, 395 insertions(+), 354 deletions(-)
[PATCH v2 0/7] vhost: replace master/slave with more accurate wording
Posted by Paolo Bonzini 3 years, 1 month ago
Compared to v1, which only affected the prose of the documentation, I
am also changing the message names in the docs and code now.

Patch 2 was also adjusted according to Stefan's documentation, and
"frontend/backend" (with no hyphen) is used consistently.

Paolo

Supersedes: <20210226143413.188046-1-pbonzini@redhat.com>

Paolo Bonzini (7):
  docs: vhost-user: clean up request/reply description
  docs: vhost-user: rewrite section on ring state machine
  docs: vhost-user: replace master/slave with front-end/back-end
  docs: vhost-user: rename message names from "SLAVE" to "BACKEND"
  vhost-user: rename message names from "SLAVE" to "BACKEND"
  libvhost-user: rename message names from "SLAVE" to "BACKEND"
  libvhost-user: rename struct VuDev fields from "slave" to "backend"

 docs/interop/vhost-user-gpu.rst           |  10 +-
 docs/interop/vhost-user.rst               | 532 ++++++++++++----------
 hw/virtio/vhost-user.c                    |  96 ++--
 subprojects/libvhost-user/libvhost-user.c |  66 +--
 subprojects/libvhost-user/libvhost-user.h |  45 +-
 5 files changed, 395 insertions(+), 354 deletions(-)

-- 
2.26.2


Re: [PATCH v2 0/7] vhost: replace master/slave with more accurate wording
Posted by no-reply@patchew.org 3 years, 1 month ago
Patchew URL: https://patchew.org/QEMU/20210311103250.532191-1-pbonzini@redhat.com/



Hi,

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

Type: series
Message-id: 20210311103250.532191-1-pbonzini@redhat.com
Subject: [PATCH v2 0/7] vhost: replace master/slave with more accurate wording

=== 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
 * [new tag]         patchew/20210311103250.532191-1-pbonzini@redhat.com -> patchew/20210311103250.532191-1-pbonzini@redhat.com
Switched to a new branch 'test'
743512a libvhost-user: rename struct VuDev fields from "slave" to "backend"
d41a226 libvhost-user: rename message names from "SLAVE" to "BACKEND"
50533a6 vhost-user: rename message names from "SLAVE" to "BACKEND"
091c293 docs: vhost-user: rename message names from "SLAVE" to "BACKEND"
522e044 docs: vhost-user: replace master/slave with frontend/backend
a31c678 docs: vhost-user: rewrite section on ring state machine
865d2fb docs: vhost-user: clean up request/reply description

=== OUTPUT BEGIN ===
1/7 Checking commit 865d2fb5c75f (docs: vhost-user: clean up request/reply description)
2/7 Checking commit a31c678027a2 (docs: vhost-user: rewrite section on ring state machine)
3/7 Checking commit 522e044c3977 (docs: vhost-user: replace master/slave with frontend/backend)
4/7 Checking commit 091c293d3260 (docs: vhost-user: rename message names from "SLAVE" to "BACKEND")
5/7 Checking commit 50533a60dcce (vhost-user: rename message names from "SLAVE" to "BACKEND")
WARNING: line over 80 characters
#96: FILE: hw/virtio/vhost-user.c:1336:
+                                                         VhostUserVringArea *area,

ERROR: space prohibited before that ':' (ctx:WxE)
#154: FILE: hw/virtio/vhost-user.c:1466:
+    case VHOST_USER_BACKEND_CONFIG_CHANGE_MSG :
                                               ^

total: 1 errors, 1 warnings, 253 lines checked

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

6/7 Checking commit d41a22625345 (libvhost-user: rename message names from "SLAVE" to "BACKEND")
WARNING: line over 80 characters
#168: FILE: subprojects/libvhost-user/libvhost-user.h:134:
+    VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG = VHOST_USER_BACKEND_VRING_HOST_NOTIFIER_MSG,

WARNING: line over 80 characters
#179: FILE: subprojects/libvhost-user/libvhost-user.h:313:
+     * size. The backend could get it from queue size field of VhostUserInflight. */

WARNING: Block comments use a trailing */ on a separate line
#179: FILE: subprojects/libvhost-user/libvhost-user.h:313:
+     * size. The backend could get it from queue size field of VhostUserInflight. */

total: 0 errors, 3 warnings, 145 lines checked

Patch 6/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/7 Checking commit 743512a20c83 (libvhost-user: rename struct VuDev fields from "slave" to "backend")
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210311103250.532191-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH v2 0/7] vhost: replace master/slave with more accurate wording
Posted by Stefano Garzarella 3 years, 1 month ago
On Thu, Mar 11, 2021 at 05:32:43AM -0500, Paolo Bonzini wrote:
>Compared to v1, which only affected the prose of the documentation, I
>am also changing the message names in the docs and code now.
>
>Patch 2 was also adjusted according to Stefan's documentation, and
>"frontend/backend" (with no hyphen) is used consistently.

A few "Back-end[s]" words (hyphenated, maybe those with a capital B) 
remained in the patch 3.
If you don't have to respin the series, I can send a followup patch if 
you want.

Anyway the series LGTM:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


Thanks,
Stefano


Re: [PATCH v2 0/7] vhost: replace master/slave with more accurate wording
Posted by Marc-André Lureau 3 years, 1 month ago
Hi

On Thu, Mar 11, 2021 at 2:33 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> Compared to v1, which only affected the prose of the documentation, I
> am also changing the message names in the docs and code now.
>
> Patch 2 was also adjusted according to Stefan's documentation, and
> "frontend/backend" (with no hyphen) is used consistently.
>
> Paolo
>
>
What about renaming  VhostUserSlaveRequest too ?
(subprojects/libvhost-user/libvhost-user.h & hw/virtio/vhost-user.c)

Supersedes: <20210226143413.188046-1-pbonzini@redhat.com>
>
> Paolo Bonzini (7):
>   docs: vhost-user: clean up request/reply description
>   docs: vhost-user: rewrite section on ring state machine
>   docs: vhost-user: replace master/slave with front-end/back-end
>   docs: vhost-user: rename message names from "SLAVE" to "BACKEND"
>   vhost-user: rename message names from "SLAVE" to "BACKEND"
>   libvhost-user: rename message names from "SLAVE" to "BACKEND"
>   libvhost-user: rename struct VuDev fields from "slave" to "backend"
>
>  docs/interop/vhost-user-gpu.rst           |  10 +-
>  docs/interop/vhost-user.rst               | 532 ++++++++++++----------
>  hw/virtio/vhost-user.c                    |  96 ++--
>  subprojects/libvhost-user/libvhost-user.c |  66 +--
>  subprojects/libvhost-user/libvhost-user.h |  45 +-
>  5 files changed, 395 insertions(+), 354 deletions(-)
>
> --
> 2.26.2
>
>
Re: [PATCH v2 0/7] vhost: replace master/slave with more accurate wording
Posted by Paolo Bonzini 3 years, 1 month ago
On 11/03/21 11:51, Marc-André Lureau wrote:
> 
> 
> What about renaming  VhostUserSlaveRequest too ?
> (subprojects/libvhost-user/libvhost-user.h & hw/virtio/vhost-user.c)
> 

I missed it in vhost-user.c (I did rename the values but not the enum), 
while in libvhost-user.h I'm keeping it for backwards compatibility as I 
don't want to break the API.

Paolo


Re: [PATCH v2 0/7] vhost: replace master/slave with more accurate wording
Posted by Marc-André Lureau 3 years, 1 month ago
Hi

On Thu, Mar 11, 2021 at 3:08 PM Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 11/03/21 11:51, Marc-André Lureau wrote:
> >
> >
> > What about renaming  VhostUserSlaveRequest too ?
> > (subprojects/libvhost-user/libvhost-user.h & hw/virtio/vhost-user.c)
> >
>
> I missed it in vhost-user.c (I did rename the values but not the enum),
> while in libvhost-user.h I'm keeping it for backwards compatibility as I
> don't want to break the API.
>
>
Keeping API stability is great and all, but libvhost-user never received an
official release, and isn't installed either.  We should be free to break
it atm imho.
Re: [PATCH v2 0/7] vhost: replace master/slave with more accurate wording
Posted by Paolo Bonzini 3 years, 1 month ago
On 11/03/21 12:18, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Mar 11, 2021 at 3:08 PM Paolo Bonzini <pbonzini@redhat.com 
> <mailto:pbonzini@redhat.com>> wrote:
> 
>     On 11/03/21 11:51, Marc-André Lureau wrote:
>      >
>      >
>      > What about renaming  VhostUserSlaveRequest too ?
>      > (subprojects/libvhost-user/libvhost-user.h & hw/virtio/vhost-user.c)
>      >
> 
>     I missed it in vhost-user.c (I did rename the values but not the enum),
>     while in libvhost-user.h I'm keeping it for backwards compatibility
>     as I
>     don't want to break the API.
> 
> 
> Keeping API stability is great and all, but libvhost-user never received 
> an official release, and isn't installed either.  We should be free to 
> break it atm imho.

That's true, but I'd rather follow (at least informally) the usual 
policy around deprecation.  There's no need to rush things.

Paolo