Re: [PATCH v2] python/qemu/qmp.py: QMP debug with VM label

2020-03-16 Thread Oksana Voshchana
Hi Eduardo
I'm already fixing it.

Thank you,

On Sun, Mar 15, 2020 at 5:39 PM Eduardo Habkost  wrote:

> On Thu, Mar 12, 2020 at 04:05:47PM +0200, Oksana Vohchana wrote:
> > QEMUMachine writes some messages to the default logger.
> > But it sometimes hard to read the output if we have requests to
> > more than one VM.
> > This patch adds a label to the logger in the debug mode.
> >
> > Signed-off-by: Oksana Vohchana 
> >
> > ---
> > v2:
> >  - Instead of shown the label in the message it provides the label
> >only in the debug logger information
> > ---
> >  python/qemu/machine.py | 2 +-
> >  python/qemu/qmp.py | 5 -
> >  2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> > index 183d8f3d38..d0aa774c1c 100644
> > --- a/python/qemu/machine.py
> > +++ b/python/qemu/machine.py
> > @@ -270,7 +270,7 @@ class QEMUMachine(object):
> >  self._vm_monitor = os.path.join(self._sock_dir,
> >  self._name +
> "-monitor.sock")
> >  self._remove_files.append(self._vm_monitor)
> > -self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
> server=True)
> > +self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor,
> server=True, nickname=self._name)
> >
> >  def _post_launch(self):
> >  if self._qmp:
> > diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
> > index f40586eedd..d58b18c304 100644
> > --- a/python/qemu/qmp.py
> > +++ b/python/qemu/qmp.py
> > @@ -46,7 +46,7 @@ class QEMUMonitorProtocol:
> >  #: Logger object for debugging messages
> >  logger = logging.getLogger('QMP')
>
> This will create a single logger instance.
>
> >
> > -def __init__(self, address, server=False):
> > +def __init__(self, address, server=False, nickname=None):
> >  """
> >  Create a QEMUMonitorProtocol class.
> >
> > @@ -62,6 +62,7 @@ class QEMUMonitorProtocol:
> >  self.__address = address
> >  self.__sock = self.__get_sock()
> >  self.__sockfile = None
> > +self._nickname = nickname
> >  if server:
> >  self.__sock.setsockopt(socket.SOL_SOCKET,
> socket.SO_REUSEADDR, 1)
> >  self.__sock.bind(self.__address)
> > @@ -188,6 +189,8 @@ class QEMUMonitorProtocol:
> >  @return QMP response as a Python dict or None if the connection
> has
> >  been closed
> >  """
> > +if self._nickname:
> > +self.logger.name = 'QMP.{}'.format(self._nickname)
>
> This will change the name of that single instance and affect
> every single QEMUMonitorProtocol object.  Please don't do that.
>
> You can just do:
>
> self.logger = logging.getLogger('QMP').getChild(self._nickname)
>
> at __init__().
>
>
> >  self.logger.debug(">>> %s", qmp_cmd)
> >  try:
> >  self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8'))
> > --
> > 2.21.1
> >
>
> --
> Eduardo
>
>


Re: [PATCH v2] python/qemu/qmp.py: QMP debug with VM label

2020-03-15 Thread Eduardo Habkost
On Thu, Mar 12, 2020 at 04:05:47PM +0200, Oksana Vohchana wrote:
> QEMUMachine writes some messages to the default logger.
> But it sometimes hard to read the output if we have requests to
> more than one VM.
> This patch adds a label to the logger in the debug mode.
> 
> Signed-off-by: Oksana Vohchana 
> 
> ---
> v2:
>  - Instead of shown the label in the message it provides the label
>only in the debug logger information
> ---
>  python/qemu/machine.py | 2 +-
>  python/qemu/qmp.py | 5 -
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> index 183d8f3d38..d0aa774c1c 100644
> --- a/python/qemu/machine.py
> +++ b/python/qemu/machine.py
> @@ -270,7 +270,7 @@ class QEMUMachine(object):
>  self._vm_monitor = os.path.join(self._sock_dir,
>  self._name + "-monitor.sock")
>  self._remove_files.append(self._vm_monitor)
> -self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor, 
> server=True)
> +self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor, 
> server=True, nickname=self._name)
>  
>  def _post_launch(self):
>  if self._qmp:
> diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
> index f40586eedd..d58b18c304 100644
> --- a/python/qemu/qmp.py
> +++ b/python/qemu/qmp.py
> @@ -46,7 +46,7 @@ class QEMUMonitorProtocol:
>  #: Logger object for debugging messages
>  logger = logging.getLogger('QMP')

This will create a single logger instance.

>  
> -def __init__(self, address, server=False):
> +def __init__(self, address, server=False, nickname=None):
>  """
>  Create a QEMUMonitorProtocol class.
>  
> @@ -62,6 +62,7 @@ class QEMUMonitorProtocol:
>  self.__address = address
>  self.__sock = self.__get_sock()
>  self.__sockfile = None
> +self._nickname = nickname
>  if server:
>  self.__sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
>  self.__sock.bind(self.__address)
> @@ -188,6 +189,8 @@ class QEMUMonitorProtocol:
>  @return QMP response as a Python dict or None if the connection has
>  been closed
>  """
> +if self._nickname:
> +self.logger.name = 'QMP.{}'.format(self._nickname)

This will change the name of that single instance and affect
every single QEMUMonitorProtocol object.  Please don't do that.

You can just do:

self.logger = logging.getLogger('QMP').getChild(self._nickname)

at __init__().


>  self.logger.debug(">>> %s", qmp_cmd)
>  try:
>  self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8'))
> -- 
> 2.21.1
> 

-- 
Eduardo




Re: [PATCH v2] python/qemu/qmp.py: QMP debug with VM label

2020-03-12 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20200312140547.20448-1-ovosh...@redhat.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==6128==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==6189==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==6189==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffd78f93000; bottom 0x7f21b4f2; size: 0x00dbc4073000 (943886643200)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
==6204==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 25 test-aio /aio-gsource/event/wait
PASS 26 test-aio /aio-gsource/event/flush
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
==6212==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
==6218==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
==6224==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==6230==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==6244==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 4 ide-test /x86_64/ide/bmdma/trim
==6255==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 test-aio-multithread /aio/multi/mutex/contended
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
==6271==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
==6283==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
==6287==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
---
PASS 2 test-hbitmap /hbitmap/size/0
PASS 3 test-hbitmap /hbitmap/size/unaligned
PASS 4 test-hbitmap /hbitmap/iter/empty
==6354==WARNING: ASan doesn't fully support 

Re: [PATCH v2] python/qemu/qmp.py: QMP debug with VM label

2020-03-12 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20200312140547.20448-1-ovosh...@redhat.com/



Hi,

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

Subject: [PATCH v2] python/qemu/qmp.py: QMP debug with VM label
Message-id: 20200312140547.20448-1-ovosh...@redhat.com
Type: series

=== 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
Switched to a new branch 'test'
8a0ac8f python/qemu/qmp.py: QMP debug with VM label

=== OUTPUT BEGIN ===
ERROR: line over 90 characters
#25: FILE: python/qemu/machine.py:273:
+self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor, server=True, 
nickname=self._name)

total: 1 errors, 0 warnings, 31 lines checked

Commit 8a0ac8f2203e (python/qemu/qmp.py: QMP debug with VM label) has style 
problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200312140547.20448-1-ovosh...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH v2] python/qemu/qmp.py: QMP debug with VM label

2020-03-12 Thread Oksana Vohchana
QEMUMachine writes some messages to the default logger.
But it sometimes hard to read the output if we have requests to
more than one VM.
This patch adds a label to the logger in the debug mode.

Signed-off-by: Oksana Vohchana 

---
v2:
 - Instead of shown the label in the message it provides the label
   only in the debug logger information
---
 python/qemu/machine.py | 2 +-
 python/qemu/qmp.py | 5 -
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 183d8f3d38..d0aa774c1c 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -270,7 +270,7 @@ class QEMUMachine(object):
 self._vm_monitor = os.path.join(self._sock_dir,
 self._name + "-monitor.sock")
 self._remove_files.append(self._vm_monitor)
-self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor, server=True)
+self._qmp = qmp.QEMUMonitorProtocol(self._vm_monitor, server=True, 
nickname=self._name)
 
 def _post_launch(self):
 if self._qmp:
diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
index f40586eedd..d58b18c304 100644
--- a/python/qemu/qmp.py
+++ b/python/qemu/qmp.py
@@ -46,7 +46,7 @@ class QEMUMonitorProtocol:
 #: Logger object for debugging messages
 logger = logging.getLogger('QMP')
 
-def __init__(self, address, server=False):
+def __init__(self, address, server=False, nickname=None):
 """
 Create a QEMUMonitorProtocol class.
 
@@ -62,6 +62,7 @@ class QEMUMonitorProtocol:
 self.__address = address
 self.__sock = self.__get_sock()
 self.__sockfile = None
+self._nickname = nickname
 if server:
 self.__sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
 self.__sock.bind(self.__address)
@@ -188,6 +189,8 @@ class QEMUMonitorProtocol:
 @return QMP response as a Python dict or None if the connection has
 been closed
 """
+if self._nickname:
+self.logger.name = 'QMP.{}'.format(self._nickname)
 self.logger.debug(">>> %s", qmp_cmd)
 try:
 self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8'))
-- 
2.21.1