On 27.10.2020 19:21, Vladimir Sementsov-Ogievskiy wrote:
27.10.2020 19:01, Andrey Shinkevich wrote:
On 27.10.2020 18:09, Vladimir Sementsov-Ogievskiy wrote:
22.10.2020 21:13, Andrey Shinkevich wrote:
Avoid writing a filter JSON file name and a filter format name to QCOW2
image when the backing file is changed after the block stream job.
A user is still able to assign the 'backing-file' parameter for a
block-stream job keeping in mind the possible issue mentioned above.
If the user does not specify the 'backing-file' parameter, QEMU will
assign it automatically.
Signed-off-by: Andrey Shinkevich <andrey.shinkev...@virtuozzo.com>
---
block/stream.c | 15 +++++++++++++--
blockdev.c | 9 ++-------
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/block/stream.c b/block/stream.c
index e0540ee..1ba74ab 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -65,6 +65,7 @@ static int stream_prepare(Job *job)
BlockDriverState *bs = blk_bs(bjob->blk);
BlockDriverState *unfiltered_bs = bdrv_skip_filters(bs);
BlockDriverState *base = bdrv_filter_or_cow_bs(s->above_base);
+ BlockDriverState *base_unfiltered = NULL;
Error *local_err = NULL;
int ret = 0;
@@ -75,8 +76,18 @@ static int stream_prepare(Job *job)
const char *base_id = NULL, *base_fmt = NULL;
if (base) {
base_id = s->backing_file_str;
- if (base->drv) {
- base_fmt = base->drv->format_name;
+ if (base_id) {
+ if (base->drv) {
+ base_fmt = base->drv->format_name;
hmm. this doesn't make real sense: so, we assume that user specified
backing_file_str, which may not relate to base, but we use
base->drv->format_name? But it may be name of the filter driver,
which would be wrong..
Any ideas?
1. we can use base_fmt=NULL, to provoke probing on next open of the
qcow2 file..
I would choose this item #1 but have to check the probing code
logic... Particularly, I do not remember now if the probing is able to
recognize a protocol.
The logic for the format_name in the QEMU existent code (I has kept it
here in the patch) is a slippery way for an imprudent user. That's why
I staked on the backing_file_str deprication in the previous version.
2. we can do probing now
3. we can at least check, if backing_file_str ==
Not bad for the sanity check but we will search a node by the file
name again - not good ((
Not search, but only check one very likely option.
Yes, just strcmp(). And why a user may not merely specify a desired
backing file as the base?
Additionally to 1. or 3. (or combined), or even keeping things as is
(i.e. wrong, but it is preexisting), we can:
- add backing-format argument to qapi as pair for backing-file
- deprecate using backing-file without backing-format.
Then, after deprecation period we'll have correct code. This may be done
in separate.
base_unfiltered->filename, in this case we can use
base_unfiltered->drv->format_name
+ }
+ } else {
+ base_unfiltered = bdrv_skip_filters(base);
+ if (base_unfiltered) {
+ base_id = base_unfiltered->filename;
+ if (base_unfiltered->drv) {
+ base_fmt = base_unfiltered->drv->format_name;
+ }
+ }
}
}
bdrv_set_backing_hd(unfiltered_bs, base, &local_err);
diff --git a/blockdev.c b/blockdev.c
index c917625..0e9c783 100644
--- a/blockdev.c
+++ b/blockdev.c
[...]
- stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name,
+ stream_start(has_job_id ? job_id : NULL, bs, base_bs,
+ has_backing_file ? backing_file : NULL,
backing_file should be NULL if has_backing_file is false, so you can
use just backing_file instead of ternary operator.
Yes, if reliable. I has kept the conformation with the ternary
operator at the first parameter above.
Andrey
job_flags, has_speed ? speed : 0, on_error,
filter_node_name, &local_err);
if (local_err) {
