[Qemu-devel] [PATCH 00/10] Trivial fixes and clean ups

[Qemu-devel] [PATCH 00/10] Trivial fixes and clean ups

[Cleber Rosa]

Just a collection of trivial fixes and clean ups that have been lying
around here for some time.

Cleber Rosa (10):
  qemu-img.c: comment typo fix
  tests/tcg/README: fix location for lm32 tests
  qemu-iotests: make 218 executable
  qemu-iotests: fix filename containing checks
  docs/devel/testing.rst: add missing newlines after code block
  thunk.c: clean up commented out definition
  scripts/decodetree.py: remove unused imports
  scripts/decodetree.py: fix reference to attributes
  scripts/qemu.py: use a more consistent docstring style
  scripts/qemu.py: remove trailing quotes on docstring

 docs/devel/testing.rst       |  2 ++
 dtc                          |  2 +-
 qemu-img.c                   |  2 +-
 scripts/decodetree.py        |  4 +--
 scripts/qemu.py              | 67 ++++++++++++++++++++++--------------
 tests/qemu-iotests/218       |  0
 tests/qemu-iotests/common.rc |  4 +--
 tests/tcg/README             |  2 +-
 thunk.c                      |  2 --
 9 files changed, 50 insertions(+), 35 deletions(-)
 mode change 100644 => 100755 tests/qemu-iotests/218

-- 
2.17.1

[Qemu-devel] [PATCH 01/10] qemu-img.c: comment typo fix

[Cleber Rosa]

A trivial comment typo fix.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index b12f4cd19b..a96b76c187 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf, int64_t n)
 }
 
 /*
- * Returns true iff the first sector pointed to by 'buf' contains at least
+ * Returns true if the first sector pointed to by 'buf' contains at least
  * a non-NUL byte.
  *
  * 'pnum' is set to the number of sectors (including and immediately following
-- 
2.17.1

Re: [Qemu-devel] [PATCH 01/10] qemu-img.c: comment typo fix

[Eric Blake]

On 10/4/18 11:18 AM, Cleber Rosa wrote:
> A trivial comment typo fix.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   qemu-img.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index b12f4cd19b..a96b76c187 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf, int64_t n)
>   }
>   
>   /*
> - * Returns true iff the first sector pointed to by 'buf' contains at least
> + * Returns true if the first sector pointed to by 'buf' contains at least

NACK.  You're not the first person to propose this change.  However, 
"iff" is an English word (albeit archaic) which is shorthand for "if and 
only if", which has a distinct logical meaning separate from the weaker 
"if".  Spelling it out in longhand instead of calling it a typo is 
probably acceptable, though.

>    * a non-NUL byte.
>    *
>    * 'pnum' is set to the number of sectors (including and immediately following
> 

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

Re: [Qemu-devel] [PATCH 01/10] qemu-img.c: comment typo fix

[Cleber Rosa]

On 10/4/18 12:48 PM, Eric Blake wrote:
> On 10/4/18 11:18 AM, Cleber Rosa wrote:
>> A trivial comment typo fix.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>   qemu-img.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index b12f4cd19b..a96b76c187 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf,
>> int64_t n)
>>   }
>>     /*
>> - * Returns true iff the first sector pointed to by 'buf' contains at
>> least
>> + * Returns true if the first sector pointed to by 'buf' contains at
>> least
> 
> NACK.  You're not the first person to propose this change.  However,
> "iff" is an English word (albeit archaic) which is shorthand for "if and
> only if", which has a distinct logical meaning separate from the weaker
> "if".  Spelling it out in longhand instead of calling it a typo is
> probably acceptable, though.
> 

Wow, thanks for the explanation.  I agree that spelling it out is a good
option, as it'll probably prevent other people from falling into the
same "language trap" as I did.

Thanks!
- Cleber.

Re: [Qemu-devel] [PATCH 01/10] qemu-img.c: comment typo fix

[Philippe Mathieu-Daudé]

On 04/10/2018 18:53, Cleber Rosa wrote:> On 10/4/18 12:48 PM, Eric Blake
wrote:
>> On 10/4/18 11:18 AM, Cleber Rosa wrote:
>>> A trivial comment typo fix.
>>>
>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>> ---
>>>   qemu-img.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/qemu-img.c b/qemu-img.c
>>> index b12f4cd19b..a96b76c187 100644
>>> --- a/qemu-img.c
>>> +++ b/qemu-img.c
>>> @@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf,
>>> int64_t n)
>>>   }
>>>     /*
>>> - * Returns true iff the first sector pointed to by 'buf' contains at
>>> least
>>> + * Returns true if the first sector pointed to by 'buf' contains at
>>> least
>>
>> NACK.  You're not the first person to propose this change.  However,
>> "iff" is an English word (albeit archaic) which is shorthand for "if and
>> only if", which has a distinct logical meaning separate from the weaker
>> "if".  Spelling it out in longhand instead of calling it a typo is
>> probably acceptable, though.
>>
> 
> Wow, thanks for the explanation.  I agree that spelling it out is a good
> option, as it'll probably prevent other people from falling into the
> same "language trap" as I did.

https://lists.gnu.org/archive/html/qemu-devel/2011-06/msg01269.html

https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg04200.html

;)

Re: [Qemu-devel] [PATCH 01/10] qemu-img.c: comment typo fix

[Peter Maydell]

On 4 October 2018 at 17:48, Eric Blake <eblake@redhat.com> wrote:
> On 10/4/18 11:18 AM, Cleber Rosa wrote:
>>
>> A trivial comment typo fix.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>

>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1085,7 +1085,7 @@ static int64_t find_nonzero(const uint8_t *buf,
>> int64_t n)
>>   }
>>     /*
>> - * Returns true iff the first sector pointed to by 'buf' contains at
>> least
>> + * Returns true if the first sector pointed to by 'buf' contains at least
>
>
> NACK.  You're not the first person to propose this change.  However, "iff"
> is an English word (albeit archaic) which is shorthand for "if and only if",
> which has a distinct logical meaning separate from the weaker "if".
> Spelling it out in longhand instead of calling it a typo is probably
> acceptable, though.

Yes; if you happen to have a mathematical background then it's
a familiar abbreviation; but otherwise it isn't, and it's more
confusing than helpful I think. (I don't think it's archaic; it's
just commonly used in university-and-higher-level maths and not
elsewhere.)

 git grep '\Wiff\W'
shows a surprisingly large number of uses. (NB that some of those
are in 3rd-party code, notably the libdecnumber stuff, and should
probably not be changed.)

thanks
-- PMM

[Qemu-devel] [PATCH 02/10] tests/tcg/README: fix location for lm32 tests

[Cleber Rosa]

Point to the right and obvious location for lm32 tests.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/tcg/README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/tcg/README b/tests/tcg/README
index a5643d33e7..2a58f9a058 100644
--- a/tests/tcg/README
+++ b/tests/tcg/README
@@ -10,6 +10,6 @@ with "make test-cris".
 
 LM32
 ====
-The testsuite for LM32 is in tests/tcg/cris.  You can run it
+The testsuite for LM32 is in tests/tcg/lm32.  You can run it
 with "make test-lm32".
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH 02/10] tests/tcg/README: fix location for lm32 tests

[Thomas Huth]

On 2018-10-04 18:18, Cleber Rosa wrote:
> Point to the right and obvious location for lm32 tests.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/tcg/README | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/tcg/README b/tests/tcg/README
> index a5643d33e7..2a58f9a058 100644
> --- a/tests/tcg/README
> +++ b/tests/tcg/README
> @@ -10,6 +10,6 @@ with "make test-cris".
>  
>  LM32
>  ====
> -The testsuite for LM32 is in tests/tcg/cris.  You can run it
> +The testsuite for LM32 is in tests/tcg/lm32.  You can run it
>  with "make test-lm32".

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 02/10] tests/tcg/README: fix location for lm32 tests

[Alex Bennée]

Cleber Rosa <crosa@redhat.com> writes:

> Point to the right and obvious location for lm32 tests.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/tcg/README | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/tcg/README b/tests/tcg/README
> index a5643d33e7..2a58f9a058 100644
> --- a/tests/tcg/README
> +++ b/tests/tcg/README
> @@ -10,6 +10,6 @@ with "make test-cris".
>
>  LM32
>  ====
> -The testsuite for LM32 is in tests/tcg/cris.  You can run it
> +The testsuite for LM32 is in tests/tcg/lm32.  You can run it
>  with "make test-lm32".

Acked-by: Alex Bennée <alex.bennee@linaro.org>

It's a correct fix, although I note running "make test-lm32" doesn't do
much either in the top of the build dir or in that directory. It's
probably best to drop the second line until I fix it.

--
Alex Bennée

Re: [Qemu-devel] [Qemu-trivial] [PATCH 02/10] tests/tcg/README: fix location for lm32 tests

[Laurent Vivier]

On 04/10/2018 17:18, Cleber Rosa wrote:
> Point to the right and obvious location for lm32 tests.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/tcg/README | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/tcg/README b/tests/tcg/README
> index a5643d33e7..2a58f9a058 100644
> --- a/tests/tcg/README
> +++ b/tests/tcg/README
> @@ -10,6 +10,6 @@ with "make test-cris".
>  
>  LM32
>  ====
> -The testsuite for LM32 is in tests/tcg/cris.  You can run it
> +The testsuite for LM32 is in tests/tcg/lm32.  You can run it
>  with "make test-lm32".
>  
> 

Applied

Thanks,
Laurent

[Qemu-devel] [PATCH 03/10] qemu-iotests: make 218 executable

[Cleber Rosa]

Commit 990dc39c made all tests executable at the time, but 218 came in
later, and missing those permissions.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/qemu-iotests/218 | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 tests/qemu-iotests/218

diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
old mode 100644
new mode 100755
-- 
2.17.1

Re: [Qemu-devel] [PATCH 03/10] qemu-iotests: make 218 executable

[Eric Blake]

On 10/4/18 11:18 AM, Cleber Rosa wrote:
> Commit 990dc39c made all tests executable at the time, but 218 came in
> later, and missing those permissions.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/qemu-iotests/218 | 0
>   1 file changed, 0 insertions(+), 0 deletions(-)
>   mode change 100644 => 100755 tests/qemu-iotests/218
> 
> diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
> old mode 100644
> new mode 100755

Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [Qemu-devel] [PATCH 03/10] qemu-iotests: make 218 executable

[Philippe Mathieu-Daudé]

On 04/10/2018 18:18, Cleber Rosa wrote:
> Commit 990dc39c made all tests executable at the time, but 218 came in
> later, and missing those permissions.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/qemu-iotests/218 | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  mode change 100644 => 100755 tests/qemu-iotests/218
> 
> diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
> old mode 100644
> new mode 100755

We could add a git pre-commit hook grep'ing for "^#!\
*(/usr/bin/env|/bin/(b?a)?sh)" in the 1st line and then checking the
file perms.

Re: [Qemu-devel] [PATCH 03/10] qemu-iotests: make 218 executable

[Cleber Rosa]

On 10/5/18 9:36 AM, Philippe Mathieu-Daudé wrote:
> On 04/10/2018 18:18, Cleber Rosa wrote:
>> Commit 990dc39c made all tests executable at the time, but 218 came in
>> later, and missing those permissions.
>>
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  tests/qemu-iotests/218 | 0
>>  1 file changed, 0 insertions(+), 0 deletions(-)
>>  mode change 100644 => 100755 tests/qemu-iotests/218
>>
>> diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
>> old mode 100644
>> new mode 100755
> 
> We could add a git pre-commit hook grep'ing for "^#!\
> *(/usr/bin/env|/bin/(b?a)?sh)" in the 1st line and then checking the
> file perms.
> 

Good idea.  Probably better to send that as a different patch, right?

Re: [Qemu-devel] [PATCH 03/10] qemu-iotests: make 218 executable

[Eric Blake]

On 10/5/18 12:49 PM, Cleber Rosa wrote:

>>
>> We could add a git pre-commit hook grep'ing for "^#!\
>> *(/usr/bin/env|/bin/(b?a)?sh)" in the 1st line and then checking the
>> file perms.
>>
> 
> Good idea.  Probably better to send that as a different patch, right?

Yes, separate patch. For that matter, "^#!/" is probably a sufficient 
pattern to catch any file that intends to be run via an interpreter, and 
therefore wants to be executable, rather than limiting to just env and 
specific shells.

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

Re: [Qemu-devel] [PATCH 03/10] qemu-iotests: make 218 executable

[Eric Blake]

On 10/5/18 1:02 PM, Eric Blake wrote:
> On 10/5/18 12:49 PM, Cleber Rosa wrote:
> 
>>>
>>> We could add a git pre-commit hook grep'ing for "^#!\
>>> *(/usr/bin/env|/bin/(b?a)?sh)" in the 1st line and then checking the
>>> file perms.
>>>
>>
>> Good idea.  Probably better to send that as a different patch, right?
> 
> Yes, separate patch. For that matter, "^#!/" is probably a sufficient 
> pattern to catch any file that intends to be run via an interpreter, and 
> therefore wants to be executable, rather than limiting to just env and 
> specific shells.

Also, many maintainers already have scripts/checkpatch.pl as a git 
pre-commit hook, and putting the check in checkpatch.pl means that 
patchew would also flag the issue.

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

Re: [Qemu-devel] [Qemu-trivial] [PATCH 03/10] qemu-iotests: make 218 executable

[Laurent Vivier]

On 04/10/2018 17:18, Cleber Rosa wrote:
> Commit 990dc39c made all tests executable at the time, but 218 came in
> later, and missing those permissions.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/qemu-iotests/218 | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  mode change 100644 => 100755 tests/qemu-iotests/218
> 
> diff --git a/tests/qemu-iotests/218 b/tests/qemu-iotests/218
> old mode 100644
> new mode 100755
> 

Applied

Thanks,
Laurent

[Qemu-devel] [PATCH 04/10] qemu-iotests: fix filename containing checks

[Cleber Rosa]

Commit cce293a2945 moved some functions from common.config to
common.rc, but the error messages still reference the old file
location.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/qemu-iotests/common.rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 44bee16a5e..70ca65b49b 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -170,7 +170,7 @@ if [ ! -e "$TEST_DIR" ]; then
 fi
 
 if [ ! -d "$TEST_DIR" ]; then
-    echo "common.config: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
+    echo "common.rc: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
     exit 1
 fi
 
@@ -179,7 +179,7 @@ if [ -z "$REMOTE_TEST_DIR" ]; then
 fi
 
 if [ ! -d "$SAMPLE_IMG_DIR" ]; then
-    echo "common.config: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a directory"
+    echo "common.rc: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a directory"
     exit 1
 fi
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH 04/10] qemu-iotests: fix filename containing checks

[Philippe Mathieu-Daudé]

On 04/10/2018 18:18, Cleber Rosa wrote:
> Commit cce293a2945 moved some functions from common.config to
> common.rc, but the error messages still reference the old file
> location.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  tests/qemu-iotests/common.rc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 44bee16a5e..70ca65b49b 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -170,7 +170,7 @@ if [ ! -e "$TEST_DIR" ]; then
>  fi
>  
>  if [ ! -d "$TEST_DIR" ]; then
> -    echo "common.config: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
> +    echo "common.rc: Error: \$TEST_DIR ($TEST_DIR) is not a directory"
>      exit 1
>  fi
>  
> @@ -179,7 +179,7 @@ if [ -z "$REMOTE_TEST_DIR" ]; then
>  fi
>  
>  if [ ! -d "$SAMPLE_IMG_DIR" ]; then
> -    echo "common.config: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a directory"
> +    echo "common.rc: Error: \$SAMPLE_IMG_DIR ($SAMPLE_IMG_DIR) is not a directory"
>      exit 1
>  fi
>  
> 

Re: [Qemu-devel] [Qemu-trivial] [PATCH 04/10] qemu-iotests: fix filename containing checks

[Laurent Vivier]

On 04/10/2018 17:18, Cleber Rosa wrote:
> Commit cce293a2945 moved some functions from common.config to
> common.rc, but the error messages still reference the old file
> location.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/qemu-iotests/common.rc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied

Thanks,
Laurent

[Qemu-devel] [PATCH 05/10] docs/devel/testing.rst: add missing newlines after code block

[Cleber Rosa]

The line immediate following a ".. code::" block is considered
to contains arguments to the "code directive".  The lack of a
new line gives me during at parse time:

   testing.rst:63: (ERROR/3) Error in "code" directive:
   maximum 1 argument(s) allowed, 3 supplied.

   .. code::
     make check-unit V=1

   testing.rst:120: (ERROR/3) Error in "code" directive:
   maximum 1 argument(s) allowed, 3 supplied.

   .. code::
     make check-qtest V=1

Let's add the missing newlines, both for consistency and to
avoid the parsing errors.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 docs/devel/testing.rst | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 727c4019b5..9355ad49f2 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -61,6 +61,7 @@ variable (which affects memory reclamation and catches invalid pointers better)
 and gtester options. If necessary, you can run
 
 .. code::
+
   make check-unit V=1
 
 and copy the actual command line which executes the unit test, then run
@@ -118,6 +119,7 @@ and using gdb on the test is still simple to do: find out the actual command
 from the output of
 
 .. code::
+
   make check-qtest V=1
 
 which you can run manually.
-- 
2.17.1

Re: [Qemu-devel] [Qemu-block] [PATCH 05/10] docs/devel/testing.rst: add missing newlines after code block

[John Snow]

On 10/04/2018 12:18 PM, Cleber Rosa wrote:
> The line immediate following a ".. code::" block is considered
> to contains arguments to the "code directive".  The lack of a
> new line gives me during at parse time:
> 
>    testing.rst:63: (ERROR/3) Error in "code" directive:
>    maximum 1 argument(s) allowed, 3 supplied.
> 
>    .. code::
>      make check-unit V=1
> 
>    testing.rst:120: (ERROR/3) Error in "code" directive:
>    maximum 1 argument(s) allowed, 3 supplied.
> 
>    .. code::
>      make check-qtest V=1
> 

pandoc doesn't complain, but rst.ninjs.org does. What tool did you use
to find these? we should formalize a formatting checker for rst files.

Actually, we should formalize building our RST docs at all...

--js

> Let's add the missing newlines, both for consistency and to
> avoid the parsing errors.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [Qemu-trivial] [PATCH 05/10] docs/devel/testing.rst: add missing newlines after code block

[Laurent Vivier]

On 04/10/2018 17:18, Cleber Rosa wrote:
> The line immediate following a ".. code::" block is considered
> to contains arguments to the "code directive".  The lack of a
> new line gives me during at parse time:
> 
>    testing.rst:63: (ERROR/3) Error in "code" directive:
>    maximum 1 argument(s) allowed, 3 supplied.
> 
>    .. code::
>      make check-unit V=1
> 
>    testing.rst:120: (ERROR/3) Error in "code" directive:
>    maximum 1 argument(s) allowed, 3 supplied.
> 
>    .. code::
>      make check-qtest V=1
> 
> Let's add the missing newlines, both for consistency and to
> avoid the parsing errors.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  docs/devel/testing.rst | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 727c4019b5..9355ad49f2 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -61,6 +61,7 @@ variable (which affects memory reclamation and catches invalid pointers better)
>  and gtester options. If necessary, you can run
>  
>  .. code::
> +
>    make check-unit V=1
>  
>  and copy the actual command line which executes the unit test, then run
> @@ -118,6 +119,7 @@ and using gdb on the test is still simple to do: find out the actual command
>  from the output of
>  
>  .. code::
> +
>    make check-qtest V=1
>  
>  which you can run manually.
> 

Applied

Thanks,
Laurent

[Qemu-devel] [PATCH 06/10] thunk.c: clean up commented out definition

[Cleber Rosa]

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 thunk.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/thunk.c b/thunk.c
index d5d8645cd4..e351ae53af 100644
--- a/thunk.c
+++ b/thunk.c
@@ -21,8 +21,6 @@
 #include "qemu.h"
 #include "exec/user/thunk.h"
 
-//#define DEBUG
-
 static unsigned int max_struct_entries;
 StructEntry *struct_entries;
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH 06/10] thunk.c: clean up commented out definition

[Laurent Vivier]

Le 04/10/2018 à 18:18, Cleber Rosa a écrit :
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  thunk.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/thunk.c b/thunk.c
> index d5d8645cd4..e351ae53af 100644
> --- a/thunk.c
> +++ b/thunk.c
> @@ -21,8 +21,6 @@
>  #include "qemu.h"
>  #include "exec/user/thunk.h"
>  
> -//#define DEBUG
> -

Generally we use that to enable easily existing debug traces.

In this case, if you remove the "#define" I think you should also
replace the "#ifdef DEBUG"s by some trace_XXXX() functions.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 06/10] thunk.c: clean up commented out definition

[Cleber Rosa]

On 10/4/18 12:24 PM, Laurent Vivier wrote:
> Le 04/10/2018 à 18:18, Cleber Rosa a écrit :
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>  thunk.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/thunk.c b/thunk.c
>> index d5d8645cd4..e351ae53af 100644
>> --- a/thunk.c
>> +++ b/thunk.c
>> @@ -21,8 +21,6 @@
>>  #include "qemu.h"
>>  #include "exec/user/thunk.h"
>>  
>> -//#define DEBUG
>> -
> 
> Generally we use that to enable easily existing debug traces.
> 

Oh, I wasn't aware that was intentional (I clearly lack some background
here).

> In this case, if you remove the "#define" I think you should also
> replace the "#ifdef DEBUG"s by some trace_XXXX() functions.
> 

Is that desirable? I don't want to disrupt the status quo just because
it looked odd to me.

Thanks!
- Cleber.

> Thanks,
> Laurent
> 

Re: [Qemu-devel] [PATCH 06/10] thunk.c: clean up commented out definition

[Laurent Vivier]

Le 04/10/2018 à 18:31, Cleber Rosa a écrit :
> 
> 
> On 10/4/18 12:24 PM, Laurent Vivier wrote:
>> Le 04/10/2018 à 18:18, Cleber Rosa a écrit :
>>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>>> ---
>>>  thunk.c | 2 --
>>>  1 file changed, 2 deletions(-)
>>>
>>> diff --git a/thunk.c b/thunk.c
>>> index d5d8645cd4..e351ae53af 100644
>>> --- a/thunk.c
>>> +++ b/thunk.c
>>> @@ -21,8 +21,6 @@
>>>  #include "qemu.h"
>>>  #include "exec/user/thunk.h"
>>>  
>>> -//#define DEBUG
>>> -
>>
>> Generally we use that to enable easily existing debug traces.
>>
> 
> Oh, I wasn't aware that was intentional (I clearly lack some background
> here).
> 
>> In this case, if you remove the "#define" I think you should also
>> replace the "#ifdef DEBUG"s by some trace_XXXX() functions.
>>
> 
> Is that desirable? I don't want to disrupt the status quo just because
> it looked odd to me.

Yes, it's better to have trace_XXX() functions than DEBUG macros. The
change is trivial (add the format strings in ./trace-events and call
them from thunk.c with the existing parameters)

Thanks,
Laurent

[Qemu-devel] [PATCH 07/10] scripts/decodetree.py: remove unused imports

[Cleber Rosa]

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 scripts/decodetree.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 277f9a9bba..457cffea90 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -149,12 +149,10 @@
 #   trans_addl_i(ctx, &arg_opi, insn)
 #
 
-import io
 import os
 import re
 import sys
 import getopt
-import pdb
 
 insnwidth = 32
 insnmask = 0xffffffff
-- 
2.17.1

Re: [Qemu-devel] [PATCH 07/10] scripts/decodetree.py: remove unused imports

[Philippe Mathieu-Daudé]

On 04/10/2018 18:18, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  scripts/decodetree.py | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/scripts/decodetree.py b/scripts/decodetree.py
> index 277f9a9bba..457cffea90 100755
> --- a/scripts/decodetree.py
> +++ b/scripts/decodetree.py
> @@ -149,12 +149,10 @@
>  #   trans_addl_i(ctx, &arg_opi, insn)
>  #
>  
> -import io
>  import os
>  import re
>  import sys
>  import getopt
> -import pdb
>  
>  insnwidth = 32
>  insnmask = 0xffffffff
> 

Re: [Qemu-devel] [Qemu-trivial] [PATCH 07/10] scripts/decodetree.py: remove unused imports

[Laurent Vivier]

On 04/10/2018 17:18, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  scripts/decodetree.py | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/scripts/decodetree.py b/scripts/decodetree.py
> index 277f9a9bba..457cffea90 100755
> --- a/scripts/decodetree.py
> +++ b/scripts/decodetree.py
> @@ -149,12 +149,10 @@
>  #   trans_addl_i(ctx, &arg_opi, insn)
>  #
>  
> -import io
>  import os
>  import re
>  import sys
>  import getopt
> -import pdb
>  
>  insnwidth = 32
>  insnmask = 0xffffffff
> 

Applied

Thanks,
Laurent

[Qemu-devel] [PATCH 08/10] scripts/decodetree.py: fix reference to attributes

[Cleber Rosa]

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 scripts/decodetree.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 457cffea90..37c76b5507 100755
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -298,7 +298,7 @@ class Field:
             s = 's'
         else:
             s = ''
-        return str(pos) + ':' + s + str(len)
+        return str(self.pos) + ':' + s + str(self.len)
 
     def str_extract(self):
         if self.sign:
-- 
2.17.1

Re: [Qemu-devel] [PATCH 08/10] scripts/decodetree.py: fix reference to attributes

[Philippe Mathieu-Daudé]

On 04/10/2018 18:18, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  scripts/decodetree.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/decodetree.py b/scripts/decodetree.py
> index 457cffea90..37c76b5507 100755
> --- a/scripts/decodetree.py
> +++ b/scripts/decodetree.py
> @@ -298,7 +298,7 @@ class Field:
>              s = 's'
>          else:
>              s = ''
> -        return str(pos) + ':' + s + str(len)
> +        return str(self.pos) + ':' + s + str(self.len)
>  
>      def str_extract(self):
>          if self.sign:
> 

[Qemu-devel] [PATCH 09/10] scripts/qemu.py: use a more consistent docstring style

[Cleber Rosa]

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 dtc             |  2 +-
 scripts/qemu.py | 65 +++++++++++++++++++++++++++++++------------------
 2 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/dtc b/dtc
index 88f18909db..e54388015a 160000
--- a/dtc
+++ b/dtc
@@ -1 +1 @@
-Subproject commit 88f18909db731a627456f26d779445f84e449536
+Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42
diff --git a/scripts/qemu.py b/scripts/qemu.py
index f099ce7278..7abe26de69 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -53,9 +53,9 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
     """
 
 class MonitorResponseError(qmp.qmp.QMPError):
-    '''
+    """
     Represents erroneous QMP monitor reply
-    '''
+    """
     def __init__(self, reply):
         try:
             desc = reply["error"]["desc"]
@@ -66,14 +66,15 @@ class MonitorResponseError(qmp.qmp.QMPError):
 
 
 class QEMUMachine(object):
-    '''A QEMU VM
+    """
+    A QEMU VM
 
     Use this object as a context manager to ensure the QEMU process terminates::
 
         with VM(binary) as vm:
             ...
         # vm is guaranteed to be shut down here
-    '''
+    """
 
     def __init__(self, binary, args=None, wrapper=None, name=None,
                  test_dir="/var/tmp", monitor_address=None,
@@ -135,7 +136,9 @@ class QEMUMachine(object):
         self._args.append(args)
 
     def add_fd(self, fd, fdset, opaque, opts=''):
-        '''Pass a file descriptor to the VM'''
+        """
+        Pass a file descriptor to the VM
+        """
         options = ['fd=%d' % fd,
                    'set=%d' % fdset,
                    'opaque=%s' % opaque]
@@ -168,7 +171,9 @@ class QEMUMachine(object):
 
     @staticmethod
     def _remove_if_exists(path):
-        '''Remove file object at path if it exists'''
+        """
+        Remove file object at path if it exists
+        """
         try:
             os.remove(path)
         except OSError as exception:
@@ -271,7 +276,9 @@ class QEMUMachine(object):
             raise
 
     def _launch(self):
-        '''Launch the VM and establish a QMP connection'''
+        """
+        Launch the VM and establish a QMP connection
+        """
         devnull = open(os.path.devnull, 'rb')
         self._pre_launch()
         self._qemu_full_args = (self._wrapper + [self._binary] +
@@ -284,14 +291,18 @@ class QEMUMachine(object):
         self._post_launch()
 
     def wait(self):
-        '''Wait for the VM to power off'''
+        """
+        Wait for the VM to power off
+        """
         self._popen.wait()
         self._qmp.close()
         self._load_io_log()
         self._post_shutdown()
 
     def shutdown(self):
-        '''Terminate the VM and clean up'''
+        """
+        Terminate the VM and clean up
+        """
         if self.is_running():
             try:
                 self._qmp.cmd('quit')
@@ -315,7 +326,9 @@ class QEMUMachine(object):
         self._launched = False
 
     def qmp(self, cmd, conv_keys=True, **args):
-        '''Invoke a QMP command and return the response dict'''
+        """
+        Invoke a QMP command and return the response dict
+        """
         qmp_args = dict()
         for key, value in args.items():
             if conv_keys:
@@ -326,11 +339,11 @@ class QEMUMachine(object):
         return self._qmp.cmd(cmd, args=qmp_args)
 
     def command(self, cmd, conv_keys=True, **args):
-        '''
+        """
         Invoke a QMP command.
         On success return the response dict.
         On failure raise an exception.
-        '''
+        """
         reply = self.qmp(cmd, conv_keys, **args)
         if reply is None:
             raise qmp.qmp.QMPError("Monitor is closed")
@@ -339,13 +352,17 @@ class QEMUMachine(object):
         return reply["return"]
 
     def get_qmp_event(self, wait=False):
-        '''Poll for one queued QMP events and return it'''
+        """
+        Poll for one queued QMP events and return it
+        """
         if len(self._events) > 0:
             return self._events.pop(0)
         return self._qmp.pull_event(wait=wait)
 
     def get_qmp_events(self, wait=False):
-        '''Poll for queued QMP events and return a list of dicts'''
+        """
+        Poll for queued QMP events and return a list of dicts
+        """
         events = self._qmp.get_events(wait=wait)
         events.extend(self._events)
         del self._events[:]
@@ -353,7 +370,7 @@ class QEMUMachine(object):
         return events
 
     def event_wait(self, name, timeout=60.0, match=None):
-        '''
+        """
         Wait for specified timeout on named event in QMP; optionally filter
         results by match.
 
@@ -361,7 +378,7 @@ class QEMUMachine(object):
         branch processing on match's value None
            {"foo": {"bar": 1}} matches {"foo": None}
            {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
-        '''
+        """
         def event_match(event, match=None):
             if match is None:
                 return True
@@ -394,29 +411,29 @@ class QEMUMachine(object):
         return None
 
     def get_log(self):
-        '''
+        """
         After self.shutdown or failed qemu execution, this returns the output
         of the qemu process.
-        '''
+        """
         return self._iolog
 
     def add_args(self, *args):
-        '''
+        """
         Adds to the list of extra arguments to be given to the QEMU binary
-        '''
+        """
         self._args.extend(args)
 
     def set_machine(self, machine_type):
-        '''
+        """
         Sets the machine type
 
         If set, the machine type will be added to the base arguments
         of the resulting QEMU command line.
-        '''
+        """
         self._machine = machine_type
 
     def set_console(self, device_type=None):
-        '''
+        """
         Sets the device type for a console device
 
         If set, the console device and a backing character device will
@@ -434,7 +451,7 @@ class QEMUMachine(object):
         @param device_type: the device type, such as "isa-serial"
         @raises: QEMUMachineAddDeviceError if the device type is not given
                  and can not be determined.
-        '''
+        """
         if device_type is None:
             if self._machine is None:
                 raise QEMUMachineAddDeviceError("Can not add a console device:"
-- 
2.17.1

Re: [Qemu-devel] [PATCH 09/10] scripts/qemu.py: use a more consistent docstring style

[Eric Blake]

On 10/4/18 11:18 AM, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   dtc             |  2 +-
>   scripts/qemu.py | 65 +++++++++++++++++++++++++++++++------------------
>   2 files changed, 42 insertions(+), 25 deletions(-)
> 
> diff --git a/dtc b/dtc
> index 88f18909db..e54388015a 160000
> --- a/dtc
> +++ b/dtc
> @@ -1 +1 @@
> -Subproject commit 88f18909db731a627456f26d779445f84e449536
> +Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42

The submodule change was probably unintended.

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

Re: [Qemu-devel] [PATCH 09/10] scripts/qemu.py: use a more consistent docstring style

[Cleber Rosa]

On 10/4/18 12:50 PM, Eric Blake wrote:
> On 10/4/18 11:18 AM, Cleber Rosa wrote:
>> Signed-off-by: Cleber Rosa <crosa@redhat.com>
>> ---
>>   dtc             |  2 +-
>>   scripts/qemu.py | 65 +++++++++++++++++++++++++++++++------------------
>>   2 files changed, 42 insertions(+), 25 deletions(-)
>>
>> diff --git a/dtc b/dtc
>> index 88f18909db..e54388015a 160000
>> --- a/dtc
>> +++ b/dtc
>> @@ -1 +1 @@
>> -Subproject commit 88f18909db731a627456f26d779445f84e449536
>> +Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42
> 
> The submodule change was probably unintended.
> 

Yep, my bad.

Re: [Qemu-devel] [Qemu-block] [PATCH 09/10] scripts/qemu.py: use a more consistent docstring style

[John Snow]

On 10/04/2018 12:18 PM, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  dtc             |  2 +-
>  scripts/qemu.py | 65 +++++++++++++++++++++++++++++++------------------
>  2 files changed, 42 insertions(+), 25 deletions(-)
> 
> diff --git a/dtc b/dtc
> index 88f18909db..e54388015a 160000
> --- a/dtc
> +++ b/dtc
> @@ -1 +1 @@
> -Subproject commit 88f18909db731a627456f26d779445f84e449536
> +Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index f099ce7278..7abe26de69 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -53,9 +53,9 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
>      """
>  
>  class MonitorResponseError(qmp.qmp.QMPError):
> -    '''
> +    """
>      Represents erroneous QMP monitor reply
> -    '''
> +    """

This seems obviously correct, as per the Python Dogma Handbook ...

>      def __init__(self, reply):
>          try:
>              desc = reply["error"]["desc"]
> @@ -66,14 +66,15 @@ class MonitorResponseError(qmp.qmp.QMPError):
>  
>  
>  class QEMUMachine(object):
> -    '''A QEMU VM
> +    """
> +    A QEMU VM
>  
>      Use this object as a context manager to ensure the QEMU process terminates::
>  
>          with VM(binary) as vm:
>              ...
>          # vm is guaranteed to be shut down here
> -    '''
> +    """

As does this,,

>  
>      def __init__(self, binary, args=None, wrapper=None, name=None,
>                   test_dir="/var/tmp", monitor_address=None,
> @@ -135,7 +136,9 @@ class QEMUMachine(object):
>          self._args.append(args)
>  
>      def add_fd(self, fd, fdset, opaque, opts=''):
> -        '''Pass a file descriptor to the VM'''
> +        """
> +        Pass a file descriptor to the VM
> +        """

However, is it established practice among ne'er-do-wells to format
one-line docstrings as three-liners? (And without punctuation to boot --
for shame!)

PEP257 suggests that one-liners are allowed, but doesn't seem to
necessitate their usage. Does this kind of change have any kind of benefit?

>          options = ['fd=%d' % fd,
>                     'set=%d' % fdset,
>                     'opaque=%s' % opaque]
> @@ -168,7 +171,9 @@ class QEMUMachine(object):
>  
>      @staticmethod
>      def _remove_if_exists(path):
> -        '''Remove file object at path if it exists'''
> +        """
> +        Remove file object at path if it exists
> +        """
>          try:
>              os.remove(path)
>          except OSError as exception:
> @@ -271,7 +276,9 @@ class QEMUMachine(object):
>              raise
>  
>      def _launch(self):
> -        '''Launch the VM and establish a QMP connection'''
> +        """
> +        Launch the VM and establish a QMP connection
> +        """
>          devnull = open(os.path.devnull, 'rb')
>          self._pre_launch()
>          self._qemu_full_args = (self._wrapper + [self._binary] +
> @@ -284,14 +291,18 @@ class QEMUMachine(object):
>          self._post_launch()
>  
>      def wait(self):
> -        '''Wait for the VM to power off'''
> +        """
> +        Wait for the VM to power off
> +        """
>          self._popen.wait()
>          self._qmp.close()
>          self._load_io_log()
>          self._post_shutdown()
>  
>      def shutdown(self):
> -        '''Terminate the VM and clean up'''
> +        """
> +        Terminate the VM and clean up
> +        """
>          if self.is_running():
>              try:
>                  self._qmp.cmd('quit')
> @@ -315,7 +326,9 @@ class QEMUMachine(object):
>          self._launched = False
>  
>      def qmp(self, cmd, conv_keys=True, **args):
> -        '''Invoke a QMP command and return the response dict'''
> +        """
> +        Invoke a QMP command and return the response dict
> +        """
>          qmp_args = dict()
>          for key, value in args.items():
>              if conv_keys:
> @@ -326,11 +339,11 @@ class QEMUMachine(object):
>          return self._qmp.cmd(cmd, args=qmp_args)
>  
>      def command(self, cmd, conv_keys=True, **args):
> -        '''
> +        """
>          Invoke a QMP command.
>          On success return the response dict.
>          On failure raise an exception.
> -        '''
> +        """
>          reply = self.qmp(cmd, conv_keys, **args)
>          if reply is None:
>              raise qmp.qmp.QMPError("Monitor is closed")
> @@ -339,13 +352,17 @@ class QEMUMachine(object):
>          return reply["return"]
>  
>      def get_qmp_event(self, wait=False):
> -        '''Poll for one queued QMP events and return it'''
> +        """
> +        Poll for one queued QMP events and return it
> +        """
>          if len(self._events) > 0:
>              return self._events.pop(0)
>          return self._qmp.pull_event(wait=wait)
>  
>      def get_qmp_events(self, wait=False):
> -        '''Poll for queued QMP events and return a list of dicts'''
> +        """
> +        Poll for queued QMP events and return a list of dicts
> +        """
>          events = self._qmp.get_events(wait=wait)
>          events.extend(self._events)
>          del self._events[:]
> @@ -353,7 +370,7 @@ class QEMUMachine(object):
>          return events
>  
>      def event_wait(self, name, timeout=60.0, match=None):
> -        '''
> +        """
>          Wait for specified timeout on named event in QMP; optionally filter
>          results by match.
>  
> @@ -361,7 +378,7 @@ class QEMUMachine(object):
>          branch processing on match's value None
>             {"foo": {"bar": 1}} matches {"foo": None}
>             {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
> -        '''
> +        """
>          def event_match(event, match=None):
>              if match is None:
>                  return True
> @@ -394,29 +411,29 @@ class QEMUMachine(object):
>          return None
>  
>      def get_log(self):
> -        '''
> +        """
>          After self.shutdown or failed qemu execution, this returns the output
>          of the qemu process.
> -        '''
> +        """
>          return self._iolog
>  
>      def add_args(self, *args):
> -        '''
> +        """
>          Adds to the list of extra arguments to be given to the QEMU binary
> -        '''
> +        """
>          self._args.extend(args)
>  
>      def set_machine(self, machine_type):
> -        '''
> +        """
>          Sets the machine type
>  
>          If set, the machine type will be added to the base arguments
>          of the resulting QEMU command line.
> -        '''
> +        """
>          self._machine = machine_type
>  
>      def set_console(self, device_type=None):
> -        '''
> +        """
>          Sets the device type for a console device
>  
>          If set, the console device and a backing character device will
> @@ -434,7 +451,7 @@ class QEMUMachine(object):
>          @param device_type: the device type, such as "isa-serial"
>          @raises: QEMUMachineAddDeviceError if the device type is not given
>                   and can not be determined.
> -        '''
> +        """
>          if device_type is None:
>              if self._machine is None:
>                  raise QEMUMachineAddDeviceError("Can not add a console device:"
> 

Re: [Qemu-devel] [Qemu-block] [PATCH 09/10] scripts/qemu.py: use a more consistent docstring style

[Eduardo Habkost]

On Mon, Oct 08, 2018 at 03:44:14PM -0400, John Snow wrote:
> 
> 
> On 10/04/2018 12:18 PM, Cleber Rosa wrote:
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  dtc             |  2 +-
> >  scripts/qemu.py | 65 +++++++++++++++++++++++++++++++------------------
> >  2 files changed, 42 insertions(+), 25 deletions(-)
> > 
> > diff --git a/dtc b/dtc
> > index 88f18909db..e54388015a 160000
> > --- a/dtc
> > +++ b/dtc
> > @@ -1 +1 @@
> > -Subproject commit 88f18909db731a627456f26d779445f84e449536
> > +Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42
> > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > index f099ce7278..7abe26de69 100644
> > --- a/scripts/qemu.py
> > +++ b/scripts/qemu.py
> > @@ -53,9 +53,9 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
> >      """
> >  
> >  class MonitorResponseError(qmp.qmp.QMPError):
> > -    '''
> > +    """
> >      Represents erroneous QMP monitor reply
> > -    '''
> > +    """
> 
> This seems obviously correct, as per the Python Dogma Handbook ...
> 
[...]
> >      def add_fd(self, fd, fdset, opaque, opts=''):
> > -        '''Pass a file descriptor to the VM'''
> > +        """
> > +        Pass a file descriptor to the VM
> > +        """
> 
> However, is it established practice among ne'er-do-wells to format
> one-line docstrings as three-liners? (And without punctuation to boot --
> for shame!)
> 
> PEP257 suggests that one-liners are allowed, but doesn't seem to
> necessitate their usage. Does this kind of change have any kind of benefit?

I don't mind having one-line docstrings.  But if we're already
touching multiple docstrings, consistency with the rest of the
module code sounds nice.

I'm queueing this on python-next.

-- 
Eduardo

[Qemu-devel] [PATCH 10/10] scripts/qemu.py: remove trailing quotes on docstring

[Cleber Rosa]

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 scripts/qemu.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 7abe26de69..676eb9709a 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -88,7 +88,7 @@ class QEMUMachine(object):
         @param name: prefix for socket and log file names (default: qemu-PID)
         @param test_dir: where to create socket and log file
         @param monitor_address: address for QMP monitor
-        @param socket_scm_helper: helper program, required for send_fd_scm()"
+        @param socket_scm_helper: helper program, required for send_fd_scm()
         @note: Qemu process is not started until launch() is used.
         '''
         if args is None:
-- 
2.17.1

Re: [Qemu-devel] [PATCH 10/10] scripts/qemu.py: remove trailing quotes on docstring

[Philippe Mathieu-Daudé]

On 04/10/2018 18:18, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <crosa@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  scripts/qemu.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 7abe26de69..676eb9709a 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -88,7 +88,7 @@ class QEMUMachine(object):
>          @param name: prefix for socket and log file names (default: qemu-PID)
>          @param test_dir: where to create socket and log file
>          @param monitor_address: address for QMP monitor
> -        @param socket_scm_helper: helper program, required for send_fd_scm()"
> +        @param socket_scm_helper: helper program, required for send_fd_scm()
>          @note: Qemu process is not started until launch() is used.
>          '''
>          if args is None:
> 

Re: [Qemu-devel] [Qemu-trivial] [PATCH 10/10] scripts/qemu.py: remove trailing quotes on docstring

[Laurent Vivier]

On 04/10/2018 17:18, Cleber Rosa wrote:
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  scripts/qemu.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 7abe26de69..676eb9709a 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -88,7 +88,7 @@ class QEMUMachine(object):
>          @param name: prefix for socket and log file names (default: qemu-PID)
>          @param test_dir: where to create socket and log file
>          @param monitor_address: address for QMP monitor
> -        @param socket_scm_helper: helper program, required for send_fd_scm()"
> +        @param socket_scm_helper: helper program, required for send_fd_scm()
>          @note: Qemu process is not started until launch() is used.
>          '''
>          if args is None:
> 

Applied

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 00/10] Trivial fixes and clean ups

[Eduardo Habkost]

On Thu, Oct 04, 2018 at 12:18:42PM -0400, Cleber Rosa wrote:
> Just a collection of trivial fixes and clean ups that have been lying
> around here for some time.

I'm queueing patches 07-10 on python-next.  I'm aware that
Laurent already queued some of them (thanks!), but I want to
avoid conflicts in case other patches touching these modules are
queued on python-next.

-- 
Eduardo