| Message ID | 1587407806-109784-6-git-send-email-andrey.shinkevich@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Apply COR-filter to the block-stream permanently | expand |
On 4/20/20 1:36 PM, Andrey Shinkevich wrote: > Provide the possibility to pass the 'filter-node-name' parameter to the > block-stream job as it is done for the commit block job. > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > --- > block/stream.c | 5 +++-- > blockdev.c | 8 +++++++- > include/block/block_int.h | 7 ++++++- > monitor/hmp-cmds.c | 4 ++-- > qapi/block-core.json | 6 ++++++ > 5 files changed, 24 insertions(+), 6 deletions(-) > I haven't reviewed the mechanics of this series yet, but from the high-level UI perspective: > +++ b/qapi/block-core.json > @@ -2552,6 +2552,11 @@ > # 'stop' and 'enospc' can only be used if the block device > # supports io-status (see BlockInfo). Since 1.3. > # > +# @filter-node-name: the node name that should be assigned to the > +# filter driver that the stream job inserts into the graph > +# above @device. If this option is not given, a node name is > +# autogenerated. (Since: 5.0) This should be 5.1, not 5.0. > +# > # @auto-finalize: When false, this job will wait in a PENDING state after it has > # finished its work, waiting for @block-job-finalize before > # making any block graph changes. > @@ -2581,6 +2586,7 @@ > 'data': { '*job-id': 'str', 'device': 'str', '*base': 'str', > '*base-node': 'str', '*backing-file': 'str', '*speed': 'int', > '*on-error': 'BlockdevOnError', > + '*filter-node-name': 'str', > '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } } > > ## >
* Andrey Shinkevich (andrey.shinkevich@virtuozzo.com) wrote: > Provide the possibility to pass the 'filter-node-name' parameter to the > block-stream job as it is done for the commit block job. > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > --- > block/stream.c | 5 +++-- > blockdev.c | 8 +++++++- > include/block/block_int.h | 7 ++++++- > monitor/hmp-cmds.c | 4 ++-- > qapi/block-core.json | 6 ++++++ > 5 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/block/stream.c b/block/stream.c > index d8b4bbe..fab7923 100644 > --- a/block/stream.c > +++ b/block/stream.c > @@ -229,7 +229,9 @@ static const BlockJobDriver stream_job_driver = { > void stream_start(const char *job_id, BlockDriverState *bs, > BlockDriverState *base, const char *backing_file_str, > int creation_flags, int64_t speed, > - BlockdevOnError on_error, Error **errp) > + BlockdevOnError on_error, > + const char *filter_node_name, > + Error **errp) > { > StreamBlockJob *s; > BlockDriverState *iter; > @@ -265,7 +267,6 @@ void stream_start(const char *job_id, BlockDriverState *bs, > } else { > bdrv_unfreeze_chain(bottom_cow_node, above_base); > } > - > /* Prevent concurrent jobs trying to modify the graph structure here, we > * already have our own plans. Also don't allow resize as the image size is > * queried only at the job start and then cached. */ > diff --git a/blockdev.c b/blockdev.c > index 72d28ce..f275a64 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3242,6 +3242,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, > bool has_backing_file, const char *backing_file, > bool has_speed, int64_t speed, > bool has_on_error, BlockdevOnError on_error, > + bool has_filter_node_name, const char *filter_node_name, > bool has_auto_finalize, bool auto_finalize, > bool has_auto_dismiss, bool auto_dismiss, > Error **errp) > @@ -3257,6 +3258,10 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, > on_error = BLOCKDEV_ON_ERROR_REPORT; > } > > + if (!has_filter_node_name) { > + filter_node_name = NULL; > + } > + > bs = bdrv_lookup_bs(device, device, errp); > if (!bs) { > return; > @@ -3324,7 +3329,8 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, > } > > stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name, > - job_flags, has_speed ? speed : 0, on_error, &local_err); > + job_flags, has_speed ? speed : 0, on_error, > + filter_node_name, &local_err); > if (local_err) { > error_propagate(errp, local_err); > goto out; > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 993bafc..5ac4891 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -1052,6 +1052,9 @@ int is_windows_drive(const char *filename); > * See @BlockJobCreateFlags > * @speed: The maximum speed, in bytes per second, or 0 for unlimited. > * @on_error: The action to take upon error. > + * @filter_node_name: The node name that should be assigned to the filter > + * driver that the commit job inserts into the graph above @bs. NULL means > + * that a node name should be autogenerated. > * @errp: Error object. > * > * Start a streaming operation on @bs. Clusters that are unallocated > @@ -1064,7 +1067,9 @@ int is_windows_drive(const char *filename); > void stream_start(const char *job_id, BlockDriverState *bs, > BlockDriverState *base, const char *backing_file_str, > int creation_flags, int64_t speed, > - BlockdevOnError on_error, Error **errp); > + BlockdevOnError on_error, > + const char *filter_node_name, > + Error **errp); > > /** > * commit_start: > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c > index 5ca3ebe..0432ac9 100644 > --- a/monitor/hmp-cmds.c > +++ b/monitor/hmp-cmds.c > @@ -2044,8 +2044,8 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict) > > qmp_block_stream(true, device, device, base != NULL, base, false, NULL, > false, NULL, qdict_haskey(qdict, "speed"), speed, true, > - BLOCKDEV_ON_ERROR_REPORT, false, false, false, false, > - &error); > + BLOCKDEV_ON_ERROR_REPORT, false, NULL, false, false, > + false, false, &error); > > hmp_handle_error(mon, &error); That's moved now into block/monitor/block-hmp-cmds.c Feel free to add the extra parameter to the HMP side as well! Dave > } > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 3c54717..169f8ea 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -2552,6 +2552,11 @@ > # 'stop' and 'enospc' can only be used if the block device > # supports io-status (see BlockInfo). Since 1.3. > # > +# @filter-node-name: the node name that should be assigned to the > +# filter driver that the stream job inserts into the graph > +# above @device. If this option is not given, a node name is > +# autogenerated. (Since: 5.0) > +# > # @auto-finalize: When false, this job will wait in a PENDING state after it has > # finished its work, waiting for @block-job-finalize before > # making any block graph changes. > @@ -2581,6 +2586,7 @@ > 'data': { '*job-id': 'str', 'device': 'str', '*base': 'str', > '*base-node': 'str', '*backing-file': 'str', '*speed': 'int', > '*on-error': 'BlockdevOnError', > + '*filter-node-name': 'str', > '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } } > > ## > -- > 1.8.3.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
20.04.2020 21:36, Andrey Shinkevich wrote: > Provide the possibility to pass the 'filter-node-name' parameter to the > block-stream job as it is done for the commit block job. So, you add it, but it actually do nothing for now. I'd prefer to make this patch the last one, so it actually make the change. > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > --- > block/stream.c | 5 +++-- > blockdev.c | 8 +++++++- > include/block/block_int.h | 7 ++++++- > monitor/hmp-cmds.c | 4 ++-- > qapi/block-core.json | 6 ++++++ > 5 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/block/stream.c b/block/stream.c > index d8b4bbe..fab7923 100644 > --- a/block/stream.c > +++ b/block/stream.c > @@ -229,7 +229,9 @@ static const BlockJobDriver stream_job_driver = { > void stream_start(const char *job_id, BlockDriverState *bs, > BlockDriverState *base, const char *backing_file_str, > int creation_flags, int64_t speed, > - BlockdevOnError on_error, Error **errp) > + BlockdevOnError on_error, > + const char *filter_node_name, > + Error **errp) > { > StreamBlockJob *s; > BlockDriverState *iter; > @@ -265,7 +267,6 @@ void stream_start(const char *job_id, BlockDriverState *bs, > } else { > bdrv_unfreeze_chain(bottom_cow_node, above_base); > } > - extra hunk > /* Prevent concurrent jobs trying to modify the graph structure here, we > * already have our own plans. Also don't allow resize as the image size is > * queried only at the job start and then cached. */ > diff --git a/blockdev.c b/blockdev.c > index 72d28ce..f275a64 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3242,6 +3242,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, > bool has_backing_file, const char *backing_file, > bool has_speed, int64_t speed, > bool has_on_error, BlockdevOnError on_error, > + bool has_filter_node_name, const char *filter_node_name, > bool has_auto_finalize, bool auto_finalize, > bool has_auto_dismiss, bool auto_dismiss, > Error **errp) > @@ -3257,6 +3258,10 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, > on_error = BLOCKDEV_ON_ERROR_REPORT; > } > > + if (!has_filter_node_name) { > + filter_node_name = NULL; > + } it is guaranteed to be zeroed in this case, so you don't need to set it > + > bs = bdrv_lookup_bs(device, device, errp); > if (!bs) { > return; > @@ -3324,7 +3329,8 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, > } > > stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name, > - job_flags, has_speed ? speed : 0, on_error, &local_err); > + job_flags, has_speed ? speed : 0, on_error, > + filter_node_name, &local_err); > if (local_err) { > error_propagate(errp, local_err); > goto out; > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 993bafc..5ac4891 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -1052,6 +1052,9 @@ int is_windows_drive(const char *filename); > * See @BlockJobCreateFlags > * @speed: The maximum speed, in bytes per second, or 0 for unlimited. > * @on_error: The action to take upon error. > + * @filter_node_name: The node name that should be assigned to the filter > + * driver that the commit job inserts into the graph above @bs. NULL means > + * that a node name should be autogenerated. > * @errp: Error object. > * > * Start a streaming operation on @bs. Clusters that are unallocated > @@ -1064,7 +1067,9 @@ int is_windows_drive(const char *filename); > void stream_start(const char *job_id, BlockDriverState *bs, > BlockDriverState *base, const char *backing_file_str, > int creation_flags, int64_t speed, > - BlockdevOnError on_error, Error **errp); > + BlockdevOnError on_error, > + const char *filter_node_name, > + Error **errp); > > /** > * commit_start: > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c > index 5ca3ebe..0432ac9 100644 > --- a/monitor/hmp-cmds.c > +++ b/monitor/hmp-cmds.c > @@ -2044,8 +2044,8 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict) > > qmp_block_stream(true, device, device, base != NULL, base, false, NULL, > false, NULL, qdict_haskey(qdict, "speed"), speed, true, > - BLOCKDEV_ON_ERROR_REPORT, false, false, false, false, > - &error); > + BLOCKDEV_ON_ERROR_REPORT, false, NULL, false, false, > + false, false, &error); > > hmp_handle_error(mon, &error); > } > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 3c54717..169f8ea 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -2552,6 +2552,11 @@ > # 'stop' and 'enospc' can only be used if the block device > # supports io-status (see BlockInfo). Since 1.3. > # > +# @filter-node-name: the node name that should be assigned to the > +# filter driver that the stream job inserts into the graph > +# above @device. If this option is not given, a node name is > +# autogenerated. (Since: 5.0) > +# > # @auto-finalize: When false, this job will wait in a PENDING state after it has > # finished its work, waiting for @block-job-finalize before > # making any block graph changes. > @@ -2581,6 +2586,7 @@ > 'data': { '*job-id': 'str', 'device': 'str', '*base': 'str', > '*base-node': 'str', '*backing-file': 'str', '*speed': 'int', > '*on-error': 'BlockdevOnError', > + '*filter-node-name': 'str', > '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } } > > ## >
21.04.2020 15:40, Vladimir Sementsov-Ogievskiy wrote: > 20.04.2020 21:36, Andrey Shinkevich wrote: >> Provide the possibility to pass the 'filter-node-name' parameter to the >> block-stream job as it is done for the commit block job. > > So, you add it, but it actually do nothing for now. I'd prefer to make this patch the last one, so it actually make the change. > Ah, I see, you need it, to fix iotests.. And otherwise we'll have to merge it into 7/7. Hmm. OK than, but note in a commit message that actual change is in further patch.
diff --git a/block/stream.c b/block/stream.c index d8b4bbe..fab7923 100644 --- a/block/stream.c +++ b/block/stream.c @@ -229,7 +229,9 @@ static const BlockJobDriver stream_job_driver = { void stream_start(const char *job_id, BlockDriverState *bs, BlockDriverState *base, const char *backing_file_str, int creation_flags, int64_t speed, - BlockdevOnError on_error, Error **errp) + BlockdevOnError on_error, + const char *filter_node_name, + Error **errp) { StreamBlockJob *s; BlockDriverState *iter; @@ -265,7 +267,6 @@ void stream_start(const char *job_id, BlockDriverState *bs, } else { bdrv_unfreeze_chain(bottom_cow_node, above_base); } - /* Prevent concurrent jobs trying to modify the graph structure here, we * already have our own plans. Also don't allow resize as the image size is * queried only at the job start and then cached. */ diff --git a/blockdev.c b/blockdev.c index 72d28ce..f275a64 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3242,6 +3242,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, bool has_backing_file, const char *backing_file, bool has_speed, int64_t speed, bool has_on_error, BlockdevOnError on_error, + bool has_filter_node_name, const char *filter_node_name, bool has_auto_finalize, bool auto_finalize, bool has_auto_dismiss, bool auto_dismiss, Error **errp) @@ -3257,6 +3258,10 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, on_error = BLOCKDEV_ON_ERROR_REPORT; } + if (!has_filter_node_name) { + filter_node_name = NULL; + } + bs = bdrv_lookup_bs(device, device, errp); if (!bs) { return; @@ -3324,7 +3329,8 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device, } stream_start(has_job_id ? job_id : NULL, bs, base_bs, base_name, - job_flags, has_speed ? speed : 0, on_error, &local_err); + job_flags, has_speed ? speed : 0, on_error, + filter_node_name, &local_err); if (local_err) { error_propagate(errp, local_err); goto out; diff --git a/include/block/block_int.h b/include/block/block_int.h index 993bafc..5ac4891 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1052,6 +1052,9 @@ int is_windows_drive(const char *filename); * See @BlockJobCreateFlags * @speed: The maximum speed, in bytes per second, or 0 for unlimited. * @on_error: The action to take upon error. + * @filter_node_name: The node name that should be assigned to the filter + * driver that the commit job inserts into the graph above @bs. NULL means + * that a node name should be autogenerated. * @errp: Error object. * * Start a streaming operation on @bs. Clusters that are unallocated @@ -1064,7 +1067,9 @@ int is_windows_drive(const char *filename); void stream_start(const char *job_id, BlockDriverState *bs, BlockDriverState *base, const char *backing_file_str, int creation_flags, int64_t speed, - BlockdevOnError on_error, Error **errp); + BlockdevOnError on_error, + const char *filter_node_name, + Error **errp); /** * commit_start: diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 5ca3ebe..0432ac9 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -2044,8 +2044,8 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict) qmp_block_stream(true, device, device, base != NULL, base, false, NULL, false, NULL, qdict_haskey(qdict, "speed"), speed, true, - BLOCKDEV_ON_ERROR_REPORT, false, false, false, false, - &error); + BLOCKDEV_ON_ERROR_REPORT, false, NULL, false, false, + false, false, &error); hmp_handle_error(mon, &error); } diff --git a/qapi/block-core.json b/qapi/block-core.json index 3c54717..169f8ea 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2552,6 +2552,11 @@ # 'stop' and 'enospc' can only be used if the block device # supports io-status (see BlockInfo). Since 1.3. # +# @filter-node-name: the node name that should be assigned to the +# filter driver that the stream job inserts into the graph +# above @device. If this option is not given, a node name is +# autogenerated. (Since: 5.0) +# # @auto-finalize: When false, this job will wait in a PENDING state after it has # finished its work, waiting for @block-job-finalize before # making any block graph changes. @@ -2581,6 +2586,7 @@ 'data': { '*job-id': 'str', 'device': 'str', '*base': 'str', '*base-node': 'str', '*backing-file': 'str', '*speed': 'int', '*on-error': 'BlockdevOnError', + '*filter-node-name': 'str', '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } } ##
Provide the possibility to pass the 'filter-node-name' parameter to the block-stream job as it is done for the commit block job. Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> --- block/stream.c | 5 +++-- blockdev.c | 8 +++++++- include/block/block_int.h | 7 ++++++- monitor/hmp-cmds.c | 4 ++-- qapi/block-core.json | 6 ++++++ 5 files changed, 24 insertions(+), 6 deletions(-)