As you correctly point out, this code needs to be looked at more
carefully so that
if the device does disconnect in the background we can handle the migration path
gracefully. In particular, we need to decide whether a migration
should be allowed
to continue if a device disconnects durning the
On 5/6/20 4:34 PM, Eyal Moscovici wrote:
The mapping operation of large disks especially ones stored over a
long chain of QCOW2 files can take a long time to finish.
Additionally when mapping fails there was no way recover by
restarting the mapping from the failed location.
The new options,
On 5/6/20 4:34 PM, Eyal Moscovici wrote:
The code handles this case correctly we merely skip the loop. However it
is probably best to return an explicit error.
Acked-by: Mark Kanda
Signed-off-by: Eyal Moscovici
---
qemu-img.c | 5 +
1 file changed, 5 insertions(+)
Reviewed-by: Eric
On 5/6/20 4:34 PM, Eyal Moscovici wrote:
All calls to cvtnum check the return value and print the same error message more
or less. And so error reporting moved to cvtnum to reduce duplicate code and
provide a single error message.
Acked-by: Mark Kanda
Signed-off-by: Eyal Moscovici
---
On 5/6/20 4:34 PM, Eyal Moscovici wrote:
Following commit f46bfdbfc8f95cf65d7818ef68a801e063c40332 (util/cutils: Change
qemu_strtosz*() from int64_t to uint64_t) which added a similar check to
cvtnum. As a result there is no need to check it separately outside of cvtnum.
Acked-by: Mark Kanda
On 5/6/20 4:34 PM, Eyal Moscovici wrote:
Hi,
The following series adds two parameters to qemu-img map:
1. start-offset: mapping starting offset.
2. max-length: the length of the mapping.
These parameters proved useful when mapping large disk spread across
long store file chains. It allows us
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
vhdx doesn't have .bdrv_co_block_status handler, so DATA|ALLOCATED is
always assumed for it. unallocated_blocks_are_zero is useless, drop it.
After the analysis I did in patch 1, this is correct. No behavior change.
Reviewed-by: Eric
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
raw_co_block_status() in block/file-posix.c never returns 0, so
unallocated_blocks_are_zero is useless. Drop it.
As in 4/8, you are correct that it had no impact on block_status, but it
did affect qemu-img convert. So again, removing the
The mapping operation of large disks especially ones stored over a
long chain of QCOW2 files can take a long time to finish.
Additionally when mapping fails there was no way recover by
restarting the mapping from the failed location.
The new options, --start-offset and --max-length allows the
All calls to cvtnum check the return value and print the same error message more
or less. And so error reporting moved to cvtnum to reduce duplicate code and
provide a single error message.
Acked-by: Mark Kanda
Signed-off-by: Eyal Moscovici
---
qemu-img.c | 63
The code handles this case correctly we merely skip the loop. However it
is probably best to return an explicit error.
Acked-by: Mark Kanda
Signed-off-by: Eyal Moscovici
---
qemu-img.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/qemu-img.c b/qemu-img.c
index 4a06ab7fe8..a1b507a0be
Previously dump_map_entry identified whether we need to start a new JSON
array based on whether start address == 0. In this refactor we remove
this assumption as in following patches we will allow map to start from
an arbitrary position.
Reviewed-by: Eric Blake
Acked-by: Mark Kanda
Hi,
The following series adds two parameters to qemu-img map:
1. start-offset: mapping starting offset.
2. max-length: the length of the mapping.
These parameters proved useful when mapping large disk spread across
long store file chains. It allows us to bound the execution time of each
qemu-img
Following commit f46bfdbfc8f95cf65d7818ef68a801e063c40332 (util/cutils: Change
qemu_strtosz*() from int64_t to uint64_t) which added a similar check to
cvtnum. As a result there is no need to check it separately outside of cvtnum.
Acked-by: Mark Kanda
Signed-off-by: Eyal Moscovici
---
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
We set bdi->unallocated_blocks_are_zero = iscsilun->lbprz, but
iscsi_co_block_status doesn't return 0 in case of iscsilun->lbprz, it
returns ZERO when appropriate. So actually unallocated_blocks_are_zero
is useless. Drop it now.
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
It's false by default, no needs to set it. We are going to drop this
variable at all, so drop it now here, it doesn't hurt.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/crypto.c | 1 -
1 file changed, 1 deletion(-)
Trivially
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
In case when get_image_offset() returns -1, we do zero out the
corresponding chunk of qiov. So, this should be reported as ZERO.
After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense. Drop
On 5/5/20 12:38 PM, Alberto Garcia wrote:
Extended L2 entries are bigger than normal L2 entries so this has an
impact on the amount of metadata needed for a qcow2 file.
Signed-off-by: Alberto Garcia
Reviewed-by: Max Reitz
---
block/qcow2.c | 19 ---
1 file changed, 12
On 5/5/20 12:38 PM, Alberto Garcia wrote:
This function is only used by qcow2_expand_zero_clusters() to
downgrade a qcow2 image to a previous version. It is however not
possible to downgrade an image with extended L2 entries because older
versions of qcow2 do not have this feature.
On 5/5/20 12:38 PM, Alberto Garcia wrote:
Now that the implementation of subclusters is complete we can finally
add the necessary options to create and read images with this feature,
which we call "extended L2 entries".
Signed-off-by: Alberto Garcia
Reviewed-by: Max Reitz
---
What you have
On 5/5/20 12:38 PM, Alberto Garcia wrote:
Setting the QCOW_OFLAG_ZERO bit of the L2 entry is forbidden if an
image has subclusters. Instead, the individual 'all zeroes' bits must
be used.
Should we s/is forbidden/is ignored/ based on your spec changes?
But the patch itself is right.
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_clusters_unallocated()
and checks individual subclusters.
Maybe
On 5/6/20 12:14 PM, Alberto Garcia wrote:
Note that you are only skipping until the first normal subcluster, even
if other zero/unallocated clusters occur between the first normal
cluster and the start of the action.
That's correct.
Or visually, suppose we have:
On Tue 05 May 2020 11:59:18 PM CEST, Eric Blake wrote:
>> +for (i = first_sc; i <= last_sc; i++) {
>> +unsigned c = i / s->subclusters_per_cluster;
>> +unsigned sc = i % s->subclusters_per_cluster;
>> +l2_entry = get_l2_entry(s, l2_slice, l2_index + c);
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> These calls have no real use for the child role yet, but it will not
> harm to give one.
>
> Notably, the bdrv_root_attach_child() call in blockjob.c is left
> unmodified because there is not much the generic BlockJob object wants
> from its
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> Replace child_file by child_of_bds in all remaining places (excluding
> tests).
>
> Signed-off-by: Max Reitz
> Reviewed-by: Eric Blake
> diff --git a/block/blkreplay.c b/block/blkreplay.c
> index f97493f45a..71628f4d56 100644
> ---
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> Make all parents of backing files pass the appropriate BdrvChildRole.
> By doing so, we can switch their BdrvChildClass over to the generic
> child_of_bds, which will do the right thing when given a correct
> BdrvChildRole.
>
> Signed-off-by:
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
Currently this field only set by qed and qcow2. But in fact, all
backing-supporting formats (parallels, qcow, qcow2, qed, vmdk) share
this semantics: on unallocated blocks, if there is no backing file they
just memset the buffer with zeroes.
06.05.2020 18:14, Eric Blake wrote:
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
Currently this field only set by qed and qcow2.
Well, only after patches 1-6 (prior to then, it was also set in protocol
drivers). I think you might be able to hoist part of this patch earlier in the
On Wed 06 May 2020 05:02:52 PM CEST, Alberto Garcia wrote:
>>> -With version 2, this is always 0.
>>> +With version 2 or with extended L2 entries (see the
>>> next
>>> +section), this is always 0.
>>
>> In your cover letter, you said you
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
Currently this field only set by qed and qcow2.
Well, only after patches 1-6 (prior to then, it was also set in protocol
drivers). I think you might be able to hoist part of this patch earlier
in the series, to make the changes to the
On Tue 05 May 2020 09:35:06 PM CEST, Eric Blake wrote:
> On 5/5/20 12:38 PM, Alberto Garcia wrote:
>> Subcluster allocation in qcow2 is implemented by extending the
>> existing L2 table entries and adding additional information to
>> indicate the allocation status of each subcluster.
>>
>> This
06.05.2020 17:49, Eric Blake wrote:
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
qemu-img convert wants to distinguish ZERO which comes from short
backing files. unallocated_blocks_are_zero field of bdi is unrelated:
space after EOF is always considered to be zero anyway. So, just
06.05.2020 16:57, Eric Blake wrote:
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
In case of !VDI_IS_ALLOCATED[], we do zero out the corresponding chunk
of qiov. So, this should be reported as ZERO.
This part makes sense outright, since vdi does not allow backing files, so all
reads
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
qemu-img convert wants to distinguish ZERO which comes from short
backing files. unallocated_blocks_are_zero field of bdi is unrelated:
space after EOF is always considered to be zero anyway. So, just make
post_backing_zero true in case of
On 5/6/20 4:25 AM, Vladimir Sementsov-Ogievskiy wrote:
In case of !VDI_IS_ALLOCATED[], we do zero out the corresponding chunk
of qiov. So, this should be reported as ZERO.
This part makes sense outright, since vdi does not allow backing files,
so all reads of a VDI disk result in data rather
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> This callback can be used by BDSs that use child_of_bds with the
> appropriate BdrvChildRole for their children.
>
> Also, make bdrv_format_default_perms() use it for child_of_bds children
> (just a temporary solution until we can drop
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> Right now, bdrv_format_default_perms() is used by format parents
> (generally). We want to switch to a model where most parents use a
> single BdrvChildClass, which then decides the permissions based on the
> child role. To do so, we have to
Am 06.05.2020 um 12:37 hat Kevin Wolf geschrieben:
> Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> > After the series this patch belongs to, we want to have a common
> > BdrvChildClass that encompasses all of child_file, child_format, and
> > child_backing. Such a single class needs a
On Tue, 5 May 2020 at 13:58, Max Reitz wrote:
>
> The following changes since commit 5375af3cd7b8adcc10c18d8083b7be63976c9645:
>
> Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging
> (2020-05-04 15:51:09 +0100)
>
> are available in the Git repository at:
>
>
On 4/28/20 3:28 PM, Eric Blake wrote:
Based-on: <20200424125448.63318-1-kw...@redhat.com>
[PATCH v7 00/10] block: Fix resize (extending) of short overlays
After reviewing Kevin's work, I questioned if we had a redundancy with
bdrv_has_zero_init_truncate. It turns out we do, and this is the
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> Any current user of child_file, child_format, and child_backing can and
> should use this generic BdrvChildClass instead, as it can handle all of
> these cases. However, to be able to do so, the users must pass the
> appropriate BdrvChildRole
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> Make bdrv_child_cb_detach() call bdrv_backing_detach() for children with
> a COW role (and drop the reverse call from bdrv_backing_detach()), so it
> can be used for any child (with a proper role set).
>
> Because so far no child has a proper
On Wed, 6 May 2020 10:39:02 +0200
Gerd Hoffmann wrote:
> Hi,
>
> > > crs = aml_resource_template();
> > > aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE,
> > > - 0x10, 0x02));
> > > + 0x10, 0x08));
> > >
On Tue 05 May 2020 10:04:32 PM CEST, Eric Blake wrote:
> What happens for an image whose size is not cluster-aligned? Must the
> bits corresponding to subclusters not present in the final cluster
> always be zero, or are they instead ignored regardless of value?
Attempting to read or write
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben:
> After the series this patch belongs to, we want to have a common
> BdrvChildClass that encompasses all of child_file, child_format, and
> child_backing. Such a single class needs a single .inherit_options()
> implementation, and this patch
On Tue 05 May 2020 09:42:08 PM CEST, Eric Blake wrote:
> On 5/5/20 12:38 PM, Alberto Garcia wrote:
>> Like offset_into_cluster() and size_to_clusters(), but for
>> subclusters.
>>
>> Signed-off-by: Alberto Garcia
>> ---
>> block/qcow2.h | 10 ++
>> 1 file changed, 10 insertions(+)
>>
On 29/04/2020, 17:58, "Eric Blake" wrote:
On 3/22/20 4:11 AM, Eyal Moscovici wrote:
> Previously dump_map_entry identified whether we need to start a new JSON
> array based on whether start address == 0. In this refactor we remove
> this assumption as in following patches we
Thanks for the review. I will send V2 based on QEMU version 5.0.
On 29/04/2020, 18:06, "Eric Blake" wrote:
On 3/22/20 4:11 AM, Eyal Moscovici wrote:
> The mapping operation of large disks especially ones stored over a
> long chain of QCOW2 files can take a long time to finish.
On Thu, Apr 30, 2020 at 3:37 PM Dima Stepanov wrote:
>
> During testing of the vhost-user-blk reconnect functionality the qemu
> SIGSEGV was triggered:
> start qemu as:
> x86_64-softmmu/qemu-system-x86_64 -m 1024M -M q35 \
>-object
>
raw_co_block_status() in block/file-posix.c never returns 0, so
unallocated_blocks_are_zero is useless. Drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/file-posix.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index
qemu-img convert wants to distinguish ZERO which comes from short
backing files. unallocated_blocks_are_zero field of bdi is unrelated:
space after EOF is always considered to be zero anyway. So, just make
post_backing_zero true in case of short backing file.
Signed-off-by: Vladimir
It's false by default, no needs to set it. We are going to drop this
variable at all, so drop it now here, it doesn't hurt.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/crypto.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/block/crypto.c b/block/crypto.c
index
Currently this field only set by qed and qcow2. But in fact, all
backing-supporting formats (parallels, qcow, qcow2, qed, vmdk) share
this semantics: on unallocated blocks, if there is no backing file they
just memset the buffer with zeroes.
So, document this behavior for .supports_backing and
vhdx doesn't have .bdrv_co_block_status handler, so DATA|ALLOCATED is
always assumed for it. unallocated_blocks_are_zero is useless, drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/vhdx.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/block/vhdx.c b/block/vhdx.c
index
In case when get_image_offset() returns -1, we do zero out the
corresponding chunk of qiov. So, this should be reported as ZERO.
After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense. Drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
We set bdi->unallocated_blocks_are_zero = iscsilun->lbprz, but
iscsi_co_block_status doesn't return 0 in case of iscsilun->lbprz, it
returns ZERO when appropriate. So actually unallocated_blocks_are_zero
is useless. Drop it now.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/iscsi.c | 1
In case of !VDI_IS_ALLOCATED[], we do zero out the corresponding chunk
of qiov. So, this should be reported as ZERO.
After block-status update, it never reports 0, so setting
unallocated_blocks_are_zero doesn't make sense. Drop it.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/vdi.c |
Hi all!
This is first step to block-status refactoring, and solves most simple
problem mentioned in my investigation of block-status described in
the thread "backing chain & block status & filters":
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg04706.html
Thanks,
Feng Li
Dima Stepanov 于2020年4月30日周四 下午9:36写道：
>
> During testing of the vhost-user-blk reconnect functionality the qemu
> SIGSEGV was triggered:
> start qemu as:
> x86_64-softmmu/qemu-system-x86_64 -m 1024M -M q35 \
>-object
>
On Sun, May 03, 2020 at 08:36:45PM -0400, Raphael Norwitz wrote:
> I’m happy from the vhost, vhost-user-blk and vhost-user-scsi side. For
> other device types it looks pretty straightforward, but their maintainers
> should probably confirm.
>
> Since you plan to change the behavior of these
On Sun, May 03, 2020 at 09:06:38PM -0400, Raphael Norwitz wrote:
> Apologies for mixing up patches last time. This looks good from a
> vhost-user-blk perspective, but I worry that some of these changes
> could impact other vhost device types.
>
> I agree with adding notifiers_set to struct
> > +static void parallel_isa_build_aml(ISADevice *isadev, Aml *scope)
> > +{
> > +ISAParallelState *isa = ISA_PARALLEL(isadev);
> > +int i, uid = 0;
> > +Aml *dev;
> > +Aml *crs;
> > +
> > +for (i = 0; i < ARRAY_SIZE(isa_parallel_io); i++) {
> > +if (isa->iobase ==
Hi,
> > crs = aml_resource_template();
> > aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE,
> > - 0x10, 0x02));
> > + 0x10, 0x08));
> > aml_append(crs, aml_irq_no_flags(RTC_ISA_IRQ));
> > -aml_append(crs,
06.05.2020 11:01, Denis Plotnikov wrote:
On 05.05.2020 15:03, Max Reitz wrote:
On 05.05.20 12:26, Max Reitz wrote:
On 30.04.20 12:19, Denis Plotnikov wrote:
v23:
Undecided: whether to add zstd(zlib) compression
details to the qcow2 spec
03: tighten assertion on zstd
06.05.2020 11:02, Kevin Wolf wrote:
Am 06.05.2020 um 08:07 hat Vladimir Sementsov-Ogievskiy geschrieben:
05.05.2020 13:03, Kevin Wolf wrote:
Am 30.04.2020 um 20:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
30.04.2020 17:27, Kevin Wolf wrote:
Since the introduction of a backup filter node
Am 06.05.2020 um 08:07 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 05.05.2020 13:03, Kevin Wolf wrote:
> > Am 30.04.2020 um 20:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > 30.04.2020 17:27, Kevin Wolf wrote:
> > > > Since the introduction of a backup filter node in commit 00e30f05d,
On 05.05.2020 15:03, Max Reitz wrote:
On 05.05.20 12:26, Max Reitz wrote:
On 30.04.20 12:19, Denis Plotnikov wrote:
v23:
Undecided: whether to add zstd(zlib) compression
details to the qcow2 spec
03: tighten assertion on zstd decompression [Eric]
04: use
27.04.2020 17:39, Vladimir Sementsov-Ogievskiy wrote:
It's safer to expand in_flight request to start before enter to
coroutine in synchronous wrappers, due to the following (theoretical)
problem:
Consider write.
It's possible, that qemu_coroutine_enter only schedules execution,
assume such
30.04.2020 14:10, 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
30.04.2020 23:51, no-re...@patchew.org wrote:
Patchew URL:
https://patchew.org/QEMU/20200430111033.29980-1-vsement...@virtuozzo.com/
Hi,
This series failed the docker-quick@centos7 build test. Please find the testing
commands and
their output below. If you have Docker installed, you can
04.05.2020 19:32, Kevin Wolf wrote:
Am 30.04.2020 um 14:47 hat Vladimir Sementsov-Ogievskiy geschrieben:
Hi all!
This series adds a bit more support for iotests skipping due to format
whitelisting. Not pretend to be something complete. It just lay in its
folder I don't know how much time, I
04.05.2020 22:59, Eric Blake wrote:
On 4/21/20 4:56 PM, Eric Blake wrote:
On 4/1/20 10:01 AM, Vladimir Sementsov-Ogievskiy wrote:
NBD spec is updated, so that max_block doesn't relate to
NBD_CMD_TRIM. So, drop the restriction.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/nbd.c | 2
05.05.2020 13:03, Kevin Wolf wrote:
Am 30.04.2020 um 20:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
30.04.2020 17:27, Kevin Wolf wrote:
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
74 matches
Mail list logo
