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

2017-08-29 Thread Stefan Hajnoczi
On Thu, Aug 24, 2017 at 04:24:42PM +0300, Alberto Garcia wrote: > 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 >

[Qemu-block] [PATCH v3] qemu-iotests: Extend non-shared storage migration test (194)

2017-08-29 Thread Kashyap Chamarthy
This is the follow-up patch that was discussed[*] as part of feedback to qemu-iotest 194. Changes in this patch: - Supply 'job-id' parameter to `drive-mirror` invocation. - Once migration completes, issue QMP `block-job-cancel` command on the source QEMU to gracefully complete

Re: [Qemu-block] [PATCH v3] qemu-iotests: Extend non-shared storage migration test (194)

2017-08-29 Thread Stefan Hajnoczi
On Tue, Aug 29, 2017 at 5:50 PM, Kashyap Chamarthy wrote: > This is the follow-up patch that was discussed[*] as part of feedback to > qemu-iotest 194. > > Changes in this patch: > > - Supply 'job-id' parameter to `drive-mirror` invocation. > > - Once migration completes,

Re: [Qemu-block] [PATCH v2 1/3] nbd-client: avoid read_reply_co entry if send failed

2017-08-29 Thread Eric Blake
On 08/29/2017 07:27 AM, Stefan Hajnoczi wrote: > The following segfault is encountered if the NBD server closes the UNIX > domain socket immediately after negotiation: > > > In the mean time blk_co_preadv() can be called and nbd_coroutine_end() > calls aio_wake() on read_reply_co. At this

[Qemu-block] [PATCH v2 2/9] IDE: Add register hints to tracing

2017-08-29 Thread John Snow
Name the registers for tracing purposes. Signed-off-by: John Snow Reviewed-by: Eric Blake --- hw/ide/core.c | 88 + hw/ide/trace-events | 8 ++--- 2 files changed, 72 insertions(+), 24 deletions(-)

[Qemu-block] [PATCH v2 0/9] IDE: replace printfs with tracing

2017-08-29 Thread John Snow
Wherever possible, replace all printfs with proper tracing. In most places I've tried to do a straight replacement, but forthcoming patches may calibrate the tracing to be a little nicer. For now, it's nice to just remove the all-or-nothing tracing. For V2, I opted to leave in the nearly

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

2017-08-29 Thread Eric Blake
On 08/24/2017 08:24 AM, Alberto Garcia wrote: > 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 > --- >

[Qemu-block] [PATCH v2 7/9] AHCI: Rework IRQ constants

2017-08-29 Thread John Snow
Create a new enum so that we can name the IRQ bits, which will make debugging them a little nicer if we can print them out. Not handled in this patch, but this will make it possible to get a nice debug printf detailing exactly which status bits are set, as it can be multiple at any given time. As

[Qemu-block] [PATCH v2 8/9] AHCI: pretty-print FIS to buffer instead of stderr

2017-08-29 Thread John Snow
The current FIS printing routines dump the FIS to screen. adjust this such that it dumps to buffer instead, then use this ability to have FIS dump mechanisms via trace-events instead of compiled defines. Signed-off-by: John Snow --- hw/ide/ahci.c | 54

[Qemu-block] [PATCH v2 6/9] AHCI: Replace DPRINTF with trace-events

2017-08-29 Thread John Snow
There are a few hangers-on that will be dealt with individually in forthcoming patches. Signed-off-by: John Snow --- hw/ide/ahci.c | 157 +++- hw/ide/trace-events | 49 2 files changed, 117 insertions(+),

[Qemu-block] [PATCH v2 9/9] AHCI: remove DPRINTF macro

2017-08-29 Thread John Snow
Signed-off-by: John Snow --- hw/ide/ahci.c | 9 - 1 file changed, 9 deletions(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 2e75f9b..57bb59d 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -34,17 +34,8 @@ #include "hw/ide/pci.h" #include

Re: [Qemu-block] [Qemu-devel] [PATCH 00/17] nbd client refactoring and fixing

2017-08-29 Thread Eric Blake
On 08/25/2017 05:10 PM, Eric Blake wrote: > On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote: >> A bit more refactoring and fixing before BLOCK_STATUS series. >> I've tried to make individual patches simple enough, so there are >> a lot of them. >> >> Vladimir Sementsov-Ogievskiy (17):

Re: [Qemu-block] [PATCH v2 6/9] AHCI: Replace DPRINTF with trace-events

2017-08-29 Thread Philippe Mathieu-Daudé
On 08/29/2017 05:49 PM, John Snow wrote: There are a few hangers-on that will be dealt with individually in forthcoming patches. Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé --- hw/ide/ahci.c | 157

Re: [Qemu-block] [PATCH v2 9/9] AHCI: remove DPRINTF macro

2017-08-29 Thread Philippe Mathieu-Daudé
On 08/29/2017 05:49 PM, John Snow wrote: Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- hw/ide/ahci.c | 9 - 1 file changed, 9 deletions(-) diff --git a/hw/ide/ahci.c

[Qemu-block] [PATCH v2 4/9] ATAPI: Replace DEBUG_IDE_ATAPI with tracing events

2017-08-29 Thread John Snow
Goodbye, printfs. Hello, fancy printfs. Signed-off-by: John Snow --- hw/ide/atapi.c| 64 +-- hw/ide/trace-events | 15 +++ include/hw/ide/internal.h | 1 - 3 files changed, 38 insertions(+), 42 deletions(-)

[Qemu-block] [PATCH v2 5/9] IDE: replace DEBUG_AIO with trace events

2017-08-29 Thread John Snow
Signed-off-by: John Snow --- hw/ide/atapi.c| 5 + hw/ide/core.c | 17 ++--- hw/ide/trace-events | 3 +++ include/hw/ide/internal.h | 6 -- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/hw/ide/atapi.c

[Qemu-block] [PATCH v2 3/9] IDE: add tracing for data ports

2017-08-29 Thread John Snow
To be used sparingly, but still interesting in the case of small firmwares designed to reproduce bugs in QEMU IDE. Signed-off-by: John Snow --- hw/ide/core.c | 12 +++- hw/ide/trace-events | 5 + 2 files changed, 16 insertions(+), 1 deletion(-) diff --git

Re: [Qemu-block] [Qemu-devel] [PATCH v2 0/9] IDE: replace printfs with tracing

2017-08-29 Thread no-reply
Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20170829204934.9039-1-js...@redhat.com Subject: [Qemu-devel] [PATCH v2 0/9] IDE: replace printfs with tracing Type: series === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1

Re: [Qemu-block] [PATCH v2 3/9] IDE: add tracing for data ports

2017-08-29 Thread Philippe Mathieu-Daudé
On 08/29/2017 05:49 PM, John Snow wrote: To be used sparingly, but still interesting in the case of small firmwares designed to reproduce bugs in QEMU IDE. Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé --- hw/ide/core.c | 12

[Qemu-block] [PATCH v2] qemu-iotests: Extend non-shared storage migration test (194)

2017-08-29 Thread Kashyap Chamarthy
This is the follow-up patch that was discussed[*] as part of feedback to qemu-iotest 194. Changes in this patch: - Supply 'job-id' parameter to `drive-mirror` invocation. - Issue QMP `block-job-cancel` command on the source QEMU to gracefully complete `drive-mirror` operation. - Stop

Re: [Qemu-block] [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

2017-08-29 Thread Liu Qing
On Mon, Aug 28, 2017 at 10:46:34AM -0500, Eric Blake wrote: > [adding qemu-block] > > On 08/28/2017 12:56 AM, Liu Qing wrote: > > Dear list, > > Recently I used fio to test qcow2 driver in the guest os, and found out > > that when a new cluster is allocated the 4K IO will occupy 64K(default

Re: [Qemu-block] [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

2017-08-29 Thread Liu Qing
On Mon, Aug 28, 2017 at 05:40:48PM -0400, John Snow wrote: > > > On 08/28/2017 01:56 AM, Liu Qing wrote: > > Dear list, > > Recently I used fio to test qcow2 driver in the guest os, and found out > > that when a new cluster is allocated the 4K IO will occupy 64K(default > > cluster > >

Re: [Qemu-block] [PATCH v4 03/15] block: introduce BDRV_REQ_ALLOCATE flag

2017-08-29 Thread Alberto Garcia
On Tue 01 Aug 2017 04:19:00 PM CEST, Anton Nefedov wrote: > The flag is supposed to indicate that the region of the disk image has > to be sufficiently allocated so it reads as zeroes. The call with the flag > set has to return -ENOTSUP if allocation cannot be done efficiently > (i.e. without

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

2017-08-29 Thread Yaniv Lavi (Dary)
YANIV LAVI (YANIV DARY) SENIOR TECHNICAL PRODUCT MANAGER Red Hat Israel Ltd. 34 Jerusalem Road, Building A, 1st floor Ra'anana, Israel 4350109 yl...@redhat.comT: +972-9-7692306/8272306 F: +972-9-7692223IM: ylavi TRIED. TESTED.

[Qemu-block] [PATCH v2 1/3] nbd-client: avoid read_reply_co entry if send failed

2017-08-29 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

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

2017-08-29 Thread Stefan Hajnoczi
v2: * Rewrote Patch 1 following Paolo's suggestion [Paolo] 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: avoid

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

2017-08-29 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

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

2017-08-29 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

Re: [Qemu-block] [PATCH v4 05/15] file-posix: support BDRV_REQ_ALLOCATE

2017-08-29 Thread Alberto Garcia
On Tue 01 Aug 2017 04:19:02 PM CEST, Anton Nefedov wrote: > Current write_zeroes implementation is good enough to satisfy this flag too > > Signed-off-by: Anton Nefedov Reviewed-by: Alberto Garcia Berto

Re: [Qemu-block] [Qemu-devel] [PATCH] misc: Remove unused Error variables

2017-08-29 Thread Philippe Mathieu-Daudé
Hi Alberto, On 08/29/2017 09:08 AM, Alberto Garcia wrote: There's a few cases which we're passing an Error pointer to a function only to discard it immediately afterwards without checking it. In these cases we can simply remove the variable and pass NULL instead. How did you notice?

Re: [Qemu-block] [PATCH v4 06/15] block: support BDRV_REQ_ALLOCATE in passthrough drivers

2017-08-29 Thread Alberto Garcia
On Tue 01 Aug 2017 04:19:03 PM CEST, Anton Nefedov wrote: > Support the flag if the underlying BDS supports it > > Signed-off-by: Anton Nefedov Reviewed-by: Alberto Garcia Berto

Re: [Qemu-block] [Qemu-devel] [PATCH] misc: Remove unused Error variables

2017-08-29 Thread Alberto Garcia
On Tue 29 Aug 2017 02:31:35 PM CEST, Philippe Mathieu-Daudé wrote: >> There's a few cases which we're passing an Error pointer to a function >> only to discard it immediately afterwards without checking it. In >> these cases we can simply remove the variable and pass NULL instead. > > How did you

Re: [Qemu-block] [PATCH v2] qemu-iotests: Extend non-shared storage migration test (194)

2017-08-29 Thread Stefan Hajnoczi
On Tue, Aug 29, 2017 at 11:06:22AM +0200, Kashyap Chamarthy wrote: > This is the follow-up patch that was discussed[*] as part of feedback to > qemu-iotest 194. > > Changes in this patch: > > - Supply 'job-id' parameter to `drive-mirror` invocation. > > - Issue QMP `block-job-cancel`

[Qemu-block] [PATCH] misc: Remove unused Error variables

2017-08-29 Thread Alberto Garcia
There's a few cases which we're passing an Error pointer to a function only to discard it immediately afterwards without checking it. In these cases we can simply remove the variable and pass NULL instead. Signed-off-by: Alberto Garcia --- block/qcow.c | 12 +++-

Re: [Qemu-block] [PATCH v2 0/7] Refactor DMG driver to have chunk size independence

2017-08-29 Thread Stefan Hajnoczi
On Sun, Aug 20, 2017 at 1:47 PM, Ashijeet Acharya wrote: > On Fri, May 5, 2017 at 7:29 PM, Stefan Hajnoczi wrote: >> >> On Thu, Apr 27, 2017 at 01:36:30PM +0530, Ashijeet Acharya wrote: >> > This series helps to provide chunk size independence for

Re: [Qemu-block] [Qemu-devel] [PATCH] misc: Remove unused Error variables

2017-08-29 Thread Eric Blake
On 08/29/2017 07:08 AM, Alberto Garcia wrote: > There's a few cases which we're passing an Error pointer to a function > only to discard it immediately afterwards without checking it. In > these cases we can simply remove the variable and pass NULL instead. Could also go in via qemu-trivial or

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

2017-08-29 Thread Eric Blake
On 08/28/2017 08:18 PM, John Snow wrote: >>> We'd have to develop a new syntax for specifying these resources that >>> can be stored in a qcow2 file, >> >> It's called the json-pseudo-protocol and was developed exactly for this. >> > > That's what I was hinting at for "or otherwise co-opt an

Re: [Qemu-block] [Qemu-devel] [PATCH] misc: Remove unused Error variables

2017-08-29 Thread Alberto Garcia
On Tue 29 Aug 2017 04:40:35 PM CEST, Eric Blake wrote: >> There's a few cases which we're passing an Error pointer to a Hmmm... this was "few cases in which", but it seems that I accidentally removed the "in". Not very important, but whoever commits this feel free to fix the message. Berto

Re: [Qemu-block] [PATCH v2] qemu-iotests: Extend non-shared storage migration test (194)

2017-08-29 Thread Kashyap Chamarthy
On Tue, Aug 29, 2017 at 03:19:54PM +0100, Stefan Hajnoczi wrote: > On Tue, Aug 29, 2017 at 11:06:22AM +0200, Kashyap Chamarthy wrote: [...] > > +iotests.log('Gracefully ending the `drive-mirror` job on source...') > > +iotests.log(source_vm.qmp('block-job-cancel', device='mirror-job0')) > > Two

Re: [Qemu-block] [Qemu-devel] [PATCH 7/9] AHCI: Rework IRQ constants

2017-08-29 Thread Philippe Mathieu-Daudé
On 08/08/2017 03:33 PM, John Snow wrote: Create a new enum so that we can name the IRQ bits, which will make debugging them a little nicer if we can print them out. Not handled in this patch, but this will make it possible to get a nice debug printf detailing exactly which status bits are set,

Re: [Qemu-block] [PATCH v2 5/9] IDE: replace DEBUG_AIO with trace events

2017-08-29 Thread Philippe Mathieu-Daudé
Hi John, On 08/29/2017 05:49 PM, John Snow wrote: Signed-off-by: John Snow --- hw/ide/atapi.c| 5 + hw/ide/core.c | 17 ++--- hw/ide/trace-events | 3 +++ include/hw/ide/internal.h | 6 -- 4 files changed, 18

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

2017-08-29 Thread Eric Blake
On 08/29/2017 07:27 AM, Stefan Hajnoczi wrote: > v2: > * Rewrote Patch 1 following Paolo's suggestion [Paolo] > > 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

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

2017-08-29 Thread John Snow
On 08/29/2017 10:30 AM, Eric Blake wrote: > On 08/28/2017 08:18 PM, John Snow wrote: We'd have to develop a new syntax for specifying these resources that can be stored in a qcow2 file, >>> >>> It's called the json-pseudo-protocol and was developed exactly for this. >>> >> >> That's

Re: [Qemu-block] [Qemu-devel] [PATCH v3] qemu-iotests: Extend non-shared storage migration test (194)

2017-08-29 Thread Eric Blake
On 08/29/2017 12:42 PM, Stefan Hajnoczi wrote: > On Tue, Aug 29, 2017 at 5:50 PM, Kashyap Chamarthy > wrote: >> This is the follow-up patch that was discussed[*] as part of feedback to >> qemu-iotest 194. >> >> Signed-off-by: Kashyap Chamarthy >> ---

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

2017-08-29 Thread Eric Blake
On 08/24/2017 08:24 AM, Alberto Garcia wrote: > 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

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

2017-08-29 Thread Eric Blake
On 08/24/2017 08:24 AM, Alberto Garcia wrote: > 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

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

2017-08-29 Thread Eric Blake
On 08/24/2017 08:24 AM, Alberto Garcia wrote: > 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 >

[Qemu-block] [PATCH v2 1/9] IDE: replace DEBUG_IDE with tracing system

2017-08-29 Thread John Snow
Remove the DEBUG_IDE preprocessor definition with something more appropriately flexible, using the trace-events subsystem. This will be less prone to bitrot and will more effectively allow us to target just the functions we care about. Signed-off-by: John Snow Reviewed-by:

Re: [Qemu-block] [Qemu-devel] [PATCH 6/9] AHCI: Replace DPRINTF with trace-events

2017-08-29 Thread Philippe Mathieu-Daudé
[...] +# hw/ide/ahci.c +ahci_port_read(void *s, int port, int offset, uint32_t ret) "ahci(%p)[%d]: port read @ 0x%x: 0x%08x" +ahci_irq_raise(void *s) "ahci(%p): raise irq" +ahci_irq_lower(void *s) "ahci(%p): lower irq" +ahci_check_irq(void *s, uint32_t old, uint32_t new) "ahci(%p): check irq

Re: [Qemu-block] [PATCH v2 7/9] AHCI: Rework IRQ constants

2017-08-29 Thread Philippe Mathieu-Daudé
On 08/29/2017 05:49 PM, John Snow wrote: Create a new enum so that we can name the IRQ bits, which will make debugging them a little nicer if we can print them out. Not handled in this patch, but this will make it possible to get a nice debug printf detailing exactly which status bits are set,