Re: [Qemu-block] [PATCH v4 1/2] live-block-ops.txt: Rename, rewrite, and improve it

On Wed 28 Jun 2017 04:58:00 PM CEST, Kashyap Chamarthy wrote: > +Once a 'mirror' job has started, there are two possible actions when a > +``drive-mirror`` job is active: > + > +1. Issuing the command ``block-job-cancel``: will, after completing > + synchronization of the content from the disk

Re: [Qemu-block] [PATCH v5 02/25] block: Use children list in bdrv_refresh_filename

ff-by: Max Reitz <mre...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH RFC v3 6/8] block: add options parameter to bdrv_new_open_driver()

On Fri 23 Jun 2017 02:46:58 PM CEST, Manos Pitsidianakis wrote: > BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char > *node_name, > - int flags, Error **errp) > + int flags, QDict *options, Error >

Re: [Qemu-block] proposed qcow2 extension: cluster reservations [was: [Qemu-devel] [RFC] Proposed qcow2 extension: subcluster allocation

On Sat 22 Apr 2017 07:56:57 PM CEST, Max Reitz wrote: >> So, if you got this far in reading, the question becomes whether >> having a mode where you can mark a cluster as >> mapping-reserved-but-unallocated has enough use case to be worth >> pursuing, knowing that it will burn an incompatible

Re: [Qemu-block] qemu-io aborts at quit after reopen

On Tue, Apr 25, 2017 at 11:13:17PM +0800, Fam Zheng wrote: > Hi Kevin, > > This happens both on master and on your block-next tree: > > $ qemu-io -f raw null-co:// -c 'reopen -r' > Unexpected error in bdrv_check_perm() at /stor/work/qemu/block.c:1437: > Block node is read-only >

Re: [Qemu-block] [PATCH v3 5/7] block: add throttle block filter driver

On Wed 09 Aug 2017 11:36:12 AM CEST, Manos Pitsidianakis wrote: > On Tue, Aug 08, 2017 at 05:04:48PM +0200, Alberto Garcia wrote: >>On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote: >>>>> So basically if we have anonymous groups, we accept limits in the >&

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

On Wed 02 Aug 2017 12:57:04 PM CEST, Manos Pitsidianakis wrote: >> At the moment I think throttle_groups_lock isn't strictly needed >> because incref/decref callers hold the QEMU global mutex anyway. >> >> But code accessing throttle_groups still has to be disciplined. >> Since

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] throttle: Make LeakyBucket.avg and LeakyBucket.max integer types

On Tue 08 Aug 2017 12:00:30 PM CEST, Stefan Hajnoczi wrote: > On Mon, Aug 07, 2017 at 07:15:29PM +0300, Alberto Garcia wrote: >> Both the throttling limits set with the throttling.iops-* and >> throttling.bps-* options and their QMP equivalents defined in the >> BlockIOThrott

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] throttle: Make LeakyBucket.avg and LeakyBucket.max integer types

On Tue 08 Aug 2017 12:17:12 PM CEST, Alberto Garcia wrote: > I was under the impression that Markus wanted to change the QAPI types > of the throttling fields in BlockDeviceInfo for 2.10 as well, so this > patch is relevant. I just saw that his series is still an RFC, so we can leave t

Re: [Qemu-block] [PATCH v3 5/7] block: add throttle block filter driver

On Tue 08 Aug 2017 03:45:44 PM CEST, Manos Pitsidianakis wrote: > On Tue, Aug 08, 2017 at 03:13:36PM +0200, Alberto Garcia wrote: >>On Mon 31 Jul 2017 11:54:41 AM CEST, Manos Pitsidianakis wrote: >>> block/throttle.c uses existing I/O throttle infrastructure inside a >>&g

Re: [Qemu-block] [PATCH v3 5/7] block: add throttle block filter driver

On Tue 08 Aug 2017 04:56:20 PM CEST, Manos Pitsidianakis wrote: >>> So basically if we have anonymous groups, we accept limits in the >>> driver options but only without a group-name. >> >>In the commit message you do however have limits and a group name, is >>that a mistake? >> >>-drive

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] throttle: Make LeakyBucket.avg and LeakyBucket.max integer types

On Tue 08 Aug 2017 05:11:27 PM CEST, Eric Blake wrote: >> Why is this marked for-2.10? Does it fix a bug? > > Theoretically, converting between int64_t and double loses precision > on any values larger than 2^53. In all practicality, though, if you > expect throttling to be precise through 2^53

Re: [Qemu-block] [PATCH v3 5/7] block: add throttle block filter driver

On Wed 09 Aug 2017 03:42:07 PM CEST, Manos Pitsidianakis wrote: > On Wed, Aug 09, 2017 at 02:36:20PM +0200, Alberto Garcia wrote: >>On Wed 09 Aug 2017 11:36:12 AM CEST, Manos Pitsidianakis wrote: >>> On Tue, Aug 08, 2017 at 05:04:48PM +0200, Alberto Garcia wrote: >>>>

[Qemu-block] [PATCH for-2.10] quorum: Set sectors-count to 0 when reporting a flush error

in all cases. Reported-by: Markus Armbruster <arm...@redhat.com> Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/quorum.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/block/quorum.c b/block/quorum.c index 55ba916655..d04da4f430 100644 --- a/block/qu

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] quorum: Handle bdrv_getlength() failures in quorum_co_flush()

On Mon 07 Aug 2017 01:29:09 PM CEST, Eric Blake wrote: > On 08/07/2017 03:43 AM, Alberto Garcia wrote: >> On Fri 04 Aug 2017 05:44:00 PM CEST, Eric Blake wrote: >>>> --- a/block/quorum.c >>>> +++ b/block/quorum.c >>>> @@ -785,8 +785,9 @@

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10] quorum: Handle bdrv_getlength() failures in quorum_co_flush()

On Fri 04 Aug 2017 05:44:00 PM CEST, Eric Blake wrote: >> --- a/block/quorum.c >> +++ b/block/quorum.c >> @@ -785,8 +785,9 @@ static coroutine_fn int quorum_co_flush(BlockDriverState >> *bs) >> for (i = 0; i < s->num_children; i++) { >> result = bdrv_co_flush(s->children[i]->bs); >>

[Qemu-block] [PATCH] quorum: Handle bdrv_getlength() failures in quorum_co_flush()

A bdrv_getlength() call can fail and return a negative value. This is not being handled in quorum_co_flush(), which can result in a QUORUM_REPORT_BAD event with an arbitrary value on the 'sectors-count' field. Reported-by: Markus Armbruster <arm...@redhat.com> Signed-off-by: Alberto

Re: [Qemu-block] Is the use of bdrv_getlength() in quorum_co_flush() kosher?

On Fri 04 Aug 2017 02:48:03 PM CEST, Markus Armbruster wrote: > Have a look at quorum_co_flush(): > > quorum_report_bad(QUORUM_OP_TYPE_FLUSH, 0, > bdrv_getlength(s->children[i]->bs), > s->children[i]->bs->node_name, result); >

Re: [Qemu-block] [PATCH v3 1/7] block: move ThrottleGroup membership to ThrottleGroupMember

fa...@redhat.com> > Signed-off-by: Manos Pitsidianakis <el13...@mail.ntua.gr> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH v3 3/7] block: tidy ThrottleGroupMember initializations

Hajnoczi <stefa...@redhat.com> > Signed-off-by: Manos Pitsidianakis <el13...@mail.ntua.gr> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

[Qemu-block] [PATCH for-2.10] throttle: Make LeakyBucket.avg and LeakyBucket.max integer types

igned-off-by: Alberto Garcia <be...@igalia.com> --- include/qemu/throttle.h | 4 ++-- util/throttle.c | 7 ++- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index d056008c18..ec37ac0fcb 100644 --- a/include/qemu/t

Re: [Qemu-block] [PATCH v2 11/54] quorum: use qapi_enum_parse() in quorum_open

On Tue 22 Aug 2017 03:22:12 PM CEST, Marc-André Lureau wrote: > @@ -925,7 +908,13 @@ static int quorum_open(BlockDriverState *bs, QDict > *options, int flags, > goto exit; > } > > -ret = parse_read_pattern(qemu_opt_get(opts, QUORUM_OPT_READ_PATTERN)); > +if

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

On Tue 22 Aug 2017 12:15:34 PM CEST, Manos Pitsidianakis wrote: > ## > +# @BlockdevOptionsThrottle: > +# > +# Driver specific block device options for the throttle driver > +# > +# @throttle-group: the name of the throttle-group object to use. It > +#must already exist. > +#

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

On Tue 22 Aug 2017 12:15:35 PM CEST, Manos Pitsidianakis wrote: > Signed-off-by: Manos Pitsidianakis <el13...@mail.ntua.gr> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [Qemu-devel] [PATCH v2 11/54] quorum: use qapi_enum_parse() in quorum_open

ret = QUORUM_READ_PATTERN_QUORUM; } else { ret = qapi_enum_parse(QuorumReadPattern_lookup, pattern_str, QUORUM_READ_PATTERN__MAX, -EINVAL, NULL); } I think I prefer this last one the best, but using the default error message is also ok. With either solution, Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH v2 5/6] block: add iotest 191 for legacy throttling interface

ntua.gr> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

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

wo's complement. > > So does HMP's "info block". > > Signed-off-by: Markus Armbruster <arm...@redhat.com> This is fine because all those parameters are limited to [0, 10^15], so changing int64_t -> uint64_t is not a problem. I have already sent a patch that change

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

the previous I/O operation. Signed-off-by: Alberto Garcia <be...@igalia.com> --- include/qemu/throttle.h | 4 ++-- util/throttle.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index 66a8ac10a4..c466f6ccaa

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

ket() and throttle_unfix_bucket() functions completely and moves the logic to throttle_compute_wait(). Signed-off-by: Alberto Garcia <be...@igalia.com> --- util/throttle.c | 62 + 1 file changed, 23 insertions(+), 39 deletions(-) diff --git a/util/thr

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

the guest to perform 100ms' worth of I/O at the target rate without being throttled. Signed-off-by: Alberto Garcia <be...@igalia.com> --- util/throttle.c | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/util/throttle.c b/util/throttle.c index b2a52b8b34..9a6bda813c 10

[Qemu-block] [PATCH 0/4] Misc throttle fixes

ains a new version of that patch that replaces the one that you have. Regards, Berto Alberto Garcia (4): throttle: Fix wrong variable name in the header documentation throttle: Update the throttle_fix_bucket() documentation throttle: Remove throttle_fix_bucket() / throttle_unfix_bucket()

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

On Fri 18 Aug 2017 05:10:18 AM CEST, Manos Pitsidianakis wrote: > 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

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

On Fri 18 Aug 2017 11:07:22 AM CEST, Manos Pitsidianakis wrote: >>> The driver can be used with the syntax >>> -drive driver=throttle,file.filename=foo.qcow2, \ >>> limits.iops-total=...,throttle-group=bar >> >>I had understood that we would get rid of the limits.* options in this

Re: [Qemu-block] [PATCH for-2.10] qemu-iotests: step clock after each test iteration

bypass throttling limits. > > Therefore it makes more sense to fix the test case than to modify QEMU. > > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

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

On Fri 18 Aug 2017 05:10:17 AM CEST, Manos Pitsidianakis wrote: > * If no ThrottleGroup is found with the given name a new one is > * created. > * > - * @name: the name of the ThrottleGroup > + * This function edits throttle_groups and must be called under the global > + * mutex. > + * > +

Re: [Qemu-block] [PATCH 02/18] block: access quiesce_counter with atomic ops

On Thu 11 May 2017 04:41:52 PM CEST, Paolo Bonzini wrote: > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

[Qemu-block] [PATCH] stream: fix crash in stream_start() when block_job_create() fails

stream_complete(). Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/stream.c b/block/stream.c index 0113710845..52d329f5c6 100644 --- a/block/stream.c +++ b/block/stream.c @@ -280,6 +280,6 @

Re: [Qemu-block] QEMU seg-fault with intermediate image streaming -- bdrv_reopen() in stream_start()

On Sat 13 May 2017 12:45:36 AM CEST, Kashyap Chamarthy wrote: > Try to perform intermediate streaming (pull clusters from 'disk1.qcow2' > into 'b.qcow2': > > (QEMU) block-stream device=#block832 base=disk1.qcow2 > > > Result > -- > > QEMU crashes with SIGSEGV: I see the problem, I'll send

Re: [Qemu-block] [PATCH] qemu-iotests: Test streaming with missing job ID

de) >> c) its node name is not valid for a block job (e.g. it contains a '#') > > I don't think c) is necessary, block jobs that are owned by a BDS > identified by a node name always require an explicit job ID. It defaults to bdrv_get_device_name() if the job ID is not set, but in your

Re: [Qemu-block] [PATCH] qemu-iotests: Test streaming with missing job ID

On Mon 15 May 2017 02:39:40 PM CEST, Kevin Wolf wrote: > This adds a small test for the image streaming error path for failing > block_job_create(), which would have found the null pointer dereference > in commit a170a91f. > > Signed-off-by: Kevin Wolf > --- >

Re: [Qemu-block] [PATCH v6 13/18] qcow2: add support for LUKS encryption format

rtual sector. This guarantees unique initialization > vectors for all sectors when qcow2 internal snapshots are > used, thus giving stronger protection against watermarking > attacks. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

[Qemu-block] [PATCH] qcow2: remove extra local_error variable

Commit d7086422b1c1e75e320519cfe26176db6ec97a37 added a local_err variable global to the qcow2_amend_options() function, so there's no need to have this other one. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)

Re: [Qemu-block] [PATCH v6 11/18] qcow2: convert QCow2 to use QCryptoBlock for encryption

On Tue 25 Apr 2017 05:38:51 PM CEST, Daniel P. Berrange wrote: > +switch (s->crypt_method_header) { > +case QCOW_CRYPT_NONE: > +if (encryptfmt) { > +error_setg(errp, "No encryption in image header, but options " > + "specified format '%s'",

Re: [Qemu-block] [PATCH v6 07/18] block: deprecate "encryption=on" in favour of "encrypt.format=aes"

> mapping to a nested QAPI schema at later date. > > e.g. the preferred syntax is now > > qemu-img create -f qcow2 -o encrypt.format=aes demo.qcow2 > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH v6 09/18] qcow: convert QCow to use QCryptoBlock for encryption

On Tue 25 Apr 2017 05:38:49 PM CEST, Daniel P. Berrange wrote: > @@ -181,8 +188,39 @@ static int qcow_open(BlockDriverState *bs, QDict > *options, int flags, [...] > +crypto_opts = block_crypto_open_opts_init( > +Q_CRYPTO_BLOCK_FORMAT_QCOW, encryptopts, _err); > +

Re: [Qemu-block] [PATCH 07/18] throttle-groups: only start one coroutine from drained_begin

to the next, because the > timer is set to "now + 1" but QEMU_CLOCK_VIRTUAL might not be running. > Set that timer to point in the present ("now") rather than the future > and things work. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH 08/18] throttle-groups: do not use qemu_co_enter_next

bonz...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH v2 3/7] qcow2: Make perform_cow() call do_perform_cow() twice

On Wed 07 Jun 2017 11:43:34 PM CEST, Eric Blake wrote: >> block/qcow2-cluster.c | 38 +++--- >> 1 file changed, 23 insertions(+), 15 deletions(-) > >> qemu_co_mutex_unlock(>lock); >> -ret = do_perform_cow(bs, m->offset, m->alloc_offset, r->offset, >>

Re: [Qemu-block] [PATCH v2 4/7] qcow2: Split do_perform_cow() into _read(), _encrypt() and _write()

On Fri 09 Jun 2017 04:53:05 PM CEST, Eric Blake wrote: > Let's suppose we have a guest issuing 512-byte aligned requests and a > host that requires 4k alignment; and the guest does an operation that > needs a COW with one sector at both the front and end of the cluster. > >> @@ -760,22 +776,59 @@

Re: [Qemu-block] [PATCH v9 07/20] block: deprecate "encryption=on" in favor of "encrypt.format=aes"

el(opts, BLOCK_OPT_ENCRYPT); > +if (buf != NULL) { > +g_free(buf); If you use qemu_opt_get() instead then you don't need "buf" at all, do you? > +if (encryptfmt) { > +buf = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT); > +if (buf != NULL) { > +g_free(buf); Same here. Everything else looks fine. Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

[Qemu-block] [PATCH v4 5/7] qcow2: Allow reading both COW regions with only one request

a conservative approach and only merges reads when the size of the middle region is <= 16KB. Signed-off-by: Alberto Garcia <be...@igalia.com> Reviewed-by: Kevin Wolf <kw...@redhat.com> --- block/qcow2-cluster.c | 51 ++- 1 file changed, 38 in

[Qemu-block] [PATCH v4 3/7] qcow2: Make perform_cow() call do_perform_cow() twice

Instead of calling perform_cow() twice with a different COW region each time, call it just once and make perform_cow() handle both regions. This patch simply moves code around. The next one will do the actual reordering of the COW operations. Signed-off-by: Alberto Garcia <be...@igalia.

[Qemu-block] [PATCH v4 1/7] qcow2: Remove unused Error variable in do_perform_cow()

to pass the original error instead of simply returning -EIO, but that would be more invasive, so let's keep the current approach. Signed-off-by: Alberto Garcia <be...@igalia.com> Reviewed-by: Eric Blake <ebl...@redhat.com> Reviewed-by: Kevin Wolf <kw...@redhat.com> --- block/q

[Qemu-block] [PATCH v4 6/7] qcow2: Pass a QEMUIOVector to do_perform_cow_{read, write}()

for consistency. Signed-off-by: Alberto Garcia <be...@igalia.com> Reviewed-by: Kevin Wolf <kw...@redhat.com> --- block/qcow2-cluster.c | 51 --- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/block/qcow2-cluster.c b

[Qemu-block] [PATCH v4 0/7] Reduce the number of I/O ops when doing COW

007/7:[0008] [FC] 'qcow2: Merge the writing of the COW regions with the guest data' Alberto Garcia (7): qcow2: Remove unused Error variable in do_perform_cow() qcow2: Use unsigned int for both members of Qcow2COWRegion qcow2: Make perform_cow() call do_perform_cow() twice qcow2:

[Qemu-block] [PATCH v4 4/7] qcow2: Split do_perform_cow() into _read(), _encrypt() and _write()

enough to hold both regions. Signed-off-by: Alberto Garcia <be...@igalia.com> Reviewed-by: Kevin Wolf <kw...@redhat.com> --- block/qcow2-cluster.c | 117 +- 1 file changed, 87 insertions(+), 30 deletions(-) diff --git a/block/qcow2-clust

[Qemu-block] [PATCH v4 7/7] qcow2: Merge the writing of the COW regions with the guest data

factors such as the media type, the cluster size and the I/O request size. Signed-off-by: Alberto Garcia <be...@igalia.com> Reviewed-by: Kevin Wolf <kw...@redhat.com> --- block/qcow2-cluster.c | 40 block/qcow2.c

[Qemu-block] [PATCH v4 2/7] qcow2: Use unsigned int for both members of Qcow2COWRegion

ype of do_perform_cow() is also updated to reflect these changes. Signed-off-by: Alberto Garcia <be...@igalia.com> Reviewed-by: Eric Blake <ebl...@redhat.com> Reviewed-by: Kevin Wolf <kw...@redhat.com> --- block/qcow2-cluster.c | 4 ++-- block/qcow2.h | 4 ++-- 2 files changed, 4 inser

Re: [Qemu-block] [PATCH v9 07/20] block: deprecate "encryption=on" in favor of "encrypt.format=aes"

On Tue 20 Jun 2017 02:02:06 PM CEST, Daniel P. Berrange wrote: >> > +if (encryptfmt) { >> > +buf = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT); >> > +if (buf != NULL) { >> > +g_free(buf); >> >> If you use qemu_opt_get() instead then you don't need "buf" at all, >> do

[Qemu-block] [PATCH] qcow2: Use offset_into_cluster() and offset_to_l2_index()

We already have functions for doing these calculations, so let's use them instead of doing everything by hand. This makes the code a bit more readable. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2-cluster.c | 4 ++-- block/qcow2.c | 2 +- 2 files changed, 3 inse

Re: [Qemu-block] [PATCH v10 07/20] block: deprecate "encryption=on" in favor of "encrypt.format=aes"

> mapping to a nested QAPI schema at later date. > > e.g. the preferred syntax is now > > qemu-img create -f qcow2 -o encrypt.format=aes demo.qcow2 > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH v10 10/20] qcow2: make qcow2_encrypt_sectors encrypt in place

tput already. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH v10 13/20] qcow2: add support for LUKS encryption format

rtual sector. This guarantees unique initialization > vectors for all sectors when qcow2 internal snapshots are > used, thus giving stronger protection against watermarking > attacks. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH v10 11/20] qcow2: convert QCow2 to use QCryptoBlock for encryption

be simplified since there is no longer a > difference in behaviour when using blockdev_add with encrypted > images for the running vs stopped CPU state. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH v10 09/20] qcow: convert QCow to use QCryptoBlock for encryption

On Fri 23 Jun 2017 06:24:08 PM CEST, Daniel P. Berrange wrote: > This converts the qcow driver to make use of the QCryptoBlock > APIs for encrypting image content. This is only wired up to > permit use of the legacy QCow encryption format. Users who wish > to have the strong LUKS format should

Re: [Qemu-block] [PATCH RFC v3 4/8] block: convert ThrottleGroup to object with QOM

On Mon 26 Jun 2017 06:58:32 PM CEST, Manos Pitsidianakis wrote: > On Mon, Jun 26, 2017 at 03:52:34PM +0100, Stefan Hajnoczi wrote: >>On Fri, Jun 23, 2017 at 03:46:56PM +0300, Manos Pitsidianakis wrote: >>> +static bool throttle_group_exists(const char *name) >>> +{ >>> +ThrottleGroup *iter;

[Qemu-block] [PATCH 6/7] qcow2: Pass a QEMUIOVector to do_perform_cow_{read, write}()

for consistency. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2-cluster.c | 51 --- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 67a761731d..ae737adf88

[Qemu-block] [PATCH 5/7] qcow2: Allow reading both COW regions with only one request

a conservative approach and only merges reads when the size of the middle region is <= 16KB. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2-cluster.c | 41 ++--- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/block/qcow2-cl

[Qemu-block] [PATCH 0/7] qcow2: Reduce the number of I/O ops when doing COW

image) and optimizing that case, but let's start with this. Regards, Berto Alberto Garcia (7): qcow2: Remove unused Error in do_perform_cow() qcow2: Use unsigned int for both members of Qcow2COWRegion qcow2: Make perform_cow() call do_perform_cow() twice qcow2: Split do_perform_cow

[Qemu-block] [PATCH 1/7] qcow2: Remove unused Error in do_perform_cow()

qcow2_encrypt_sectors() does not need an Error parameter, and we're not checking its value anyway, so we can safely remove it. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2-cluster.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/block/qcow2-clust

[Qemu-block] [PATCH 7/7] qcow2: Merge the writing of the COW regions with the guest data

factors such as the media type, the cluster size and the I/O request size. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2-cluster.c | 34 ++ block/qcow2.c | 58 ++- block/qcow2.h

[Qemu-block] [PATCH 4/7] qcow2: Split do_perform_cow() into _read(), _encrypt() and _write()

enough to hold both regions. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2-cluster.c | 114 +- 1 file changed, 84 insertions(+), 30 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 59a0ffba1a..643c

[Qemu-block] [PATCH 2/7] qcow2: Use unsigned int for both members of Qcow2COWRegion

ype of do_perform_cow() is also updated to reflect these changes. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2-cluster.c | 4 ++-- block/qcow2.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 3

[Qemu-block] [PATCH 3/7] qcow2: Make perform_cow() call do_perform_cow() twice

Instead of calling perform_cow() twice with a different COW region each time, call it just once and make perform_cow() handle both regions. This patch simply moves code around. The next one will do the actual reordering of the COW operations. Signed-off-by: Alberto Garcia <be...@igalia.

Re: [Qemu-block] [PATCH 1/7] qcow2: Remove unused Error in do_perform_cow()

On Tue 23 May 2017 10:21:49 PM CEST, Eric Blake wrote: >> qcow2_encrypt_sectors() does not need an Error parameter, and we're >> not checking its value anyway, so we can safely remove it. > > Misleading. You are NOT removing the Error parameter from > qcow2_encrypt_sectors(), but rather are

Re: [Qemu-block] [PATCH 0/7] qcow2: Reduce the number of I/O ops when doing COW

On Tue 23 May 2017 04:36:52 PM CEST, Eric Blake wrote: >> here's a patch series that rewrites the copy-on-write code in the >> qcow2 driver to reduce the number of I/O operations. > > And it competes with Denis and Anton's patches: >

Re: [Qemu-block] [PATCH 0/7] qcow2: Reduce the number of I/O ops when doing COW

On Wed 24 May 2017 06:09:42 PM CEST, Anton Nefedov wrote: > I agree; as mentioned we have similar patches and they don't conflict > much. We noticed a performance regression on HDD though, for the > presumably optimized case (random 4k write over a large backed image); > so the patches were put

Re: [Qemu-block] [PATCH 0/7] qcow2: Reduce the number of I/O ops when doing COW

On Fri 26 May 2017 03:32:49 PM CEST, Anton Nefedov wrote: >>> [root@localhost ~]# fio --name=randwrite --blocksize=4k >>> --filename=/dev/sdb --rw=randwrite --direct=1 --ioengine=libaio >>> --size=2g --io_size=32m >> >> In my tests I sometimes detected slight performance decreases in that >> HDD

Re: [Qemu-block] [PATCH v7 09/20] qcow: convert QCow to use QCryptoBlock for encryption

directly to block_crypto_create_opts_init() and then check if crypto_opts is NULL. Actually none of the error_propagate() calls in qcow_create() is really necessary, but that could be fixed in a separate patch, if at all (it's not so important). The leak however needs to be fixed. With that, Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH 7/7] qcow2: Merge the writing of the COW regions with the guest data

On Tue 23 May 2017 01:23:02 PM CEST, Alberto Garcia wrote: You can still review this now if you want, but here's a couple of minor things I'll correct in the next revision: > +if (m->data_qiov) { > +qemu_iovec_reset(); > +qemu_iovec_add(, start_buffer, sta

Re: [Qemu-block] [PATCH v7 11/20] qcow2: convert QCow2 to use QCryptoBlock for encryption

be simplified since there is no longer a > difference in behaviour when using blockdev_add with encrypted > images for the running vs stopped CPU state. > > Reviewed-by: Eric Blake <ebl...@redhat.com> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH 0/7] qcow2: Reduce the number of I/O ops when doing COW

On Fri 26 May 2017 02:47:55 PM CEST, Anton Nefedov wrote: > Tried the another machine; about 10% improvement here [...] > [root@localhost ~]# fio --name=randwrite --blocksize=4k > --filename=/dev/sdb --rw=randwrite --direct=1 --ioengine=libaio > --size=2g --io_size=32m In my tests I sometimes

Re: [Qemu-block] [PATCH v7 20/20] docs: document encryption options for qcow, qcow2 and luks

On Thu 25 May 2017 06:38:51 PM CEST, "Daniel P. Berrange" <berra...@redhat.com> wrote: > Expand the image format docs to cover the new options for > the qcow, qcow2 and luks disk image formats > > Signed-off-by: Daniel P. Berrange <berra...@redhat.co

Re: [Qemu-block] [PATCH v7 17/20] block: remove all encryption handling APIs

edhat.com> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH 09/18] throttle-groups: protect throttled requests with a CoMutex

gt; alternative. > > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH v6 11/18] qcow2: convert QCow2 to use QCryptoBlock for encryption

On Wed 24 May 2017 06:46:31 PM CEST, Daniel P. Berrange wrote: >> > +case QCOW_CRYPT_AES: >> > +if (encryptfmt && !g_str_equal(encryptfmt, "aes")) { >> > +error_setg(errp, >> > + "Header reported 'aes' encryption format but " >> > +

Re: [Qemu-block] [PATCH 0/7] qcow2: Reduce the number of I/O ops when doing COW

On Wed 24 May 2017 06:26:23 PM CEST, Anton Nefedov wrote: >>> I agree; as mentioned we have similar patches and they don't >>> conflict much. We noticed a performance regression on HDD though, >>> for the presumably optimized case (random 4k write over a large >>> backed image); so the patches

Re: [Qemu-block] [PATCH 3/7] qcow2: Make perform_cow() call do_perform_cow() twice

On Fri 26 May 2017 10:11:29 AM CEST, Kevin Wolf wrote: >> --- a/block/qcow2-cluster.c >> +++ b/block/qcow2-cluster.c >> @@ -414,6 +414,10 @@ static int coroutine_fn do_perform_cow(BlockDriverState >> *bs, >> struct iovec iov; >> int ret; >> >> +if (bytes == 0) {

Re: [Qemu-block] [RFC] Making 'block-stream', and 'block-commit' accept node-name

On Mon 29 May 2017 09:03:22 PM CEST, Kashyap Chamarthy wrote: > Observe the following ('qmp-shell', for brevity) invocation of the > four major types (stream, commit, mirror, backup) of live block > operations: > > (QEMU) block-stream device=node-D base=a.qcow2 job-id=job-block-stream >

Re: [Qemu-block] Throttling groups vs filter nodes

On Sat 27 May 2017 09:56:03 AM CEST, Stefan Hajnoczi wrote: > Throttling groups allow multiple drives to share the same throttling > state (i.e. budget) between them. Manos is working on moving the > throttling code into a block filter driver so it is no longer > hardcoded into the I/O code path.

Re: [Qemu-block] [PATCH v8 19/20] qcow2: report encryption specific image information

lated to it. Reviewed-by: Alberto Garcia <be...@igalia.com> Berto

Re: [Qemu-block] [PATCH] blockjob: cancel blockjobs before stopping all iothreads

On Sat 03 Jun 2017 07:48:37 AM CEST, sochin.jiang wrote: > --- a/block.c > +++ b/block.c > @@ -3084,9 +3084,16 @@ static void bdrv_close(BlockDriverState *bs) > bdrv_drained_end(bs); > } > > +void bdrv_cancel_all(void) > +{ > +if (!block_jobs_is_empty()) { > +

Re: [Qemu-block] [PATCH 0/7] qcow2: Reduce the number of I/O ops when doing COW

On Wed 07 Jun 2017 01:59:58 PM CEST, Kevin Wolf wrote: > Am 07.06.2017 um 13:44 hat Alberto Garcia geschrieben: >> ping > > You wanted to address two or three things in the next version, so I > assumed that this version shouldn't be merged. Right, I had a couple of minor cha

Re: [Qemu-block] [PATCH 0/7] qcow2: Reduce the number of I/O ops when doing COW

ping On Tue, May 23, 2017 at 01:22:55PM +0200, Alberto Garcia wrote: > Hi all, > > here's a patch series that rewrites the copy-on-write code in the > qcow2 driver to reduce the number of I/O operations. > > The situation is that when a guest sends a write request and QEMU &

Re: [Qemu-block] [PATCH v2 2/7] qcow2: Use unsigned int for both members of Qcow2COWRegion

On Wed 07 Jun 2017 06:02:06 PM CEST, Eric Blake wrote: >> - The offset of the COW region from the start of the first cluster >> touched by the I/O request. Since it's always going to be positive >> and the maximum request size is at most INT_MAX, we can use a >> regular unsigned int to

Re: [Qemu-block] [PATCH 2/5] block: Don't try to set *errp directly

s no need to check if errp is NULL anymore, as > error_propagate() and error_prepend() are able to handle that. > > Cc: Kevin Wolf <kw...@redhat.com> > Cc: Max Reitz <mre...@redhat.com> > Cc: qemu-block@nongnu.org > Signed-off-by: Eduardo Habkost <ehabk..

[Qemu-block] [PATCH v2 2/7] qcow2: Use unsigned int for both members of Qcow2COWRegion

ype of do_perform_cow() is also updated to reflect these changes. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2-cluster.c | 4 ++-- block/qcow2.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index d

[Qemu-block] [PATCH v2 0/7] qcow2: Reduce the number of I/O ops when doing COW

: Allow reading both COW regions with only one request' 006/7:[] [--] 'qcow2: Pass a QEMUIOVector to do_perform_cow_{read,write}()' 007/7:[0014] [FC] 'qcow2: Merge the writing of the COW regions with the guest data' Alberto Garcia (7): qcow2: Remove unused Error variable in do_perform_cow

[Qemu-block] [PATCH v2 1/7] qcow2: Remove unused Error variable in do_perform_cow()

to pass the original error instead of simply returning -EIO, but that would be more invasive, so let's keep the current approach. Signed-off-by: Alberto Garcia <be...@igalia.com> --- block/qcow2-cluster.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/block/qcow2-clu

[Qemu-block] [PATCH v2 3/7] qcow2: Make perform_cow() call do_perform_cow() twice

Instead of calling perform_cow() twice with a different COW region each time, call it just once and make perform_cow() handle both regions. This patch simply moves code around. The next one will do the actual reordering of the COW operations. Signed-off-by: Alberto Garcia <be...@igalia.

<    5   6   7   8   9   10   11   12   13   14   >