[libvirt] [PATCH 3/2] docs: Document adaptive timeout for qemu monitor

Michal Privoznik posted 2 patches 8 years, 11 months ago
[libvirt] [PATCH 3/2] docs: Document adaptive timeout for qemu monitor
Posted by Michal Privoznik 8 years, 11 months ago
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 docs/news.xml | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/docs/news.xml b/docs/news.xml
index 04783aa5e..6ce6ab362 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -43,7 +43,20 @@
       </change>
     </section>
     <section title="Bug fixes">
-      <change/>
+      <change>
+        <summary>
+          qemu: Adaptive timeout for connecting to monitor
+        </summary>
+        <description>
+          When starting qemu, libvirt waits for qemu to create the monitor
+          socket which libvirt connects to. Historically, there was sharp 30
+          seconds timeout after which the qemu process was killed. This
+          approach is suboptimal as in some scenarios with huge amounts of
+          guest RAM it can take a minute or more for kernel to allocate and
+          zero out pages for qemu. The timeout is now flexible and computed by
+          libvirt at domain startup.
+        </description>
+        </change>
     </section>
   </release>
   <release version="v3.1.0" date="2017-03-03">
-- 
2.11.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/2] docs: Document adaptive timeout for qemu monitor
Posted by Martin Kletzander 8 years, 11 months ago
On Wed, Mar 15, 2017 at 01:06:12PM +0100, Michal Privoznik wrote:
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> docs/news.xml | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
>diff --git a/docs/news.xml b/docs/news.xml
>index 04783aa5e..6ce6ab362 100644
>--- a/docs/news.xml
>+++ b/docs/news.xml
>@@ -43,7 +43,20 @@
>       </change>
>     </section>
>     <section title="Bug fixes">
>-      <change/>
>+      <change>
>+        <summary>
>+          qemu: Adaptive timeout for connecting to monitor
>+        </summary>
>+        <description>
>+          When starting qemu, libvirt waits for qemu to create the monitor
>+          socket which libvirt connects to. Historically, there was sharp 30
>+          seconds timeout after which the qemu process was killed. This

s/seconds/second/ (as in 30-second timeout) feels more English, I guess.

>+          approach is suboptimal as in some scenarios with huge amounts of
>+          guest RAM it can take a minute or more for kernel to allocate and
>+          zero out pages for qemu. The timeout is now flexible and computed by
>+          libvirt at domain startup.
>+        </description>
>+        </change>
>     </section>

I'll take the opportunity to repeat here what I said in another
news-related thread.

The good thing about the new news file layout is that we can express
freely what is the feature that was added and we don't have to
copy-paste the restricted commit messages.  So I, personally, like to
have nicely formatted sentences in the summary (well, I like that even
in the commit messages sometimes, but that's another story).  I would go
with:

 "When connecting to qemu monitor, the timeout is now adaptive"

or even:

 "Better (or dynamic determination) strategy is now used for qemu monitor connection timeout"

for the summary.  Or something along the lines.  What I say is (as
before) pretty subjective, so I'll leave the final decision up to you,
just wanted to put it out there (yet again).  I'm trying to imagine the
user going through these and immediately having an idea of the list of
things being done.  Commit names are more for developers.

ACK

P.S.: Isn't it nice when you can just copy-paste reviews? =D

P.P.S.: Would it be to much if I also said that shortening the
        description to something like the following would be enough?

          "When launching QEMU, libvirt now adjusts the timeout for the
           initial connection dynamically as oposed to the static 30
           seconds before"

Still, your decision on that.

>   </release>
>   <release version="v3.1.0" date="2017-03-03">
>--
>2.11.0
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/2] docs: Document adaptive timeout for qemu monitor
Posted by Andrea Bolognani 8 years, 11 months ago
On Wed, 2017-03-15 at 14:10 +0100, Martin Kletzander wrote:
> I'll take the opportunity to repeat here what I said in another
> news-related thread.
> 
> The good thing about the new news file layout is that we can express
> freely what is the feature that was added and we don't have to
> copy-paste the restricted commit messages.  So I, personally, like to
> have nicely formatted sentences in the summary (well, I like that even
> in the commit messages sometimes, but that's another story).  I would go
> with:
> 
>  "When connecting to qemu monitor, the timeout is now adaptive"
> 
> or even:
> 
>  "Better (or dynamic determination) strategy is now used for qemu monitor connection timeout"
> 
> for the summary.  Or something along the lines.  What I say is (as
> before) pretty subjective, so I'll leave the final decision up to you,
> just wanted to put it out there (yet again).  I'm trying to imagine the
> user going through these and immediately having an idea of the list of
> things being done.  Commit names are more for developers.

I think having a prefix, such a "qemu:" in this case, helps
you scanning the release notes and very quickly realize
whether or not any of the changes are relevant to you.

I also believe that, while we don't necessarily have to
artificially limit ourselves, having a fairly short summary
is usually good for the same reasons explained above.

So basically I like Michal's original summary more than I
like yours ;)

I would s/Adaptive/Use adaptive/ though.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/2] docs: Document adaptive timeout for qemu monitor
Posted by Andrea Bolognani 8 years, 11 months ago
On Wed, 2017-03-15 at 14:10 +0100, Martin Kletzander wrote:
I would s/Adaptive/Use adaptive/ though.

Oh, and it should be "QEMU" rather than "qemu" in the
description ;)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/2] docs: Document adaptive timeout for qemu monitor
Posted by Martin Kletzander 8 years, 11 months ago
On Wed, Mar 15, 2017 at 03:40:04PM +0100, Andrea Bolognani wrote:
>On Wed, 2017-03-15 at 14:10 +0100, Martin Kletzander wrote:
>> I'll take the opportunity to repeat here what I said in another
>> news-related thread.
>> 
>> The good thing about the new news file layout is that we can express
>> freely what is the feature that was added and we don't have to
>> copy-paste the restricted commit messages.  So I, personally, like to
>> have nicely formatted sentences in the summary (well, I like that even
>> in the commit messages sometimes, but that's another story).  I would go
>> with:
>> 
>>  "When connecting to qemu monitor, the timeout is now adaptive"
>> 
>> or even:
>> 
>>  "Better (or dynamic determination) strategy is now used for qemu monitor connection timeout"
>> 
>> for the summary.  Or something along the lines.  What I say is (as
>> before) pretty subjective, so I'll leave the final decision up to you,
>> just wanted to put it out there (yet again).  I'm trying to imagine the
>> user going through these and immediately having an idea of the list of
>> things being done.  Commit names are more for developers.
>
>I think having a prefix, such a "qemu:" in this case, helps
>you scanning the release notes and very quickly realize
>whether or not any of the changes are relevant to you.
>

OK, fair enough, in this case (some other changes have non-related
prefix every now and then).

>I also believe that, while we don't necessarily have to
>artificially limit ourselves, having a fairly short summary
>is usually good for the same reasons explained above.
>
>So basically I like Michal's original summary more than I
>like yours ;)
>
>I would s/Adaptive/Use adaptive/ though.
>

Sure, that makes it a sentence as well ;)  So it fits my requirement as
well =)

>-- 
>Andrea Bolognani / Red Hat / Virtualization
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list