Re: [Qemu-block] [PATCH v6 03/22] blockdev: Add and parse "lock-mode" option for image locking

2016-07-07 Thread Fam Zheng
On Tue, 07/05 15:37, Kevin Wolf wrote: > Am 17.06.2016 um 11:23 hat Kevin Wolf geschrieben: > > Am 03.06.2016 um 10:48 hat Fam Zheng geschrieben: > > > Respect the locking mode from CLI or QMP, and set the open flags > > > accordingly. > > > > > > Signed-off-by: Fam Zheng > > > Reviewed-by: Max R

Re: [Qemu-block] [PATCH v2 6/7] mirror: efficiently zero out target

2016-07-07 Thread Eric Blake
On 07/07/2016 03:35 AM, Denis V. Lunev wrote: > With a bdrv_co_write_zeroes method on a target BDS zeroes will not be placed The commit message doesn't match the code. A target BDS may have bdrv_co_write_zeroes but still send zeroes across the wire (example: NBD client that has my patch for using

Re: [Qemu-block] [PATCH v2 5/7] mirror: optimize dirty bitmap filling in mirror_run a bit

2016-07-07 Thread Eric Blake
On 07/07/2016 03:35 AM, Denis V. Lunev wrote: > There is no need to scan allocation tables if we have mark_all_dirty flag > set. Just mark it all dirty. > > Signed-off-by: Denis V. Lunev > Reviewed-by: Vladimir Sementsov-Ogievskiy > CC: Stefan Hajnoczi > CC: Fam Zheng > CC: Kevin Wolf > CC: Ma

Re: [Qemu-block] [PATCH v2 4/7] block: remove extra condition in bdrv_can_write_zeroes_with_unmap

2016-07-07 Thread Eric Blake
On 07/07/2016 03:35 AM, Denis V. Lunev wrote: > All nowadays .bdrv_co_write_zeroes callbacks could perfectly work even Grammar: All .bdrv_co_write_zeroes callbacks nowadays work perfectly even... > with backing store attached. If future new callbacks will unable to do s/will/would be/ > that -

Re: [Qemu-block] [Qemu-devel] [PATCH v5 4/4] block: ignore flush requests when storage is clean

2016-07-07 Thread Eric Blake
On 07/04/2016 08:38 AM, Denis V. Lunev wrote: > From: Evgeny Yakovlev > > Some guests (win2008 server for example) do a lot of unnecessary > flushing when underlying media has not changed. This adds additional > overhead on host when calling fsync/fdatasync. > > This change introduces a write ge

Re: [Qemu-block] [PATCH v3 02/11] block: Accept node-name for block-commit

2016-07-07 Thread Eric Blake
On 07/07/2016 06:11 AM, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow node-names everywhere. This converts > block-commit to accept a node-name without lifting the restriction that > we're operating at a root node. > > As lib

Re: [Qemu-block] [PATCH v3 01/11] block: Accept node-name for block-stream

2016-07-07 Thread Eric Blake
On 07/07/2016 06:11 AM, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow node-names everywhere. This converts > block-stream to accept a node-name without lifting the restriction that > we're operating at a root node. > > In cas

Re: [Qemu-block] [PATCH v2 3/7] mirror: create mirror_dirty_init helper for mirror_run

2016-07-07 Thread Eric Blake
On 07/07/2016 03:35 AM, Denis V. Lunev wrote: > The code inside the helper will be extended in the next patch. mirror_run > itself is overbloated at the moment. > > Signed-off-by: Denis V. Lunev > Reviewed-by: Vladimir Sementsov-Ogievskiy > CC: Stefan Hajnoczi > CC: Fam Zheng > CC: Kevin Wolf

Re: [Qemu-block] [Qemu-devel] [PATCH v5 0/4] block: ignore flush requests when storage is clean

2016-07-07 Thread John Snow
On 07/04/2016 10:53 AM, Paolo Bonzini wrote: > On 04/07/2016 16:38, Denis V. Lunev wrote: >> Changes from v4: >> - Moved to write generation scheme instead of dirty flag >> - Added retry setup to IDE PIO and FLUSH requests >> >> Changes from v3: >> - Fixed a typo in commit message >> - Rebased on

Re: [Qemu-block] [PATCH v2 2/7] mirror: make sectors_in_flight int64_t

2016-07-07 Thread Eric Blake
On 07/07/2016 03:35 AM, Denis V. Lunev wrote: > We keep here the sum of int fields. Thus this could easily overflow, > especially when we will start sending big requests in next patches. > > Signed-off-by: Denis V. Lunev > Reviewed-by: Vladimir Sementsov-Ogievskiy > CC: Stefan Hajnoczi > CC: Fa

Re: [Qemu-block] [PATCH v2 1/7] dirty-bitmap: operate with int64_t amount

2016-07-07 Thread Eric Blake
On 07/07/2016 03:35 AM, Denis V. Lunev wrote: > Underlying HBitmap operates even with uint64_t. Thus this change is safe. > This would be useful f.e. to mark entire bitmap dirty in one call. > > Signed-off-by: Denis V. Lunev > Reviewed-by: Vladimir Sementsov-Ogievskiy > CC: Stefan Hajnoczi > CC

Re: [Qemu-block] [PATCH v2 0/2] block: Add qemu_dup in osdep.c and use it

2016-07-07 Thread John Snow
On 06/22/2016 08:53 AM, Fam Zheng wrote: > v2: Fix patch 1 #else branch, and "r" => "ret". [Kevin] > Add Kevin's r-b line in patch 2. > > This is an independent tiny change extracted from the image locking series, > which can be processed separately. > > Improved according to Kevin's sugges

Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

2016-07-07 Thread Colin Lord
On 07/06/2016 04:24 AM, Kevin Wolf wrote: > Am 05.07.2016 um 22:50 hat John Snow geschrieben: >> >> >> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote: >>> On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote: This puts the bochs probe function into its own separate file as part of

Re: [Qemu-block] [PATCH v3 00/32] Dynamic module loading for block drivers

2016-07-07 Thread John Snow
On 07/05/2016 11:24 AM, Colin Lord wrote: > This is the next version of this patch series. The first three patches > in the series are mostly the same as they were last time, but with the > issues mentioned in the reviews fixed. Most notably this means much less > copy-paste happening in block.c.

Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

2016-07-07 Thread John Snow
On 07/07/2016 12:01 PM, Paolo Bonzini wrote: > > > On 06/07/2016 18:09, John Snow wrote: >> Recently the include files in hw/ide/ were moved to include/ without >> anybody mentioning it to me, including internal.h. >> >> Why? > > Because hw/ide/internal.h is not so internal. In particular, it

Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

2016-07-07 Thread Paolo Bonzini
On 06/07/2016 18:09, John Snow wrote: > Recently the include files in hw/ide/ were moved to include/ without > anybody mentioning it to me, including internal.h. > > Why? Because hw/ide/internal.h is not so internal. In particular, it is included by hw/ide/pci.h, which is included by hw/i386/p

Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

2016-07-07 Thread Paolo Bonzini
On 05/07/2016 22:50, John Snow wrote: > So, something like: > > block/drivers/bochs/ > > bochs.c > probe.c (or bochs-probe.c) > > and > > include/block/drivers/bochs/ > > common.h (or internal.h) > > > Any objections from the gallery? I guess I'm missing why it is useful to modularize dri

Re: [Qemu-block] [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

2016-07-07 Thread John Snow
On 07/07/2016 02:36 AM, Markus Armbruster wrote: > John Snow writes: > >> On 07/06/2016 04:24 AM, Kevin Wolf wrote: >>> Am 05.07.2016 um 22:50 hat John Snow geschrieben: On 07/05/2016 11:49 AM, Daniel P. Berrange wrote: > On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord w

Re: [Qemu-block] [PATCH v4 00/11] Allow creating block jobs with a user-defined ID

2016-07-07 Thread John Snow
On 07/07/2016 11:12 AM, Alberto Garcia wrote: > On Thu 07 Jul 2016 02:48:17 PM CEST, Kevin Wolf wrote: >>> Hi all, >>> >>> block jobs are currently identified by the name of the block backend >>> of the BDS where the job was started. >>> >>> The problem with this is that you cannot have block job

Re: [Qemu-block] [PATCH v4 00/11] Allow creating block jobs with a user-defined ID

2016-07-07 Thread Alberto Garcia
On Thu 07 Jul 2016 02:48:17 PM CEST, Kevin Wolf wrote: >> Hi all, >> >> block jobs are currently identified by the name of the block backend >> of the BDS where the job was started. >> >> The problem with this is that you cannot have block jobs on nodes >> where there is no such name. >> >> This

Re: [Qemu-block] [PATCH v3 17/32] blockdev: Separate bochs probe from its driver

2016-07-07 Thread Colin Lord
On 07/06/2016 11:59 AM, Max Reitz wrote: > On 05.07.2016 17:24, Colin Lord wrote: >> Modifies the bochs probe to return the format name as well as the >> score as the final step of separating the probe function from the >> driver. This keeps the probe completely independent of the driver, >> making

Re: [Qemu-block] [PATCH v3 01/11] block: Accept node-name for block-stream

2016-07-07 Thread Kevin Wolf
Am 07.07.2016 um 16:39 hat Alberto Garcia geschrieben: > On Thu 07 Jul 2016 04:17:21 PM CEST, Kevin Wolf wrote: > >> > +static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp) > >> > +{ > >> > +BlockDriverState *bs; > >> > + > >> > +bs = bdrv_lookup_bs(name, name, errp); >

Re: [Qemu-block] [PATCH v3 01/11] block: Accept node-name for block-stream

2016-07-07 Thread Alberto Garcia
On Thu 07 Jul 2016 04:17:21 PM CEST, Kevin Wolf wrote: >> > +static BlockDriverState *qmp_get_root_bs(const char *name, Error **errp) >> > +{ >> > +BlockDriverState *bs; >> > + >> > +bs = bdrv_lookup_bs(name, name, errp); >> > +if (bs == NULL) { >> > +return NULL; >> > +} >>

Re: [Qemu-block] [PATCH v3 01/11] block: Accept node-name for block-stream

2016-07-07 Thread Kevin Wolf
Am 07.07.2016 um 14:59 hat Alberto Garcia geschrieben: > On Thu 07 Jul 2016 02:11:27 PM CEST, Kevin Wolf wrote: > > In order to remove the necessity to use BlockBackend names in the > > external API, we want to allow node-names everywhere. This converts > > block-stream to accept a node-name withou

Re: [Qemu-block] [PATCH v3 01/11] block: Accept node-name for block-stream

2016-07-07 Thread Alberto Garcia
On Thu 07 Jul 2016 02:11:27 PM CEST, Kevin Wolf wrote: > In order to remove the necessity to use BlockBackend names in the > external API, we want to allow node-names everywhere. This converts > block-stream to accept a node-name without lifting the restriction that > we're operating at a root node

Re: [Qemu-block] [PATCH v4 00/11] Allow creating block jobs with a user-defined ID

2016-07-07 Thread Kevin Wolf
Am 05.07.2016 um 16:28 hat Alberto Garcia geschrieben: > Hi all, > > block jobs are currently identified by the name of the block backend > of the BDS where the job was started. > > The problem with this is that you cannot have block jobs on nodes > where there is no such name. > > This series t

[Qemu-block] [PATCH v3 11/11] nbd-server: Allow node name for nbd-server-add

2016-07-07 Thread Kevin Wolf
There is no reason why an NBD server couldn't be started for any node, even if it's not on the top level. This converts nbd-server-add to accept a node-name. Note that there is a semantic difference between using a BlockBackend name and the node name of its root: In the former case, the NBD server

[Qemu-block] [PATCH v3 09/11] block: Accept node-name for drive-mirror

2016-07-07 Thread Kevin Wolf
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts drive-mirror to accept a node-name without lifting the restriction that we're operating at a root node. In case of an invalid device name, the command returns the G

Re: [Qemu-block] [PATCH] block/iscsi: precache the allocation status of a target

2016-07-07 Thread Paolo Bonzini
On 07/07/2016 13:40, Peter Lieven wrote: >>> >> This can take a long time and the disks may not even be ever used. I >> don't think it's a good idea. > > Sure, the target might stay unused, but why do you suspect its slow? I don't suspect it's slow; it's just that it can be O(size of disk), an

[Qemu-block] [PATCH v3 08/11] block: Accept node-name for drive-backup

2016-07-07 Thread Kevin Wolf
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts drive-backup and the corresponding transaction action to accept a node-name without lifting the restriction that we're operating at a root node. In case of an inval

[Qemu-block] [PATCH v3 10/11] nbd-server: Use a separate BlockBackend

2016-07-07 Thread Kevin Wolf
The builtin NBD server uses its own BlockBackend now instead of reusing the monitor/guest device one. This means that it has its own writethrough setting now. The builtin NBD server always uses writeback caching now regardless of whether the guest device has WCE enabled. qemu-nbd respects the cach

[Qemu-block] [PATCH v3 05/11] block: Accept node-name for blockdev-snapshot-delete-internal-sync

2016-07-07 Thread Kevin Wolf
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts blockdev-snapshot-delete-internal-sync to accept a node-name without lifting the restriction that we're operating at a root node. In case of an invalid device name,

[Qemu-block] [PATCH v3 06/11] block: Accept node-name for blockdev-snapshot-internal-sync

2016-07-07 Thread Kevin Wolf
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts blockdev-snapshot-internal-sync to accept a node-name without lifting the restriction that we're operating at a root node. In case of an invalid device name, the co

[Qemu-block] [PATCH v3 07/11] block: Accept node-name for change-backing-file

2016-07-07 Thread Kevin Wolf
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts change-backing-file to accept a node-name without lifting the restriction that we're operating at a root node. In case of an invalid device name, the command return

[Qemu-block] [PATCH v3 03/11] block: Accept node-name for blockdev-backup

2016-07-07 Thread Kevin Wolf
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts blockdev-backup and the corresponding transaction action to accept a node-name without lifting the restriction that we're operating at a root node. In case of an in

[Qemu-block] [PATCH v3 04/11] block: Accept node-name for blockdev-mirror

2016-07-07 Thread Kevin Wolf
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts blockdev-mirror to accept a node-name without lifting the restriction that we're operating at a root node. Signed-off-by: Kevin Wolf --- blockdev.c | 10

[Qemu-block] [PATCH v3 02/11] block: Accept node-name for block-commit

2016-07-07 Thread Kevin Wolf
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts block-commit to accept a node-name without lifting the restriction that we're operating at a root node. As libvirt makes use of the DeviceNotFound error class, we m

[Qemu-block] [PATCH v3 00/11] block: Accept node-name in all node level QMP commands

2016-07-07 Thread Kevin Wolf
As stated in the RFC I sent two weeks ago: * Node level commands: We need to complete the conversion that makes commands accept node names instead of BlockBackend names. In some places we intentionally allow only BlockBackends because we don't know if the command works in other p

[Qemu-block] [PATCH v3 01/11] block: Accept node-name for block-stream

2016-07-07 Thread Kevin Wolf
In order to remove the necessity to use BlockBackend names in the external API, we want to allow node-names everywhere. This converts block-stream to accept a node-name without lifting the restriction that we're operating at a root node. In case of an invalid device name, the command returns the G

Re: [Qemu-block] [PATCH] block/iscsi: precache the allocation status of a target

2016-07-07 Thread Peter Lieven
Am 30.06.2016 um 17:59 schrieb Paolo Bonzini: On 30/06/2016 13:08, Peter Lieven wrote: this fills up the allocationmap at iscsi_open. This helps to reduce the number of get_block_status requests during runtime significantly. Signed-off-by: Peter Lieven --- block/iscsi.c | 16 +++

[Qemu-block] [PATCH v2 1/7] dirty-bitmap: operate with int64_t amount

2016-07-07 Thread Denis V. Lunev
Underlying HBitmap operates even with uint64_t. Thus this change is safe. This would be useful f.e. to mark entire bitmap dirty in one call. Signed-off-by: Denis V. Lunev Reviewed-by: Vladimir Sementsov-Ogievskiy CC: Stefan Hajnoczi CC: Fam Zheng CC: Kevin Wolf CC: Max Reitz CC: Jeff Cody C

[Qemu-block] [PATCH v2 7/7] mirror: improve performance of mirroring of empty disk

2016-07-07 Thread Denis V. Lunev
We should not take into account zero blocks for delay calculations. They are not read and thus IO throttling is not required. In the other case VM migration with 16 Tb QCOW2 disk with 4 Gb of data takes days. Signed-off-by: Denis V. Lunev Reviewed-by: Vladimir Sementsov-Ogievskiy CC: Stefan Hajn

[Qemu-block] [PATCH v2 0/7] drive-mirror improvements

2016-07-07 Thread Denis V. Lunev
This patchset contains patches dealing with known-to-be-zero areas in drive mirror from [PATCH 0/9] major rework of drive-mirror patchset. Changes from v1: - only patches dealing with zeroes remains - ported to current HEAD - fixed issue with dirty-bitmap, int length is changed with int64 - fixed

[Qemu-block] [PATCH v2 3/7] mirror: create mirror_dirty_init helper for mirror_run

2016-07-07 Thread Denis V. Lunev
The code inside the helper will be extended in the next patch. mirror_run itself is overbloated at the moment. Signed-off-by: Denis V. Lunev Reviewed-by: Vladimir Sementsov-Ogievskiy CC: Stefan Hajnoczi CC: Fam Zheng CC: Kevin Wolf CC: Max Reitz CC: Jeff Cody CC: Eric Blake --- block/mirro

[Qemu-block] [PATCH v2 2/7] mirror: make sectors_in_flight int64_t

2016-07-07 Thread Denis V. Lunev
We keep here the sum of int fields. Thus this could easily overflow, especially when we will start sending big requests in next patches. Signed-off-by: Denis V. Lunev Reviewed-by: Vladimir Sementsov-Ogievskiy CC: Stefan Hajnoczi CC: Fam Zheng CC: Kevin Wolf CC: Max Reitz CC: Jeff Cody CC: E

[Qemu-block] [PATCH v2 4/7] block: remove extra condition in bdrv_can_write_zeroes_with_unmap

2016-07-07 Thread Denis V. Lunev
All nowadays .bdrv_co_write_zeroes callbacks could perfectly work even with backing store attached. If future new callbacks will unable to do that - they have a chance to block this in bdrv_get_info(). Signed-off-by: Denis V. Lunev CC: Stefan Hajnoczi CC: Fam Zheng CC: Kevin Wolf CC: Max Reitz

[Qemu-block] [PATCH v2 6/7] mirror: efficiently zero out target

2016-07-07 Thread Denis V. Lunev
With a bdrv_co_write_zeroes method on a target BDS zeroes will not be placed into the wire. Thus the target could be very efficiently zeroed out. This is should be done with the largest chunk possible. Signed-off-by: Denis V. Lunev Reviewed-by: Vladimir Sementsov-Ogievskiy CC: Stefan Hajnoczi CC

[Qemu-block] [PATCH v2 5/7] mirror: optimize dirty bitmap filling in mirror_run a bit

2016-07-07 Thread Denis V. Lunev
There is no need to scan allocation tables if we have mark_all_dirty flag set. Just mark it all dirty. Signed-off-by: Denis V. Lunev Reviewed-by: Vladimir Sementsov-Ogievskiy CC: Stefan Hajnoczi CC: Fam Zheng CC: Kevin Wolf CC: Max Reitz CC: Jeff Cody CC: Eric Blake --- block/mirror.c | 8

Re: [Qemu-block] [Qemu-devel] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm

2016-07-07 Thread Daniel P. Berrange
On Wed, Jul 06, 2016 at 08:53:56AM -0600, Eric Blake wrote: > On 07/06/2016 05:58 AM, Alberto Garcia wrote: > > On Tue 05 Jul 2016 12:49:59 PM CEST, "Daniel P. Berrange" > > wrote: > > > >> GLib >= 2.16 provides GChecksum API which is good enough > >> for md5, sha1, sha256 and sha512. Use this a

Re: [Qemu-block] [PATCH v2] vmdk: fix metadata write regression

2016-07-07 Thread Fam Zheng
On Thu, 07/07 10:42, Reda Sallahi wrote: > Commit "cdeaf1f vmdk: add bdrv_co_write_zeroes" causes a regression on > writes. It writes metadata after every write instead of doing it only once > for each cluster. > > vmdk_pwritev() writes metadata whenever m_data is set as valid so this patch > sets

Re: [Qemu-block] [PATCH v1 3/2] crypto: don't open-code qcrypto_hash_supports

2016-07-07 Thread Alberto Garcia
On Tue 05 Jul 2016 06:43:13 PM CEST, "Daniel P. Berrange" wrote: > Call the existing qcrypto_hash_supports method from > qcrypto_hash_bytesv instead of open-coding it again. > > Signed-off-by: Daniel P. Berrange "PATCH 3/2" ? :-) Reviewed-by: Alberto Garcia Berto

Re: [Qemu-block] [PATCH v1 1/2] crypto: use glib as fallback for hash algorithm

2016-07-07 Thread Alberto Garcia
On Tue 05 Jul 2016 12:49:59 PM CEST, "Daniel P. Berrange" wrote: > +cs = g_checksum_new(qcrypto_hash_alg_map[alg]); > + > +for (i = 0; i < niov; i++) { > +g_checksum_update(cs, iov[i].iov_base, iov[i].iov_len); > +} Not too important, but you could do this after checking the

Re: [Qemu-block] [PATCH v1 2/2] Revert "block: don't register quorum driver if SHA256 support is unavailable"

2016-07-07 Thread Alberto Garcia
On Tue 05 Jul 2016 12:50:00 PM CEST, "Daniel P. Berrange" wrote: > The qcrypto hash APIs now guarantee that sha256 is available at > compile time, so skipping registration is rarely needed. A check > at time of open is kept to ensure good error reporting in the > (unlikely) case sha256 is runtime

[Qemu-block] [PATCH v2] vmdk: fix metadata write regression

2016-07-07 Thread Reda Sallahi
Commit "cdeaf1f vmdk: add bdrv_co_write_zeroes" causes a regression on writes. It writes metadata after every write instead of doing it only once for each cluster. vmdk_pwritev() writes metadata whenever m_data is set as valid so this patch sets m_data as valid only when we have a new cluster whic

[Qemu-block] [PATCH] vmdk: fix metadata write regression

2016-07-07 Thread Reda Sallahi
Commit "cde6361 vmdk: add bdrv_co_write_zeroes" causes a regression on writes. It writes metadata after every write instead of doing it only once for each cluster. vmdk_pwritev() writes metadata whenever m_data is set as valid so this patch sets m_data as valid only when we have a new cluster whic