On 07.05.20 20:38, Vladimir Sementsov-Ogievskiy wrote:
> Obviously, we should g_free the task after trace point and offset
> update.
>
> Reported-by: Coverity
> Fixes: 4ce5dd3e9b5ee0fac18625860eb3727399ee965e
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
>
> Be free to add Coverity number
Philippe Mathieu-Daudé wrote:
> Rename qemu_ram_writeback() as qemu_ram_msync() to better
> match what it does.
>
> Suggested-by: Stefan Hajnoczi
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Juan Quintela
if block people preffers that name, who I am to complain.
Philippe Mathieu-Daudé wrote:
> The first field of RAMBlock is a rcu_head structure.
> We need to include the "qemu/rcu.h" to avoid errors when
> including "exec/ramblock.h" without "qemu/rcu.h":
>
> include/exec/ramblock.h:27:21: error: field ‘rcu’ has incomplete type
> 27 | struct rcu
Philippe Mathieu-Daudé wrote:
> Move these generic functions to a more common place, with other
> functions related to host page size. Document a little.
>
> Cc: Alexey Kardashevskiy
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Juan Quintela
Philippe Mathieu-Daudé wrote:
> The CONFIG_SOFTMMU definition is poisoned in "exec/poison.h".
> As this header is internal, check for poisoned CONFIG_SOFTMMU
> instead of CONFIG_USER_ONLY to make this include harder to use.
>
> Signed-off-by: Philippe Mathieu-Daudé >
Reviewed-by: Juan Quintela
Philippe Mathieu-Daudé wrote:
> The ramblock_recv_bitmap_offset() function is only used once
> in migration/ram.c, move it there.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Juan Quintela
But, if you are in the mood:
> }
>
> +static inline unsigned long int ramblock_recv_bitmap_
Philippe Mathieu-Daudé wrote:
> The RAMBlock API was dispersed in 3 different headers.
> One of these headers, "exec/ram_addr.h", is restricted
> to target dependent code. However these functions are
> not target specific. Move all functions into a single
> place. Now all these functions can be a
Philippe Mathieu-Daudé wrote:
> Now than qemu_ram_msync() has been ...
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Juan Quintela
Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé
Once here, can someone give a test that meassures if it makes sense that
this functions are inline?
They are relatively big, and I would preffer them to be in a .c file,
but I have no clue about how performance critical they a
Paolo Bonzini wrote:
> On 07/05/20 19:39, Philippe Mathieu-Daudé wrote:
>> Stefan suggested to make qemu_ram_writeback() target agnostic,
>> Paolo to add memory_region_msync(), and Peter to remove
>> "exec/ram_addr.h" [*].
>>
>> I let a single function in this file,
>> cpu_physical_memory_sync_di
Am 07.05.2020 um 19:43 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 21.04.2020 19:56, Kevin Wolf wrote:
> > Am 21.04.2020 um 09:35 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > Add python script with new logic of searching for tests:
> > >
> > > Current ./check behavior:
> > > - tests a
Philippe Mathieu-Daudé wrote:
> We will move this code in the next commit. Clean it up
> first to avoid checkpatch.pl errors.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Juan Quintela
On Thu, 7 May 2020 19:39:53 +0200
Philippe Mathieu-Daudé wrote:
> Move these generic functions to a more common place, with other
> functions related to host page size. Document a little.
>
> Cc: Alexey Kardashevskiy
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/exec/ram_addr.h
On 5/8/20 10:07 AM, Juan Quintela wrote:
Philippe Mathieu-Daudé wrote:
The ramblock_recv_bitmap_offset() function is only used once
in migration/ram.c, move it there.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Juan Quintela
But, if you are in the mood:
}
+static inline unsi
On Thu, 7 May 2020 19:39:55 +0200
Philippe Mathieu-Daudé wrote:
> The RAMBlock API was dispersed in 3 different headers.
> One of these headers, "exec/ram_addr.h", is restricted
> to target dependent code. However these functions are
> not target specific. Move all functions into a single
> plac
08.05.2020 11:49, Kevin Wolf wrote:
Am 07.05.2020 um 19:43 hat Vladimir Sementsov-Ogievskiy geschrieben:
21.04.2020 19:56, Kevin Wolf wrote:
Am 21.04.2020 um 09:35 hat Vladimir Sementsov-Ogievskiy geschrieben:
Add python script with new logic of searching for tests:
Current ./check behavior:
Am 30.04.2020 um 16:50 hat Eric Blake geschrieben:
> On 4/30/20 8:59 AM, Max Reitz wrote:
> > On 21.04.20 23:20, Eric Blake wrote:
> > > Upcoming patches want to add some basic bitmap manipulation abilities
> > > to qemu-img. But blockdev.o is too heavyweight to link into qemu-img
> > > (among oth
On Wed 06 May 2020 07:55:29 PM CEST, Eric Blake wrote:
> On 5/5/20 12:38 PM, Alberto Garcia wrote:
>> The logic of this function remains pretty much the same, except that
>> it uses count_contiguous_subclusters(), which combines the logic of
>> count_contiguous_clusters() / count_contiguous_cluster
From: Vladimir Sementsov-Ogievskiy
Drop check for no block-jobs: it's obvious that there no jobs
immediately after vm.launch().
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20200430124713.3067-5-vsement...@virtuozzo.com>
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/041 | 8 ---
The following changes since commit 1b8c45899715d292398152ba97ef755ccaf84680:
Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20200507a'
into staging (2020-05-07 18:43:20 +0100)
are available in the Git repository at:
git://repo.or.cz/qemu/kevin.git tags/for-upstream
for
From: Vladimir Sementsov-Ogievskiy
Test fails if bochs not whitelisted, so, skip it in this case.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20200430124713.3067-3-vsement...@virtuozzo.com>
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/082 | 1 +
1 file changed, 1 insertion(+)
From: Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20200430124713.3067-7-vsement...@virtuozzo.com>
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/055 | 4
1 file changed, 4 insertions(+)
diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/0
From: Vladimir Sementsov-Ogievskiy
Some tests requires O_DIRECT, or want it by default. Introduce smarter
O_DIRECT handling:
- Check O_DIRECT in common.rc, if it is requested by selected
cache-mode.
- Support second fall-through argument in _default_cache_mode
Inspired-by: Max's 23e1d054112cec
If a cluster is already zeroed, we don't have to call vmdk_L2update(),
which is rather slow because it flushes the image file.
Signed-off-by: Kevin Wolf
Message-Id: <20200430133007.170335-5-kw...@redhat.com>
Reviewed-by: Eric Blake
Signed-off-by: Kevin Wolf
---
block/vmdk.c | 2 +-
1 file chan
m_data must contain valid data even for zero clusters when no cluster
was allocated in the image file. Without this, zero writes segfault with
images that have zeroed_grain=on.
For zero writes, we don't want to allocate a cluster in the image file
even in compressed files.
Fixes: 524089bce43fd1cd
From: Vladimir Sementsov-Ogievskiy
Skip test-case with quorum if quorum is not whitelisted.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20200430124713.3067-4-vsement...@virtuozzo.com>
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/148 | 1 +
1 file changed, 1 insertion(+)
diff
From: Vladimir Sementsov-Ogievskiy
Instead of looping in each test, let's better refactor vmdk target case
as a subclass.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20200430124713.3067-6-vsement...@virtuozzo.com>
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/055 | 70
From: Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20200430124713.3067-9-vsement...@virtuozzo.com>
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/113 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/113 b/test
When overwriting a zero cluster, we must not perform copy-on-write from
the backing file, but from a zeroed buffer.
Signed-off-by: Kevin Wolf
Message-Id: <20200430133007.170335-4-kw...@redhat.com>
Reviewed-by: Eric Blake
Signed-off-by: Kevin Wolf
---
block/vmdk.c | 18 --
1 fil
m_data is used for zero clusters even though valid == 0. It really only
means that a new cluster was allocated in the image file. Rename it to
reflect this.
While at it, change it from int to bool, too.
Signed-off-by: Kevin Wolf
Message-Id: <20200430133007.170335-2-kw...@redhat.com>
Reviewed-by:
From: Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy
Message-Id: <20200430124713.3067-8-vsement...@virtuozzo.com>
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/109 | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
i
This tests that the backup job catches situations where the target node
has a different size than the source node. It must also forbid resize
operations when the job is already running.
Signed-off-by: Kevin Wolf
Message-Id: <20200430142755.315494-5-kw...@redhat.com>
Reviewed-by: Vladimir Sementso
From: Eric Blake
Our .bdrv_has_zero_init_truncate always returns 1 because rbd always
0-fills; we can use that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it.
Signed-off-by: Eric Blake
Message-Id: <20200428202905.770727-5-ebl...@redhat.com>
Signed-off-by: Kevin Wolf
---
block/
From: Eric Blake
When using bdrv_file, .bdrv_has_zero_init_truncate always returns 1;
therefore, we can behave just like file-posix, and always implement
BDRV_REQ_ZERO_WRITE by ignoring it since the OS gives it to us for
free (note that file-posix.c had to use an 'if' because it shared code
betwe
From: Alberto Garcia
After commit f01643fb8b47e8a70c04bbf45e0f12a9e5bc54de when an image is
extended and BDRV_REQ_ZERO_WRITE is set then the new clusters are
zeroized.
The code however does not detect correctly situations when the old and
the new end of the image are within the same cluster. The
bdrv_get_device_name() will be an empty string with modern management
tools that don't use -drive. Use bdrv_get_device_or_node_name() instead
so that the node name is used if the BlockBackend is anonymous.
While at it, start with upper case to make the message consistent with
the rest of the funct
The test case forgot to specify the null-co size for the target node.
When adding a check to backup that both sizes match, this would fail
because of the size mismatch and not the behaviour that the test really
wanted to test.
Fixes: a541fcc27c98b96da187c7d4573f3270f3ddd283
Signed-off-by: Kevin Wo
From: Eric Blake
Our .bdrv_has_zero_init_truncate always returns 1 because sheepdog
always 0-fills; we can use that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it.
Signed-off-by: Eric Blake
Message-Id: <20200428202905.770727-6-ebl...@redhat.com>
Signed-off-by: Kevin Wolf
---
b
Since the introduction of a backup filter node in commit 00e30f05d, the
backup block job crashes when the target image is smaller than the
source image because it will try to write after the end of the target
node without having BLK_PERM_RESIZE. (Previously, the BlockBackend layer
would have caught
If we have a backup L2 table, we currently flush once after writing to
the active L2 table and again after writing to the backup table. A
single flush is enough and makes things a little less slow.
Signed-off-by: Kevin Wolf
Message-Id: <20200430133007.170335-6-kw...@redhat.com>
Reviewed-by: Eric
From: Eric Blake
Our .bdrv_has_zero_init_truncate returns 1 if we detect that the OS
always 0-fills; we can use that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it when the OS gives it to us for
free.
Signed-off-by: Eric Blake
Message-Id: <20200428202905.770727-4-ebl...@redhat.c
From: Max Reitz
Calling bdrv_getlength() to get the pre-truncate file size will not
really work on block devices, because they have always the same length,
and trying to write beyond it will fail with a rather cryptic error
message.
Instead, we should use qcow2_get_last_cluster() and bdrv_getlen
From: Eric Blake
The vhdx driver uses truncation for image growth, with a special case
for blocks that already read as zero but which are only being
partially written. But with a bit of rearranging, it's just as easy
to defer the decision on whether truncation resulted in zeroes to the
actual al
From: Eric Blake
Now that there are no clients of bdrv_has_zero_init_truncate, none of
the drivers need to worry about providing it.
What's more, this eliminates a source of some confusion: a literal
reading of the documentation as written in ceaca56f and implemented in
commit 1dcaf527 claims th
Philippe Mathieu-Daudé writes:
> The CPU() macro is defined as:
>
> #define CPU(obj) ((CPUState *)(obj))
>
> which expands to:
>
> ((CPUState *)object_dynamic_cast_assert((Object *)(obj), (name),
> __FILE__, __LINE__, __func__))
>
> This assertion can
Philippe Mathieu-Daudé writes:
> The OBJECT() macro is defined as:
>
> #define OBJECT(obj) ((Object *)(obj))
>
> which expands to:
>
> ((Object *)object_dynamic_cast_assert((Object *)(obj), (name),
> __FILE__, __LINE__, __func__))
Nope :)
> This asser
055 uses the backup block job to create a compressed backup of an
$IMGFMT image with both qcow2 and vmdk targets. However, cluster
allocation in vmdk is very slow because it flushes the image file after
each L2 update.
There is no reason why we need this level of safety in this test, so
let's disa
In order to avoid bitrot in the zero cluster code in VMDK, enable
zeroed_grain=on by default for the tests.
059 now unsets the default options because zeroed_grain=on works only
with some subformats and the test case tests many different subformats,
including those for which it doesn't work.
Sign
From: Eric Blake
The parallels driver tries to use truncation for image growth, but can
only do so when reads are guaranteed as zero. Now that we have a way
to request zero contents from truncation, we can defer the decision to
actual allocation attempts rather than up front, reducing the number
From: Eric Blake
Our .bdrv_has_zero_init_truncate can detect when the remote side
always zero fills; we can reuse that same knowledge to implement
BDRV_REQ_ZERO_WRITE by ignoring it when the server gives it to us for
free.
Signed-off-by: Eric Blake
Message-Id: <20200428202905.770727-7-ebl...@re
From: Eric Blake
block.c already defaults to 0 if we don't provide a callback; there's
no need to write a callback that always fails.
Signed-off-by: Eric Blake
Reviewed-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Alberto Garcia
Message-Id: <20200428202905.770727-2-ebl...@redhat.com>
Signed-
Philippe Mathieu-Daudé writes:
> The DEVICE() macro is defined as:
>
> #define DEVICE(obj) OBJECT_CHECK(DeviceState, (obj), TYPE_DEVICE)
>
> which expands to:
>
> ((DeviceState *)object_dynamic_cast_assert((Object *)(obj), (name),
> __FILE__, __LIN
On 5/8/20 2:49 PM, Markus Armbruster wrote:
Philippe Mathieu-Daudé writes:
The OBJECT() macro is defined as:
#define OBJECT(obj) ((Object *)(obj))
which expands to:
((Object *)object_dynamic_cast_assert((Object *)(obj), (name),
__FILE__, __LINE
On 5/8/20 6:37 AM, Kevin Wolf wrote:
---
Makefile.objs | 2 +-
include/sysemu/blockdev.h | 14 ++
blockbitmaps.c| 324 ++
Hm. Can we get a better name? blockdev-bitmaps.c, for example?
Sure, I'm open to bike-shed suggesti
On Fri, 8 May 2020 at 13:41, Kevin Wolf wrote:
>
> The following changes since commit 1b8c45899715d292398152ba97ef755ccaf84680:
>
> Merge remote-tracking branch
> 'remotes/dgilbert/tags/pull-migration-20200507a' into staging (2020-05-07
> 18:43:20 +0100)
>
> are available in the Git repository
On Fri, 8 May 2020 at 07:33, Paolo Bonzini wrote:
>
> On 08/05/20 08:24, Philippe Mathieu-Daudé wrote:
> > It is not clear if dccvap_writefn() really needs
> > memory_region_writeback() or could use memory_region_msync().
>
> Indeed, I don't understand the code and why it matters that
> mr->dirty_
On 5/8/20 5:33 PM, Paolo Bonzini wrote:
On 08/05/20 17:20, Beata Michalska wrote:
mr->dirty_log_mask tells if dirty tracking has been enabled, not if the
page is dirty. It would always be true during live migration and when
running on TCG, but otherwise it would always be false.
Beata, can yo
On 08/05/20 17:20, Beata Michalska wrote:
>>
>> mr->dirty_log_mask tells if dirty tracking has been enabled, not if the
>> page is dirty. It would always be true during live migration and when
>> running on TCG, but otherwise it would always be false.
>>
>> Beata, can you explain what you had in m
On Fri, May 8, 2020 at 5:34 PM Philippe Mathieu-Daudé wrote:
>
> On 5/8/20 5:33 PM, Paolo Bonzini wrote:
> > On 08/05/20 17:20, Beata Michalska wrote:
> >>>
> >>> mr->dirty_log_mask tells if dirty tracking has been enabled, not if the
> >>> page is dirty. It would always be true during live migra
On 08/05/20 17:35, Philippe Mathieu-Daudé wrote:
>> So I understand I can simply rename memory_region_writeback ->
>> memory_region_msync in patch #2.
>
> (removing the mr->dirty_log_mask).
>
Yes.
Paolo
v2 was here, to see the original cover letter:
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg03464.html
Since then:
- patch 2 was previously posted in isolation but fits well in this series
- patch 3 is new
- patch 4 and 5 were split from v2:2/6 [Max]
- new filename chosen in patch 5 is
Missed in commit e13c59fa.
Signed-off-by: Eric Blake
---
qemu-img.c | 2 +-
qemu-img-cmds.hx | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 6a4327aaba56..b6e8af9202a5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -108,7 +108,7 @@ static v
We already list the subcommand summaries alphabetically, we should do
the same for the documentation related to subcommand-specific
parameters.
Signed-off-by: Eric Blake
Reviewed-by: Max Reitz
---
docs/tools/qemu-img.rst | 48 -
1 file changed, 24 inserti
The next patch will split blockdev.c, which will require accessing
some previously-static functions from more than one .c file. But part
of promoting a function to public is picking a naming scheme that does
not reek of exposing too many internals (two of the three functions
were named starting wi
Upcoming patches will enhance bitmap support in qemu-img, but in doing
so, it turns out to be nice to suppress output when bitmaps make no
sense (such as on a qcow2 v2 image). Add a hook to make this easier
to query.
In the future, when we improve the ability to look up bitmaps through
a filter,
It's useful to know how much space can be occupied by qcow2 persistent
bitmaps, even though such metadata is unrelated to the guest-visible
data. Report this value as an additional field, present when
measuring an existing image and the output format supports bitmaps.
Update iotest 178 and 190 to
Include actions for --add, --remove, --clear, --enable, --disable, and
--merge (note that --clear is a bit of fluff, because the same can be
accomplished by removing a bitmap and then adding a new one in its
place, but it matches what QMP commands exist). Listing is omitted,
because it does not re
Make it easier to copy all the persistent bitmaps of (the top layer
of) a source image along with its guest-visible contents, by adding a
boolean flag for use with qemu-img convert. This is basically
shorthand, as the same effect could be accomplished with a series of
'qemu-img bitmap --add' and '
Add a new test covering the 'qemu-img bitmap' subcommand, as well as
'qemu-img convert --bitmaps', both added in recent patches.
Signed-off-by: Eric Blake
---
tests/qemu-iotests/291 | 112 +
tests/qemu-iotests/291.out | 78 ++
test
Upcoming patches want to add some basic bitmap manipulation abilities
to qemu-img. But blockdev.o is too heavyweight to link into qemu-img
(among other things, it would drag in block jobs and transaction
support - qemu-img does offline manipulation, where atomicity is less
important because there
On 4/30/20 6:10 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be
On 5/7/20 11:24 PM, Philippe Mathieu-Daudé wrote:
> We usually use '_do_' for internal functions. Rename
> memory_region_do_writeback() as memory_region_writeback().
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/exec/memory.h | 4 ++--
> memory.c | 2 +-
> target/arm/hel
On 5/7/20 11:24 PM, Philippe Mathieu-Daudé wrote:
> Now than the non-target specific memory_region_msync() function
> is available, use it to make this device target-agnostic.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/block/nvme.c| 6 ++
> hw/block/Makefile.objs | 2 +-
>
On 5/7/20 11:24 PM, Philippe Mathieu-Daudé wrote:
> Rename qemu_ram_writeback() as qemu_ram_msync() to better
> match what it does.
>
> Suggested-by: Stefan Hajnoczi
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/exec/ram_addr.h | 4 ++--
> exec.c | 2 +-
> memory.c
On 4/30/20 6:10 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be
75 matches
Mail list logo