Re: [Qemu-block] [Qemu-devel] Persistent bitmaps for non-qcow2 formats

2017-08-24 Thread John Snow
Sorry in advance for :words: ... On 08/23/2017 02:04 PM, Vladimir Sementsov-Ogievskiy wrote: > 23.08.2017 11:59, Vladimir Sementsov-Ogievskiy wrote: >> 22.08.2017 22:07, John Snow wrote: >>> Well, we knew we'd want this sooner or later. I've got some pings >>> downstream over whether or not we sup

Re: [Qemu-block] [Qemu-devel] [PATCH v7 6/6] qemu-iotests: add 184 for throttle filter driver

2017-08-24 Thread Manos Pitsidianakis
On Thu, Aug 24, 2017 at 07:43:08PM +0100, Stefan Hajnoczi wrote: On Tue, Aug 22, 2017 at 01:15:35PM +0300, Manos Pitsidianakis wrote: +_supported_fmt qcow2 What makes this test qcow2-specific? With additional filtering for IMGFMT it should be possible to run this on any image format. I thin

Re: [Qemu-block] [Qemu-devel] [PATCH v7 6/6] qemu-iotests: add 184 for throttle filter driver

2017-08-24 Thread Stefan Hajnoczi
On Tue, Aug 22, 2017 at 01:15:35PM +0300, Manos Pitsidianakis wrote: > +_supported_fmt qcow2 What makes this test qcow2-specific? With additional filtering for IMGFMT it should be possible to run this on any image format.

Re: [Qemu-block] [PATCH v7 5/6] block: add throttle block filter driver

2017-08-24 Thread Stefan Hajnoczi
On Tue, Aug 22, 2017 at 01:15:34PM +0300, Manos Pitsidianakis wrote: > @@ -3095,6 +3096,20 @@ > '*tls-creds': 'str' } } > > ## > +# @BlockdevOptionsThrottle: > +# > +# Driver specific block device options for the throttle driver > +# > +# @throttle-group: the name of the throttle-

Re: [Qemu-block] [PATCH v7 4/6] block: convert ThrottleGroup to object with QOM

2017-08-24 Thread Stefan Hajnoczi
On Tue, Aug 22, 2017 at 01:15:33PM +0300, Manos Pitsidianakis wrote: > /* Increments the reference count of a ThrottleGroup given its name. > * > * If no ThrottleGroup is found with the given name a new one is > * created. > * > + * This function edits throttle_groups and must be called un

Re: [Qemu-block] [PATCH 3/3] qemu-iotests: test NBD over UNIX domain sockets in 083

2017-08-24 Thread Eric Blake
On 08/24/2017 10:33 AM, Stefan Hajnoczi wrote: > 083 only tests TCP. Some failures might be specific to UNIX domain > sockets. > > A few adjustments are necessary: > > 1. Generating a port number and waiting for server startup is >TCP-specific. Use the new nbd-fault-injector.py startup prot

Re: [Qemu-block] [PATCH 1/3] nbd-client: enter read_reply_co during init to avoid crash

2017-08-24 Thread Paolo Bonzini
On 24/08/2017 19:37, Eric Blake wrote: > On 08/24/2017 11:21 AM, Paolo Bonzini wrote: >> On 24/08/2017 17:33, Stefan Hajnoczi wrote: >>> This patch enters read_reply_co directly in >>> nbd_client_attach_aio_context(). This is safe because new_context is >>> acquired by the caller. This ensures th

Re: [Qemu-block] [PATCH 2/3] qemu-iotests: improve nbd-fault-injector.py startup protocol

2017-08-24 Thread Eric Blake
On 08/24/2017 10:33 AM, Stefan Hajnoczi wrote: > Currently 083 waits for the nbd-fault-injector.py server to start up by > looping until netstat shows the TCP listen socket. > > The startup protocol can be simplified by passing a 0 port number to > nbd-fault-injector.py. The kernel will allocate

Re: [Qemu-block] [PATCH 1/3] nbd-client: enter read_reply_co during init to avoid crash

2017-08-24 Thread Eric Blake
On 08/24/2017 11:21 AM, Paolo Bonzini wrote: > On 24/08/2017 17:33, Stefan Hajnoczi wrote: >> This patch enters read_reply_co directly in >> nbd_client_attach_aio_context(). This is safe because new_context is >> acquired by the caller. This ensures that read_reply_co reaches its >> first yield p

Re: [Qemu-block] [PATCH 1/3] nbd-client: enter read_reply_co during init to avoid crash

2017-08-24 Thread Paolo Bonzini
On 24/08/2017 17:33, Stefan Hajnoczi wrote: > This patch enters read_reply_co directly in > nbd_client_attach_aio_context(). This is safe because new_context is > acquired by the caller. This ensures that read_reply_co reaches its > first yield point and its ctx is set up. I'm not very confident

Re: [Qemu-block] [PATCH 0/3] nbd-client: enter read_reply_co during init to avoid crash

2017-08-24 Thread Stefan Hajnoczi
On Thu, Aug 24, 2017 at 4:52 PM, Eric Blake wrote: > On 08/24/2017 10:33 AM, Stefan Hajnoczi wrote: >> See Patch 1 for the segfault fix. Patches 2 & 3 add qemu-iotests coverage. >> >> This is a rare crash that we'll probably only see in testing. It only seems >> to >> happen with UNIX domain so

Re: [Qemu-block] [PATCH] qemu-iotests: add 194 non-shared storage migration test

2017-08-24 Thread Eric Blake
On 08/23/2017 09:05 AM, Stefan Hajnoczi wrote: > Non-shared storage migration with NBD and drive-mirror is currently not > tested by qemu-iotests. This test case covers the basic migration > scenario. > > Signed-off-by: Stefan Hajnoczi > Based-on: <20170823134242.12080-1-f...@redhat.com> > --- >

Re: [Qemu-block] [PATCH 0/3] nbd-client: enter read_reply_co during init to avoid crash

2017-08-24 Thread Eric Blake
On 08/24/2017 10:33 AM, Stefan Hajnoczi wrote: > See Patch 1 for the segfault fix. Patches 2 & 3 add qemu-iotests coverage. > > This is a rare crash that we'll probably only see in testing. It only seems > to > happen with UNIX domain sockets. Rare enough that I don't think it is 2.10-rc4 mate

Re: [Qemu-block] [Qemu-devel] [PATCH 06/10] scsi, file-posix: add support for persistent reservation management

2017-08-24 Thread Paolo Bonzini
On 24/08/2017 17:37, Eric Blake wrote: >> # >> # @filename:path to the image file >> +# @pr-manager: the if for the object that will handle persistent >> reservations > s/if/interface/ for legibility > "id" actually. Paolo signature.asc Description: OpenPGP digital signature

Re: [Qemu-block] [Qemu-devel] [PATCH 08/10] scsi: build qemu-pr-helper

2017-08-24 Thread Eric Blake
On 08/22/2017 08:18 AM, Paolo Bonzini wrote: > Introduce a privileged helper to run persistent reservation commands. > This lets virtual machines send persistent reservations without using > CAP_SYS_RAWIO or out-of-tree patches. The helper uses Unix permissions > and SCM_RIGHTS to restrict access

Re: [Qemu-block] [Qemu-devel] [PATCH 06/10] scsi, file-posix: add support for persistent reservation management

2017-08-24 Thread Eric Blake
On 08/22/2017 08:18 AM, Paolo Bonzini wrote: > It is a common requirement for virtual machine to send persistent > reservations, but this currently requires either running QEMU with > CAP_SYS_RAWIO, or using out-of-tree patches that let an unprivileged > QEMU bypass Linux's filter on SG_IO commands

[Qemu-block] [PATCH 3/3] qemu-iotests: test NBD over UNIX domain sockets in 083

2017-08-24 Thread Stefan Hajnoczi
083 only tests TCP. Some failures might be specific to UNIX domain sockets. A few adjustments are necessary: 1. Generating a port number and waiting for server startup is TCP-specific. Use the new nbd-fault-injector.py startup protocol to fetch the address. This is a little more elegant

[Qemu-block] [PATCH 2/3] qemu-iotests: improve nbd-fault-injector.py startup protocol

2017-08-24 Thread Stefan Hajnoczi
Currently 083 waits for the nbd-fault-injector.py server to start up by looping until netstat shows the TCP listen socket. The startup protocol can be simplified by passing a 0 port number to nbd-fault-injector.py. The kernel will allocate a port in bind(2) and the final port number can be printe

[Qemu-block] [PATCH 1/3] nbd-client: enter read_reply_co during init to avoid crash

2017-08-24 Thread Stefan Hajnoczi
The following segfault is encountered if the NBD server closes the UNIX domain socket immediately after negotiation: Program terminated with signal SIGSEGV, Segmentation fault. #0 aio_co_schedule (ctx=0x0, co=0xd3c0ff2ef0) at util/async.c:441 441 QSLIST_INSERT_HEAD_ATOMIC(&ctx->schedu

[Qemu-block] [PATCH 0/3] nbd-client: enter read_reply_co during init to avoid crash

2017-08-24 Thread Stefan Hajnoczi
See Patch 1 for the segfault fix. Patches 2 & 3 add qemu-iotests coverage. This is a rare crash that we'll probably only see in testing. It only seems to happen with UNIX domain sockets. Stefan Hajnoczi (3): nbd-client: enter read_reply_co during init to avoid crash qemu-iotests: improve nb

[Qemu-block] [PATCH v2 0/7] Misc throttle fixes

2017-08-24 Thread Alberto Garcia
Hi, here's the new version of my patch series with misc fixes for the throttling code. Stefan, once this is reviewed, can you please remove the "Make LeakyBucket.avg and LeakyBucket.max integer types" commit (id 218f470639117a8e39) from your block-next branch? This series contains a new version o

[Qemu-block] [PATCH v2 6/7] throttle: Make burst_length 64bit and add range checks

2017-08-24 Thread Alberto Garcia
LeakyBucket.burst_length is defined as an unsigned integer but the code never checks for overflows and it only makes sure that the value is not 0. In practice this means that the user can set something like throttling.iops-total-max-length=4294967300 despite being larger than UINT_MAX and the fina

[Qemu-block] [PATCH v2 2/7] throttle: Update the throttle_fix_bucket() documentation

2017-08-24 Thread Alberto Garcia
The way the throttling algorithm works is that requests start being throttled once the bucket level exceeds the burst limit. When we get there the bucket leaks at the level set by the user (bkt->avg), and that leak rate is what prevents guest I/O from exceeding the desired limit. If we don't allow

[Qemu-block] [PATCH v2 4/7] throttle: Remove throttle_fix_bucket() / throttle_unfix_bucket()

2017-08-24 Thread Alberto Garcia
The throttling code can change internally the value of bkt->max if it hasn't been set by the user. The problem with this is that if we want to retrieve the original value we have to undo this change first. This is ugly and unnecessary: this patch removes the throttle_fix_bucket() and throttle_unfix

[Qemu-block] [PATCH v2 7/7] throttle: Test the valid range of config values

2017-08-24 Thread Alberto Garcia
Signed-off-by: Alberto Garcia --- tests/test-throttle.c | 77 +++ 1 file changed, 77 insertions(+) diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 41c0dd2529..bf7a5a648a 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.

[Qemu-block] [PATCH v2 5/7] throttle: Make LeakyBucket.avg and LeakyBucket.max integer types

2017-08-24 Thread Alberto Garcia
Both the throttling limits set with the throttling.iops-* and throttling.bps-* options and their QMP equivalents defined in the BlockIOThrottle struct are integer values. Those limits are also reported in the BlockDeviceInfo struct and they are integers there as well. Therefore there's no reason

[Qemu-block] [PATCH v2 1/7] throttle: Fix wrong variable name in the header documentation

2017-08-24 Thread Alberto Garcia
The level of the burst bucket is stored in bkt.burst_level, not bkt.burst_length. Signed-off-by: Alberto Garcia Reviewed-by: Manos Pitsidianakis --- include/qemu/throttle.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h inde

[Qemu-block] [PATCH v2 3/7] throttle: Make throttle_is_valid() a bit less verbose

2017-08-24 Thread Alberto Garcia
Use a pointer to the bucket instead of repeating cfg->buckets[i] all the time. This makes the code more concise and will help us expand the checks later and save a few line breaks. Signed-off-by: Alberto Garcia --- util/throttle.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletion

[Qemu-block] [PATCH v8 6/6] qemu-iotests: add 184 for throttle filter driver

2017-08-24 Thread Manos Pitsidianakis
Reviewed-by: Alberto Garcia Signed-off-by: Manos Pitsidianakis --- tests/qemu-iotests/184 | 205 +++ tests/qemu-iotests/184.out | 300 + tests/qemu-iotests/group | 1 + 3 files changed, 506 insertions(+) create mode

Re: [Qemu-block] [Qemu-devel] [PATCH v5 08/13] tests: Rely more on global_qtest

2017-08-24 Thread Markus Armbruster
Paolo Bonzini writes: > On 24/08/2017 12:09, Markus Armbruster wrote: >> Cut-and-paste cuts both ways (pardon the pun): >> >> initialize with QTestState A >> frobnicate with QTestState A >> glomnify with QTestState A >> frobnicate with QTestState A >> initialize with QTestSta

[Qemu-block] [PATCH v8 2/6] block: add aio_context field in ThrottleGroupMember

2017-08-24 Thread Manos Pitsidianakis
timer_cb() needs to know about the current Aio context of the throttle request that is woken up. In order to make ThrottleGroupMember backend agnostic, this information is stored in an aio_context field instead of accessing it from BlockBackend. Reviewed-by: Alberto Garcia Reviewed-by: Stefan Haj

[Qemu-block] [PATCH v8 0/6] add throttle block driver filter

2017-08-24 Thread Manos Pitsidianakis
This series adds a throttle block driver filter. Currently throttling is done at the BlockBackend level. Using block driver interfaces we can move the throttling to any point in the BDS graph using a throttle node which uses the existing throttling code. This allows for potentially more complex con

[Qemu-block] [PATCH v8 4/6] block: convert ThrottleGroup to object with QOM

2017-08-24 Thread Manos Pitsidianakis
ThrottleGroup is converted to an object. This will allow the future throttle block filter drive easy creation and configuration of throttle groups in QMP and cli. A new QAPI struct, ThrottleLimits, is introduced to provide a shared struct for all throttle configuration needs in QMP. ThrottleGroup

[Qemu-block] [PATCH v8 5/6] block: add throttle block filter driver

2017-08-24 Thread Manos Pitsidianakis
block/throttle.c uses existing I/O throttle infrastructure inside a block filter driver. I/O operations are intercepted in the filter's read/write coroutines, and referred to block/throttle-groups.c The driver can be used with the syntax -drive driver=throttle,file.filename=foo.qcow2,throttle-grou

[Qemu-block] [PATCH v8 3/6] block: tidy ThrottleGroupMember initializations

2017-08-24 Thread Manos Pitsidianakis
Move the CoMutex and CoQueue inits inside throttle_group_register_tgm() which is called whenever a ThrottleGroupMember is initialized. There's no need for them to be separate. Reviewed-by: Alberto Garcia Reviewed-by: Stefan Hajnoczi Signed-off-by: Manos Pitsidianakis --- block/block-backend.c

[Qemu-block] [PATCH v8 1/6] block: move ThrottleGroup membership to ThrottleGroupMember

2017-08-24 Thread Manos Pitsidianakis
This commit eliminates the 1:1 relationship between BlockBackend and throttle group state. Users will be able to create multiple throttle nodes, each with its own throttle group state, in the future. The throttle group state cannot be per-BlockBackend anymore, it must be per-throttle node. This i

Re: [Qemu-block] [Qemu-devel] [PATCH v2 06/54] qapi: introduce qapi_enum_lookup()

2017-08-24 Thread Dr. David Alan Gilbert
* Marc-André Lureau (marcandre.lur...@redhat.com) wrote: > Hi > > - Original Message - > > Marc-André Lureau writes: > > > > > This will help with the introduction of a new structure to handle > > > enum lookup. It would be good to make that comment explain why it's necessary. > > > Si

Re: [Qemu-block] [Qemu-devel] [PATCH v5 08/13] tests: Rely more on global_qtest

2017-08-24 Thread Paolo Bonzini
On 24/08/2017 12:09, Markus Armbruster wrote: > Cut-and-paste cuts both ways (pardon the pun): > > initialize with QTestState A > frobnicate with QTestState A > glomnify with QTestState A > frobnicate with QTestState A > initialize with QTestState B > boingboing with QTestS

Re: [Qemu-block] [Qemu-devel] [PATCH v5 08/13] tests: Rely more on global_qtest

2017-08-24 Thread Markus Armbruster
Paolo Bonzini writes: > On 24/08/2017 09:42, Markus Armbruster wrote: >> >> In a language less primitive than C, I'd write it exactly that way, and >> nobody would complain. In old, primitive C, I have to write >> >> global_qtest = A; >> do this >> do that >> >> global_qtest =

Re: [Qemu-block] [Qemu-devel] [PATCH v5 08/13] tests: Rely more on global_qtest

2017-08-24 Thread Paolo Bonzini
On 24/08/2017 09:42, Markus Armbruster wrote: > > In a language less primitive than C, I'd write it exactly that way, and > nobody would complain. In old, primitive C, I have to write > > global_qtest = A; > do this > do that > > global_qtest = B; > do something > > glo

[Qemu-block] [PATCH 07/16] block: Use qemu_enum_parse() in blkdebug_debug_breakpoint()

2017-08-24 Thread Markus Armbruster
From: Marc-André Lureau The error message on invalid blkdebug events changes from qemu-system-x86_64: LOCATION: Invalid event name "VALUE" to qemu-system-x86_64: LOCATION: invalid parameter value: VALUE Slight degradation, but the message is sub-par even before the patch. When complai

Re: [Qemu-block] [Qemu-devel] [PATCH v5 08/13] tests: Rely more on global_qtest

2017-08-24 Thread Markus Armbruster
Paolo Bonzini writes: > On 23/08/2017 23:30, Eric Blake wrote: >>> Well, whatever is assigning to global_qtest should be using the long form. >> Question - what about going the other way, and switching ALL callers to >> always use the explicit form? I'd really like to maintain only one >> form,

Re: [Qemu-block] [Qemu-devel] [RFC PATCH 31/56] block: Make throttle byte rates and sizes unsigned in QAPI/QMP

2017-08-24 Thread Markus Armbruster
Alberto Garcia writes: > On Mon 07 Aug 2017 04:45:35 PM CEST, Markus Armbruster wrote: >> Sizes and byte rates should use QAPI type 'size' (uint64_t). >> BlockIOThrottle and BlockDeviceInfo members @bps, @bps_rd, @bps_wr, >> @bps_max, @bps_rd_max, @bps_wr_max, @iops_size are 'int' (int64_t). >> q