[PATCH 3/3] block: drop BlockBackendRootState::read_only

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
Instead of keeping additional boolean field, let's store the information in BDRV_O_RDWR bit of BlockBackendRootState::open_flags. Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block_int.h | 1 - block/block-backend.c | 10 ++ blockdev.c| 4 +++- 3

[PATCH 2/3] block: drop BlockDriverState::read_only

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
This variable is just a cache for !(bs->open_flags & BDRV_O_RDWR), which we have to synchronize everywhere. Let's just drop it and consistently use bdrv_is_read_only(). Signed-off-by: Vladimir Sementsov-Ogievskiy --- include/block/block_int.h| 1 - block.c | 7

[PATCH 1/3] block: consistently use bdrv_is_read_only()

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
It's better to use accessor function instead of bs->read_only directly. In some places use bdrv_is_writable() instead of checking both BDRV_O_RDWR set and BDRV_O_INACTIVE not set. In bdrv_open_common() it's a bit strange to add one more variable, but we are going to drop bs->read_only in the next

[PATCH 0/3] block: drop BlockDriverState::read_only

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
Hi all! The field duplicates information in .open_flags. We have to carefully sync these two fields everywhere. It's simple to introduce a bug by forgetting it. Let's drop the field, and fix users to call bdrv_is_read_only() and bdrv_is_writable() instead. Vladimir Sementsov-Ogievskiy (3):

Re: [PATCH v7 31/31] gitlab: add python linters to CI

2021-05-26 Thread John Snow
On 5/26/21 2:47 PM, Vladimir Sementsov-Ogievskiy wrote: build requisite for PyPI packages in the event that PyPI only has a sdist and not a bdist for a given dependency during installation. i.e. some packages are compiled during installation? Realized I didn't answer this directly. Yes,

Re: [PATCH v7 31/31] gitlab: add python linters to CI

2021-05-26 Thread John Snow
On 5/26/21 2:47 PM, Vladimir Sementsov-Ogievskiy wrote: Worth leaving the comment somewhere? (not worth any kind of resending of course) Yeah, I'll explain the package selection in the commit message at least. --js

Re: [PATCH v4 04/15] qemu-iotests: add option to attach gdbserver

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: Define -gdb flag and GDB_OPTIONS environment variable to python tests to attach a gdbserver to each qemu instance. This patch only adds and parses this flag, it does not yet add the implementation for it. if -gdb is not provided but

Re: [PATCH v7 31/31] gitlab: add python linters to CI

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 21:24, John Snow wrote: On 5/26/21 5:33 AM, Vladimir Sementsov-Ogievskiy wrote: 26.05.2021 03:24, John Snow wrote: Add a python container that contains just enough juice for us to run the python code quality analysis tools. Base this container on fedora, because fedora has very

Re: [PATCH v7 29/31] python: add .gitignore

2021-05-26 Thread John Snow
On 5/26/21 5:18 AM, Vladimir Sementsov-Ogievskiy wrote: 26.05.2021 03:24, John Snow wrote: Ignore *Python* build and package output (build, dist, qemu.egg-info); these files are not created as part of a QEMU build. They are created by running the commands 'python3 setup.py ' when preparing

Re: [PATCH v7 31/31] gitlab: add python linters to CI

2021-05-26 Thread John Snow
On 5/26/21 11:32 AM, Alex Bennée wrote: John Snow writes: Add a python container that contains just enough juice for us to run the python code quality analysis tools. Base this container on fedora, because fedora has very convenient packaging for testing multiple python versions. Add two

[PATCH] iotests/check: move general long options to double dash

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
So, the change: -makecheck -> --makecheck -valgrind -> --valgrind -nocache -> --nocache -misalign -> --misalign Motivation: 1. Several long options are already have double dash: --dry-run, --color, --groups, --exclude-groups, --start-from 2. -misalign is used to add --misalign

Re: [PATCH v7 31/31] gitlab: add python linters to CI

2021-05-26 Thread John Snow
On 5/26/21 5:33 AM, Vladimir Sementsov-Ogievskiy wrote: 26.05.2021 03:24, John Snow wrote: Add a python container that contains just enough juice for us to run the python code quality analysis tools. Base this container on fedora, because fedora has very convenient packaging for testing

Re: [PATCH v7 17/31] python: add pylint to pipenv

2021-05-26 Thread John Snow
On 5/26/21 5:14 AM, Vladimir Sementsov-Ogievskiy wrote: 26.05.2021 03:24, John Snow wrote: We are specifying >= pylint 2.8.x for several reasons: 1. For setup.cfg support, added in pylint 2.5.x 2. To specify a version that has incompatibly dropped     bad-whitespace checks (2.6.x) 3. 2.7.x

Re: [PATCH v4 00/15] qemu_iotests: improve debugging options

2021-05-26 Thread Emanuele Giuseppe Esposito
On 26/05/2021 13:32, Vladimir Sementsov-Ogievskiy wrote: 20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: This series adds the option to attach gdbserver and valgrind to the QEMU binary running in qemu_iotests. It also allows to redirect QEMU binaries output of the python tests to the

Re: [PATCH v4 04/15] qemu-iotests: add option to attach gdbserver

2021-05-26 Thread Emanuele Giuseppe Esposito
On 26/05/2021 15:25, Vladimir Sementsov-Ogievskiy wrote: 26.05.2021 15:48, Paolo Bonzini wrote: On 26/05/21 13:24, Vladimir Sementsov-Ogievskiy wrote: Define -gdb flag and GDB_OPTIONS environment variable Let's use --option notation for new long options Why make a mix of two styles?

Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 17:48, Paolo Bonzini wrote: On 25/05/21 13:00, Vladimir Sementsov-Ogievskiy wrote: Hmm. OK, let me think: First look at block_copy_do_copy(). It's called only from block_copy_task_entry. block_copy_task_entry() has mutex-critical-section anyway around handling return value. That

Re: [PATCH v2 4/7] block-copy: add a CoMutex to the BlockCopyTask list

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 17:58, Paolo Bonzini wrote: On 25/05/21 12:25, Vladimir Sementsov-Ogievskiy wrote: Next, even if we take bitmaps lock in bdrv_dirty_bitmap_next_dirty_area() or around it, it doesn't bring thread-safety to block_copy_task_create(): The simplest solution here seems to protect

Re: [PATCH v7 31/31] gitlab: add python linters to CI

2021-05-26 Thread Alex Bennée
John Snow writes: > Add a python container that contains just enough juice for us to run the > python > code quality analysis tools. > > Base this container on fedora, because fedora has very convenient > packaging for testing multiple python versions. > > Add two tests: > >

Re: [PATCH v2 4/7] block-copy: add a CoMutex to the BlockCopyTask list

2021-05-26 Thread Paolo Bonzini
On 25/05/21 12:25, Vladimir Sementsov-Ogievskiy wrote: Next, even if we take bitmaps lock in bdrv_dirty_bitmap_next_dirty_area() or around it, it doesn't bring thread-safety to block_copy_task_create(): The simplest solution here seems to protect bdrv_dirty_bitmap_next_dirty_area and also

Re: [PATCH v2 5/7] block-copy: add QemuMutex lock for BlockCopyCallState list

2021-05-26 Thread Paolo Bonzini
On 25/05/21 12:58, Emanuele Giuseppe Esposito wrote: At this point, I would just rename the other lock (tasks_lock) in "lock" or "state_lock", and substitute it in the calls_lock usages of this patch. Depending on how it comes out, I may merge this with the previous patch. Renaming the

Re: [PATCH v2 7/7] block-copy: protect BlockCopyState .method fields

2021-05-26 Thread Paolo Bonzini
On 25/05/21 13:00, Vladimir Sementsov-Ogievskiy wrote: Hmm. OK, let me think: First look at block_copy_do_copy(). It's called only from block_copy_task_entry. block_copy_task_entry() has mutex-critical-section anyway around handling return value. That means that we can simply move s->method

Re: [PATCH v4 04/15] qemu-iotests: add option to attach gdbserver

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 15:48, Paolo Bonzini wrote: On 26/05/21 13:24, Vladimir Sementsov-Ogievskiy wrote: Define -gdb flag and GDB_OPTIONS environment variable Let's use --option notation for new long options Why make a mix of two styles? -- suggests that single-character options like -d and -v can

Re: [PATCH v4 04/15] qemu-iotests: add option to attach gdbserver

2021-05-26 Thread Paolo Bonzini
On 26/05/21 13:24, Vladimir Sementsov-Ogievskiy wrote: Define -gdb flag and GDB_OPTIONS environment variable Let's use --option notation for new long options Why make a mix of two styles? -- suggests that single-character options like -d and -v can be combined, is that the case? if

Re: [PATCH 0/2] vvfat: fix two crashes.

2021-05-26 Thread Programmingkid
> On May 25, 2021, at 12:18 PM, Kevin Wolf wrote: > > Am 24.05.2021 um 18:06 hat Programmingkid geschrieben: > qemu-system-ppc -usb -device usb-storage,drive=fat16 -drive > file=fat:rw:fat-type=16:" folder>",id=fat16,format=raw,if=none > >> On a related topic would you know

Re: [PATCH v4 00/15] qemu_iotests: improve debugging options

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: This series adds the option to attach gdbserver and valgrind to the QEMU binary running in qemu_iotests. It also allows to redirect QEMU binaries output of the python tests to the stdout, instead of a log file. Patches 1-6 introduce the -gdb

Re: [PATCH v4 04/15] qemu-iotests: add option to attach gdbserver

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: Define -gdb flag and GDB_OPTIONS environment variable Let's use --option notation for new long options to python tests to attach a gdbserver to each qemu instance. This patch only adds and parses this flag, it does not yet add the

Re: [PATCH v4 03/15] docs/devel/testing: add debug section to the QEMU iotests chapter

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: Introduce the "Debugging a test case" section, in preparation to the additional flags that will be added in the next patches. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir

Re: [PATCH v4 02/15] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
in subject: s/QEMUQtestmachine/QEMUQtestMachine 20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: Reviewed-by: John Snow Reviewed-by: Max Reitz Signed-off-by: Emanuele Giuseppe Esposito Usually, r-b lines are placed below s-o-b line. Reviewed-by: Vladimir Sementsov-Ogievskiy ---

Re: [PATCH v4 01/15] python: qemu: add timer parameter for qmp.accept socket

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: Alsp add a new _qmp_timer field to the QEMUMachine class. Let's change the default socket timeout to None, so that if a subclass needs to add a timer, it can be done by modifying this private field. At the same time, restore the timer to be

Re: [PATCH v7 31/31] gitlab: add python linters to CI

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 03:24, John Snow wrote: Add a python container that contains just enough juice for us to run the python code quality analysis tools. Base this container on fedora, because fedora has very convenient packaging for testing multiple python versions. Add two tests: check-python-pipenv

Re: [PATCH v7 29/31] python: add .gitignore

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 03:24, John Snow wrote: Ignore *Python* build and package output (build, dist, qemu.egg-info); these files are not created as part of a QEMU build. They are created by running the commands 'python3 setup.py ' when preparing tarballs to upload to e.g. PyPI. Ignore miscellaneous cached

[PATCH 3/3] ui/vdagent: fix clipboard info memory leak in error path

2021-05-26 Thread Stefan Hajnoczi
If the size of a VD_AGENT_CLIPBOARD_GRAB message is invalid we leak info when returning early. Thanks to Coverity for spotting this: *** CID 1453266: Resource leaks (RESOURCE_LEAK) /qemu/ui/vdagent.c: 465 in vdagent_chr_recv_clipboard() 459 info = qemu_clipboard_info_new(>cbpeer,

Re: [PATCH v7 17/31] python: add pylint to pipenv

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 03:24, John Snow wrote: We are specifying >= pylint 2.8.x for several reasons: 1. For setup.cfg support, added in pylint 2.5.x 2. To specify a version that has incompatibly dropped bad-whitespace checks (2.6.x) 3. 2.7.x fixes "unsubscriptable" warnings in Python 3.9 4. 2.8.x adds

[PATCH 2/3] vhost-user-blk-test: fix Coverity mkstemp(2) umask warning

2021-05-26 Thread Stefan Hajnoczi
The Linux man page for mkstemp(3) states: In glibc versions 2.06 and earlier, the file is created with permissions 0666, that is, read and write for all users. This old behavior may be a security risk, especially since other UNIX flavors use 0600, and somebody might overlook this detail

[PATCH 1/3] vhost-user-blk-test: fix Coverity open(2) false positives

2021-05-26 Thread Stefan Hajnoczi
Coverity checks that the file descriptor return value of open(2) is checked and used. Normally this is helpful in identifying real bugs but vhost-user-blk-test opens /dev/null as stdin and stdout after fork. In this case we don't need to look at the return value because these open(2) calls cannot

[PATCH 0/3] vhost-user-blk-test and vdagent Coverity fixes

2021-05-26 Thread Stefan Hajnoczi
This patch series addresses recent Coverity reports. Please see the individual patches for details. Stefan Hajnoczi (3): vhost-user-blk-test: fix Coverity open(2) false positives vhost-user-blk-test: fix Coverity mkstemp(2) umask warning ui/vdagent: fix clipboard info memory leak in error

Re: [PATCH v7 13/31] python: add MANIFEST.in

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 03:24, John Snow wrote: When creating a source or binary distribution via 'python3 setup.py ', the VERSION and PACKAGE.rst files aren't bundled by default. Create a MANIFEST.in file that instructs the build tools to include these so that installation from these files won't fail. This

Re: [PATCH v7 10/31] python: add qemu package installer

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 03:24, John Snow wrote: Add setup.cfg and setup.py, necessary for installing a package via pip. Add a ReST document (PACKAGE.rst) explaining the basics of what this package is for and who to contact for more information. This document will be used as the landing page for the package

Re: [PATCH v7 07/31] python/machine: Trim line length to below 80 chars

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 03:24, John Snow wrote: One more little delinting fix that snuck in. Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir

Re: [PATCH v7 09/31] python: create qemu packages

2021-05-26 Thread Vladimir Sementsov-Ogievskiy
26.05.2021 03:24, John Snow wrote: move python/qemu/*.py to python/qemu/[machine, qmp, utils]/*.py and update import directives across the tree. This is done to create a PEP420 namespace package, in which we may create subpackages. To do this, the namespace directory ("qemu") should not have