[Qemu-devel] [PATCH 0/5] Block patches

[Qemu-devel] [PATCH 0/5] Block patches

[Stefan Hajnoczi]

The following changes since commit fb68096da3d35e64c88cd610c1fa42766c58e92a:

  Revert "tests: use memfd in vhost-user-test" (2018-02-13 09:51:52 +0000)

are available in the Git repository at:

  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 64b01feca991e5b19a5d750ef77cdca92b68bdbb:

  misc: fix spelling (2018-02-13 15:38:17 +0000)

----------------------------------------------------------------

----------------------------------------------------------------

Fam Zheng (2):
  Add a git-publish configuration file
  README: Document 'git-publish' workflow

Marc-André Lureau (1):
  misc: fix spelling

Stefan Hajnoczi (1):
  vl: pause vcpus before stopping iothreads

Wolfgang Bumiller (1):
  ratelimit: don't align wait time with slices

 include/qemu/ratelimit.h   | 11 ++++-----
 util/qemu-coroutine-lock.c |  2 +-
 vl.c                       | 12 ++++++++--
 .gitpublish                | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 README                     | 31 ++++++++++++++++++++++++-
 5 files changed, 104 insertions(+), 10 deletions(-)
 create mode 100644 .gitpublish

-- 
2.14.3

[Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

git-publish [1] is a convenient tool to send patches and has been
popular among QEMU developers.  Recently it has been made available in
Fedora official repo thanks to Stefan's work.

One nice feature of the tool is a per-project configuration with
profiles, especially in which the cccmd option is a handy method to
create the Cc list.

[1]: https://github.com/stefanha/git-publish

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20180205054725.25634-2-famz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 .gitpublish | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 .gitpublish

diff --git a/.gitpublish b/.gitpublish
new file mode 100644
index 0000000000..ed48f6e52c
--- /dev/null
+++ b/.gitpublish
@@ -0,0 +1,58 @@
+#
+# Common git-publish profiles that can be used to send patches to QEMU upstream.
+#
+# See https://github.com/stefanha/git-publish for more information
+#
+[gitpublishprofile "default"]
+base = master
+prefix = PATCH
+to = qemu-devel@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
+
+[gitpublishprofile "rfc"]
+base = master
+prefix = RFC PATCH
+to = qemu-devel@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
+
+[gitpublishprofile "stable"]
+base = master
+prefix = PATCH
+to = qemu-devel@nongnu.org
+cc = qemu-stable@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
+
+[gitpublishprofile "trivial"]
+base = master
+prefix = PATCH
+to = qemu-devel@nongnu.org
+cc = qemu-trivial@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
+
+[gitpublishprofile "block"]
+base = master
+prefix = PATCH
+to = qemu-devel@nongnu.org
+cc = qemu-block@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
+
+[gitpublishprofile "arm"]
+base = master
+prefix = PATCH
+to = qemu-devel@nongnu.org
+cc = qemu-arm@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
+
+[gitpublishprofile "s390"]
+base = master
+prefix = PATCH
+to = qemu-devel@nongnu.org
+cc = qemu-s390@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
+
+[gitpublishprofile "ppc"]
+base = master
+prefix = PATCH
+to = qemu-devel@nongnu.org
+cc = qemu-ppc@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
-- 
2.14.3

[Qemu-devel] [PATCH 2/5] README: Document 'git-publish' workflow

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

[Fixed s/git-profile/git-publish/ typo and s/for once/the first time/.
--Stefan]

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20180205054725.25634-3-famz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 README | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/README b/README
index 2c8e1c8cc4..627633fe1d 100644
--- a/README
+++ b/README
@@ -56,12 +56,41 @@ The QEMU source code is maintained under the GIT version control system.
 
    git clone git://git.qemu.org/qemu.git
 
-When submitting patches, the preferred approach is to use 'git
+When submitting patches, one common approach is to use 'git
 format-patch' and/or 'git send-email' to format & send the mail to the
 qemu-devel@nongnu.org mailing list. All patches submitted must contain
 a 'Signed-off-by' line from the author. Patches should follow the
 guidelines set out in the HACKING and CODING_STYLE files.
 
+A 'git-publish' utility was created to make above process less
+cumbersome, and is highly recommended for making regular contributions,
+or even just for sending consecutive patch series revisions. It also
+requires a working 'git send-email' setup, and by default doesn't
+automate everything, so you may want to go through the above steps
+manually the first time.
+
+For installation instructions, please go to
+
+  https://github.com/stefanha/git-publish
+
+The workflow with 'git-publish' is:
+
+  $ git checkout master -b my-feature
+  $ # work on new commits, add your 'Signed-off-by' lines to each
+  $ git publish
+
+Your patch series will be sent and tagged as my-feature-v1 if you need to refer
+back to it in the future.
+
+Sending v2:
+
+  $ git checkout my-feature # same topic branch
+  $ # making changes to the commits (using 'git rebase', for example)
+  $ git publish
+
+Your patch series will be sent with 'v2' tag in the subject and the git tip
+will be tagged as my-feature-v2.
+
 Additional information on submitting patches can be found online via
 the QEMU website
 
-- 
2.14.3

[Qemu-devel] [PATCH 3/5] vl: pause vcpus before stopping iothreads

[Stefan Hajnoczi]

Commit dce8921b2baaf95974af8176406881872067adfa ("iothread: Stop threads
before main() quits") introduced iothread_stop_all() to avoid the
following virtio-scsi assertion failure:

  assert(blk_get_aio_context(d->conf.blk) == s->ctx);

Back then the assertion failed because when bdrv_close_all() made
d->conf.blk NULL, blk_get_aio_context() returned the global AioContext
instead of s->ctx.

The same assertion can still fail today when vcpus submit new I/O
requests after iothread_stop_all() has moved the BDS to the global
AioContext.

This patch hardens the iothread_stop_all() approach by pausing vcpus
before calling iothread_stop_all().

Note that the assertion failure is a race condition.  It is not possible
to reproduce it reliably.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20180201110708.8080-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 vl.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index 21878496ec..7a5554bc41 100644
--- a/vl.c
+++ b/vl.c
@@ -4767,10 +4767,18 @@ int main(int argc, char **argv, char **envp)
 
     main_loop();
     replay_disable_events();
+
+    /* The ordering of the following is delicate.  Stop vcpus to prevent new
+     * I/O requests being queued by the guest.  Then stop IOThreads (this
+     * includes a drain operation and completes all request processing).  At
+     * this point emulated devices are still associated with their IOThreads
+     * (if any) but no longer have any work to do.  Only then can we close
+     * block devices safely because we know there is no more I/O coming.
+     */
+    pause_all_vcpus();
     iothread_stop_all();
-
-    pause_all_vcpus();
     bdrv_close_all();
+
     res_free();
 
     /* vhost-user must be cleaned up before chardevs.  */
-- 
2.14.3

[Qemu-devel] [PATCH 4/5] ratelimit: don't align wait time with slices

[Stefan Hajnoczi]

From: Wolfgang Bumiller <w.bumiller@proxmox.com>

It is possible for rate limited writes to keep overshooting a slice's
quota by a tiny amount causing the slice-aligned waiting period to
effectively halve the rate.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-id: 20180207071758.6818-1-w.bumiller@proxmox.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/qemu/ratelimit.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h
index 8dece483f5..1b38291823 100644
--- a/include/qemu/ratelimit.h
+++ b/include/qemu/ratelimit.h
@@ -36,7 +36,7 @@ typedef struct {
 static inline int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
 {
     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
-    uint64_t delay_slices;
+    double delay_slices;
 
     assert(limit->slice_quota && limit->slice_ns);
 
@@ -55,12 +55,11 @@ static inline int64_t ratelimit_calculate_delay(RateLimit *limit, uint64_t n)
         return 0;
     }
 
-    /* Quota exceeded. Calculate the next time slice we may start
-     * sending data again. */
-    delay_slices = (limit->dispatched + limit->slice_quota - 1) /
-        limit->slice_quota;
+    /* Quota exceeded. Wait based on the excess amount and then start a new
+     * slice. */
+    delay_slices = (double)limit->dispatched / limit->slice_quota;
     limit->slice_end_time = limit->slice_start_time +
-        delay_slices * limit->slice_ns;
+        (uint64_t)(delay_slices * limit->slice_ns);
     return limit->slice_end_time - now;
 }
 
-- 
2.14.3

[Qemu-devel] [PATCH 5/5] misc: fix spelling

[Stefan Hajnoczi]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

s/pupulate/populate

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20180208162447.10851-1-marcandre.lureau@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/qemu-coroutine-lock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index 78fb79acf8..5a80c10690 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -89,7 +89,7 @@ void qemu_co_queue_run_restart(Coroutine *co)
      * invalid memory.  Therefore, use a temporary queue and do not touch
      * the "co" coroutine as soon as you enter another one.
      *
-     * In its turn resumed "co" can pupulate "co_queue_wakeup" queue with
+     * In its turn resumed "co" can populate "co_queue_wakeup" queue with
      * new coroutines to be woken up.  The caller, who has resumed "co",
      * will be responsible for traversing the same queue, which may cause
      * a different wakeup order but not any missing wakeups.
-- 
2.14.3

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Daniel P. Berrangé]

On Tue, Feb 13, 2018 at 05:34:25PM +0000, Stefan Hajnoczi wrote:
> From: Fam Zheng <famz@redhat.com>
> 
> git-publish [1] is a convenient tool to send patches and has been
> popular among QEMU developers.  Recently it has been made available in
> Fedora official repo thanks to Stefan's work.
> 
> One nice feature of the tool is a per-project configuration with
> profiles, especially in which the cccmd option is a handy method to
> create the Cc list.
> 
> [1]: https://github.com/stefanha/git-publish
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-id: 20180205054725.25634-2-famz@redhat.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  .gitpublish | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 .gitpublish
> 
> diff --git a/.gitpublish b/.gitpublish
> new file mode 100644
> index 0000000000..ed48f6e52c
> --- /dev/null
> +++ b/.gitpublish
> @@ -0,0 +1,58 @@
> +#
> +# Common git-publish profiles that can be used to send patches to QEMU upstream.
> +#
> +# See https://github.com/stefanha/git-publish for more information
> +#
> +[gitpublishprofile "default"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "rfc"]
> +base = master
> +prefix = RFC PATCH
> +to = qemu-devel@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "stable"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cc = qemu-stable@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "trivial"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cc = qemu-trivial@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "block"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cc = qemu-block@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null

Why is a custom entry needed for block here (and other things
below).   Won't running get_maintainer.pl already correctly
report when a patch needs cc'ing to qemu-block@nongnu.org
based on MAINTAINER rules ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 0/5] Block patches

[Fam Zheng]

On Tue, 02/13 17:34, Stefan Hajnoczi wrote:
> The following changes since commit fb68096da3d35e64c88cd610c1fa42766c58e92a:
> 
>   Revert "tests: use memfd in vhost-user-test" (2018-02-13 09:51:52 +0000)
> 
> are available in the Git repository at:
> 
>   git://github.com/stefanha/qemu.git tags/block-pull-request
> 
> for you to fetch changes up to 64b01feca991e5b19a5d750ef77cdca92b68bdbb:
> 
>   misc: fix spelling (2018-02-13 15:38:17 +0000)

Did you mean "PULL" in the subject?

Fam

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Fam Zheng]

On Tue, 02/13 18:09, Daniel P. Berrangé wrote:
> On Tue, Feb 13, 2018 at 05:34:25PM +0000, Stefan Hajnoczi wrote:
> > From: Fam Zheng <famz@redhat.com>
> > 
> > git-publish [1] is a convenient tool to send patches and has been
> > popular among QEMU developers.  Recently it has been made available in
> > Fedora official repo thanks to Stefan's work.
> > 
> > One nice feature of the tool is a per-project configuration with
> > profiles, especially in which the cccmd option is a handy method to
> > create the Cc list.
> > 
> > [1]: https://github.com/stefanha/git-publish
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Message-id: 20180205054725.25634-2-famz@redhat.com
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  .gitpublish | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >  create mode 100644 .gitpublish
> > 
> > diff --git a/.gitpublish b/.gitpublish
> > new file mode 100644
> > index 0000000000..ed48f6e52c
> > --- /dev/null
> > +++ b/.gitpublish
> > @@ -0,0 +1,58 @@
> > +#
> > +# Common git-publish profiles that can be used to send patches to QEMU upstream.
> > +#
> > +# See https://github.com/stefanha/git-publish for more information
> > +#
> > +[gitpublishprofile "default"]
> > +base = master
> > +prefix = PATCH
> > +to = qemu-devel@nongnu.org
> > +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> > +
> > +[gitpublishprofile "rfc"]
> > +base = master
> > +prefix = RFC PATCH
> > +to = qemu-devel@nongnu.org
> > +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> > +
> > +[gitpublishprofile "stable"]
> > +base = master
> > +prefix = PATCH
> > +to = qemu-devel@nongnu.org
> > +cc = qemu-stable@nongnu.org
> > +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> > +
> > +[gitpublishprofile "trivial"]
> > +base = master
> > +prefix = PATCH
> > +to = qemu-devel@nongnu.org
> > +cc = qemu-trivial@nongnu.org
> > +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> > +
> > +[gitpublishprofile "block"]
> > +base = master
> > +prefix = PATCH
> > +to = qemu-devel@nongnu.org
> > +cc = qemu-block@nongnu.org
> > +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> 
> Why is a custom entry needed for block here (and other things
> below).   Won't running get_maintainer.pl already correctly
> report when a patch needs cc'ing to qemu-block@nongnu.org
> based on MAINTAINER rules ?

Yeah, dropping them should be fine. What do you think, Stefan?

Fam

Re: [Qemu-devel] [PATCH 0/5] Block patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 949 bytes --]

On Wed, Feb 14, 2018 at 08:48:20AM +0800, Fam Zheng wrote:
> On Tue, 02/13 17:34, Stefan Hajnoczi wrote:
> > The following changes since commit fb68096da3d35e64c88cd610c1fa42766c58e92a:
> > 
> >   Revert "tests: use memfd in vhost-user-test" (2018-02-13 09:51:52 +0000)
> > 
> > are available in the Git repository at:
> > 
> >   git://github.com/stefanha/qemu.git tags/block-pull-request
> > 
> > for you to fetch changes up to 64b01feca991e5b19a5d750ef77cdca92b68bdbb:
> > 
> >   misc: fix spelling (2018-02-13 15:38:17 +0000)
> 
> Did you mean "PULL" in the subject?

git-publish is just obeying your profile settings:

+[gitpublishprofile "default"]
+base = master
+prefix = PATCH
 ^---- :)

I used PULL myself in the past but am happy with using whatever we
decide the standard should be.

Shall we remove prefix from the profiles (it defaults to "PATCH" for
regular patches and "PULL" for pull requests)?

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1253 bytes --]

On Wed, Feb 14, 2018 at 09:01:11AM +0800, Fam Zheng wrote:
> On Tue, 02/13 18:09, Daniel P. Berrangé wrote:
> > On Tue, Feb 13, 2018 at 05:34:25PM +0000, Stefan Hajnoczi wrote:
> > > +[gitpublishprofile "block"]
> > > +base = master
> > > +prefix = PATCH
> > > +to = qemu-devel@nongnu.org
> > > +cc = qemu-block@nongnu.org
> > > +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> > 
> > Why is a custom entry needed for block here (and other things
> > below).   Won't running get_maintainer.pl already correctly
> > report when a patch needs cc'ing to qemu-block@nongnu.org
> > based on MAINTAINER rules ?
> 
> Yeah, dropping them should be fine. What do you think, Stefan?

scripts/get_maintainer.pl only works when patches touch files associated
with qemu-block@nongnu.org in MAINTAINERS.  It would be
counter-intuitive for --profile block to not include
qemu-block@nongnu.org, so I think these entries do serve a purpose.

On the other hand, I personally just use --cc qemu-block instead of a
profile for this.  I'll probably only use the default profile.

Fam, if you want it dropped, let me know.  Otherwise, we can leave it
in if there is no further discussion.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Fam Zheng]

On Wed, Feb 14, 2018 at 9:34 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Wed, Feb 14, 2018 at 09:01:11AM +0800, Fam Zheng wrote:
> Fam, if you want it dropped, let me know.  Otherwise, we can leave it
> in if there is no further discussion.

Yeah, I think we can drop block/arm/s390/ppc and rely on get_maintainer.pl here.

Fam

Re: [Qemu-devel] [PATCH 0/5] Block patches

[Fam Zheng]

On Wed, Feb 14, 2018 at 9:25 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Wed, Feb 14, 2018 at 08:48:20AM +0800, Fam Zheng wrote:
>> On Tue, 02/13 17:34, Stefan Hajnoczi wrote:
>> > The following changes since commit fb68096da3d35e64c88cd610c1fa42766c58e92a:
>> >
>> >   Revert "tests: use memfd in vhost-user-test" (2018-02-13 09:51:52 +0000)
>> >
>> > are available in the Git repository at:
>> >
>> >   git://github.com/stefanha/qemu.git tags/block-pull-request
>> >
>> > for you to fetch changes up to 64b01feca991e5b19a5d750ef77cdca92b68bdbb:
>> >
>> >   misc: fix spelling (2018-02-13 15:38:17 +0000)
>>
>> Did you mean "PULL" in the subject?
>
> git-publish is just obeying your profile settings:
>
> +[gitpublishprofile "default"]
> +base = master
> +prefix = PATCH
>  ^---- :)
>
> I used PULL myself in the past but am happy with using whatever we
> decide the standard should be.
>
> Shall we remove prefix from the profiles (it defaults to "PATCH" for
> regular patches and "PULL" for pull requests)?

Yes, this is a bug. In my ~/.gitconfig I had "prefix = PATCH ..." for
all other qemu-* profiles but not the main one, "qemu". When I wrote
the patch I didn't realize it is so for a reason, and "fixed" it!

Fam

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Paolo Bonzini]

On 13/02/2018 19:09, Daniel P. Berrangé wrote:
> Why is a custom entry needed for block here (and other things
> below).   Won't running get_maintainer.pl already correctly
> report when a patch needs cc'ing to qemu-block@nongnu.org
> based on MAINTAINER rules ?

Sometimes you want to CC block people even on a non-block patch, for
example if you want their opinion about using the API.

We should probably add at least arm and ppc profiles, too.

> --nogit --nogit-fallback

QEMU's getmaintainer (unlike the Linux version!) already tries to avoid
falling back to git if a maintainer is listed.  Are these options necessary?

Paolo

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Paolo Bonzini]

On 14/02/2018 15:08, Paolo Bonzini wrote:
> On 13/02/2018 19:09, Daniel P. Berrangé wrote:
>> Why is a custom entry needed for block here (and other things
>> below).   Won't running get_maintainer.pl already correctly
>> report when a patch needs cc'ing to qemu-block@nongnu.org
>> based on MAINTAINER rules ?
> 
> Sometimes you want to CC block people even on a non-block patch, for
> example if you want their opinion about using the API.
> 
> We should probably add at least arm and ppc profiles, too.

Nevermind, they were there below qemu-block.

Paolo

>> --nogit --nogit-fallback
> 
> QEMU's getmaintainer (unlike the Linux version!) already tries to avoid
> falling back to git if a maintainer is listed.  Are these options necessary?
> 
> Paolo
> 

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Eric Blake]

On 02/14/2018 07:34 AM, Stefan Hajnoczi wrote:
> On Wed, Feb 14, 2018 at 09:01:11AM +0800, Fam Zheng wrote:
>> On Tue, 02/13 18:09, Daniel P. Berrangé wrote:
>>> On Tue, Feb 13, 2018 at 05:34:25PM +0000, Stefan Hajnoczi wrote:
>>>> +[gitpublishprofile "block"]
>>>> +base = master
>>>> +prefix = PATCH
>>>> +to = qemu-devel@nongnu.org
>>>> +cc = qemu-block@nongnu.org
>>>> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
>>>
>>> Why is a custom entry needed for block here (and other things
>>> below).   Won't running get_maintainer.pl already correctly
>>> report when a patch needs cc'ing to qemu-block@nongnu.org
>>> based on MAINTAINER rules ?
>>
>> Yeah, dropping them should be fine. What do you think, Stefan?
> 
> scripts/get_maintainer.pl only works when patches touch files associated
> with qemu-block@nongnu.org in MAINTAINERS.  It would be
> counter-intuitive for --profile block to not include
> qemu-block@nongnu.org, so I think these entries do serve a purpose.

More precisely, the cover letter does not touch any files, and git 
doesn't know how to automatically cc the cover letter to any recipient 
of any of the rest of the series.  Having these entries ensures that the 
cover letter goes to the right list (on the other hand, git publish 
itself could be taught to be smarter about cc'ing the cover letter to 
every email scraped from later in the series, even if git send-email 
can't do that yet).

> 
> On the other hand, I personally just use --cc qemu-block instead of a
> profile for this.  I'll probably only use the default profile.
> 
> Fam, if you want it dropped, let me know.  Otherwise, we can leave it
> in if there is no further discussion.
> 
> Stefan
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Philippe Mathieu-Daudé]

On 02/13/2018 02:34 PM, Stefan Hajnoczi wrote:
> From: Fam Zheng <famz@redhat.com>
> 
> git-publish [1] is a convenient tool to send patches and has been
> popular among QEMU developers.  Recently it has been made available in
> Fedora official repo thanks to Stefan's work.

Also Debian:
https://packages.debian.org/sid/git-publish

> 
> One nice feature of the tool is a per-project configuration with
> profiles, especially in which the cccmd option is a handy method to
> create the Cc list.
> 
> [1]: https://github.com/stefanha/git-publish
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-id: 20180205054725.25634-2-famz@redhat.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  .gitpublish | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 .gitpublish
> 
> diff --git a/.gitpublish b/.gitpublish
> new file mode 100644
> index 0000000000..ed48f6e52c
> --- /dev/null
> +++ b/.gitpublish
> @@ -0,0 +1,58 @@
> +#
> +# Common git-publish profiles that can be used to send patches to QEMU upstream.
> +#
> +# See https://github.com/stefanha/git-publish for more information
> +#
> +[gitpublishprofile "default"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "rfc"]
> +base = master
> +prefix = RFC PATCH
> +to = qemu-devel@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "stable"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cc = qemu-stable@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "trivial"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cc = qemu-trivial@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "block"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cc = qemu-block@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "arm"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cc = qemu-arm@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "s390"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cc = qemu-s390@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> +
> +[gitpublishprofile "ppc"]
> +base = master
> +prefix = PATCH
> +to = qemu-devel@nongnu.org
> +cc = qemu-ppc@nongnu.org
> +cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit --nogit-fallback 2>/dev/null
> 

Re: [Qemu-devel] [PATCH 0/5] Block patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]

On Wed, Feb 14, 2018 at 09:58:27PM +0800, Fam Zheng wrote:
> On Wed, Feb 14, 2018 at 9:25 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Wed, Feb 14, 2018 at 08:48:20AM +0800, Fam Zheng wrote:
> >> On Tue, 02/13 17:34, Stefan Hajnoczi wrote:
> >> > The following changes since commit fb68096da3d35e64c88cd610c1fa42766c58e92a:
> >> >
> >> >   Revert "tests: use memfd in vhost-user-test" (2018-02-13 09:51:52 +0000)
> >> >
> >> > are available in the Git repository at:
> >> >
> >> >   git://github.com/stefanha/qemu.git tags/block-pull-request
> >> >
> >> > for you to fetch changes up to 64b01feca991e5b19a5d750ef77cdca92b68bdbb:
> >> >
> >> >   misc: fix spelling (2018-02-13 15:38:17 +0000)
> >>
> >> Did you mean "PULL" in the subject?
> >
> > git-publish is just obeying your profile settings:
> >
> > +[gitpublishprofile "default"]
> > +base = master
> > +prefix = PATCH
> >  ^---- :)
> >
> > I used PULL myself in the past but am happy with using whatever we
> > decide the standard should be.
> >
> > Shall we remove prefix from the profiles (it defaults to "PATCH" for
> > regular patches and "PULL" for pull requests)?
> 
> Yes, this is a bug. In my ~/.gitconfig I had "prefix = PATCH ..." for
> all other qemu-* profiles but not the main one, "qemu". When I wrote
> the patch I didn't realize it is so for a reason, and "fixed" it!

Please resend your series once this email thread has run its course.

I am dropping it from the pull request because I'd like to merge the
other fixes in this pull request as soon as possible.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Fam Zheng]

On Wed, 02/14 15:08, Paolo Bonzini wrote:
> On 13/02/2018 19:09, Daniel P. Berrangé wrote:
> > Why is a custom entry needed for block here (and other things
> > below).   Won't running get_maintainer.pl already correctly
> > report when a patch needs cc'ing to qemu-block@nongnu.org
> > based on MAINTAINER rules ?
> 
> Sometimes you want to CC block people even on a non-block patch, for
> example if you want their opinion about using the API.
> 
> We should probably add at least arm and ppc profiles, too.
> 
> > --nogit --nogit-fallback
> 
> QEMU's getmaintainer (unlike the Linux version!) already tries to avoid
> falling back to git if a maintainer is listed.  Are these options necessary?
> 

Falling back to git is usually more noisy than useful even when a maintainer is
not listed, in my past experience. That's why I don't use it myself and propose
such here. But I'm open to different opinions.

Fam

Re: [Qemu-devel] [PATCH 1/5] Add a git-publish configuration file

[Fam Zheng]

On Wed, 02/14 14:37, Philippe Mathieu-Daudé wrote:
> On 02/13/2018 02:34 PM, Stefan Hajnoczi wrote:
> > From: Fam Zheng <famz@redhat.com>
> > 
> > git-publish [1] is a convenient tool to send patches and has been
> > popular among QEMU developers.  Recently it has been made available in
> > Fedora official repo thanks to Stefan's work.
> 
> Also Debian:
> https://packages.debian.org/sid/git-publish

Thank you for doing that! I will add the pointer.

Fam