We are going to use the COR-filter for a block-stream job.
To limit COR operations by the base node in the backing chain during
stream job, pass the bottom node name, that is the first non-filter
overlay of the base, to the copy-on-read driver as the base node itself
may change due to possible concurrent jobs.
The rest of the functionality will be implemented in the patch that
follows.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
block/copy-on-read.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 618c4c4..3d8e4db 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -24,18 +24,24 @@
#include "block/block_int.h"
#include "qemu/module.h"
#include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
+#include "qapi/qmp/qdict.h"
#include "block/copy-on-read.h"
typedef struct BDRVStateCOR {
bool active;
+ BlockDriverState *bottom_bs;
} BDRVStateCOR;
static int cor_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
+ BlockDriverState *bottom_bs = NULL;
BDRVStateCOR *state = bs->opaque;
+ /* Find a bottom node name, if any */
+ const char *bottom_node = qdict_get_try_str(options, "bottom");
bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
@@ -51,7 +57,17 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
bs->file->bs->supported_zero_flags);
+ if (bottom_node) {
+ bottom_bs = bdrv_lookup_bs(NULL, bottom_node, errp);
+ if (!bottom_bs) {
+ error_setg(errp, QERR_BASE_NOT_FOUND, bottom_node);
+ qdict_del(options, "bottom");
+ return -EINVAL;
+ }
+ qdict_del(options, "bottom");
+ }
state->active = true;
+ state->bottom_bs = bottom_bs;
/*
* We don't need to call bdrv_child_refresh_perms() now as the permissions
--
1.8.3.1
22.10.2020 21:13, Andrey Shinkevich wrote:
> We are going to use the COR-filter for a block-stream job.
> To limit COR operations by the base node in the backing chain during
> stream job, pass the bottom node name, that is the first non-filter
> overlay of the base, to the copy-on-read driver as the base node itself
> may change due to possible concurrent jobs.
> The rest of the functionality will be implemented in the patch that
> follows.
>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> block/copy-on-read.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> index 618c4c4..3d8e4db 100644
> --- a/block/copy-on-read.c
> +++ b/block/copy-on-read.c
> @@ -24,18 +24,24 @@
> #include "block/block_int.h"
> #include "qemu/module.h"
> #include "qapi/error.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qapi/qmp/qdict.h"
> #include "block/copy-on-read.h"
>
>
> typedef struct BDRVStateCOR {
> bool active;
> + BlockDriverState *bottom_bs;
> } BDRVStateCOR;
>
>
> static int cor_open(BlockDriverState *bs, QDict *options, int flags,
> Error **errp)
> {
> + BlockDriverState *bottom_bs = NULL;
> BDRVStateCOR *state = bs->opaque;
> + /* Find a bottom node name, if any */
> + const char *bottom_node = qdict_get_try_str(options, "bottom");
>
> bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
> BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
> @@ -51,7 +57,17 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
> ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
> bs->file->bs->supported_zero_flags);
>
> + if (bottom_node) {
> + bottom_bs = bdrv_lookup_bs(NULL, bottom_node, errp);
> + if (!bottom_bs) {
> + error_setg(errp, QERR_BASE_NOT_FOUND, bottom_node);
QERR_BASE_NOT_FOUND is unrelated here. Also, I see a comment in qerror.h that such macros should not be used in new code. And don't forget to drop qerror.h include line.
> + qdict_del(options, "bottom");
this may be moved above "bottom_bs = ..", to not call it after "if" in separate.
> + return -EINVAL;
> + }
> + qdict_del(options, "bottom");
> + }
> state->active = true;
> + state->bottom_bs = bottom_bs;
>
> /*
> * We don't need to call bdrv_child_refresh_perms() now as the permissions
>
--
Best regards,
Vladimir
On 23.10.2020 17:45, Vladimir Sementsov-Ogievskiy wrote:
> 22.10.2020 21:13, Andrey Shinkevich wrote:
>> We are going to use the COR-filter for a block-stream job.
>> To limit COR operations by the base node in the backing chain during
>> stream job, pass the bottom node name, that is the first non-filter
>> overlay of the base, to the copy-on-read driver as the base node itself
>> may change due to possible concurrent jobs.
>> The rest of the functionality will be implemented in the patch that
>> follows.
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>> block/copy-on-read.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
>> index 618c4c4..3d8e4db 100644
>> --- a/block/copy-on-read.c
>> +++ b/block/copy-on-read.c
>> @@ -24,18 +24,24 @@
>> #include "block/block_int.h"
>> #include "qemu/module.h"
>> #include "qapi/error.h"
>> +#include "qapi/qmp/qerror.h"
>> +#include "qapi/qmp/qdict.h"
>> #include "block/copy-on-read.h"
>> typedef struct BDRVStateCOR {
>> bool active;
>> + BlockDriverState *bottom_bs;
>> } BDRVStateCOR;
>> static int cor_open(BlockDriverState *bs, QDict *options, int flags,
>> Error **errp)
>> {
>> + BlockDriverState *bottom_bs = NULL;
>> BDRVStateCOR *state = bs->opaque;
>> + /* Find a bottom node name, if any */
>> + const char *bottom_node = qdict_get_try_str(options, "bottom");
>> bs->file = bdrv_open_child(NULL, options, "file", bs,
>> &child_of_bds,
>> BDRV_CHILD_FILTERED |
>> BDRV_CHILD_PRIMARY,
>> @@ -51,7 +57,17 @@ static int cor_open(BlockDriverState *bs, QDict
>> *options, int flags,
>> ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
>> bs->file->bs->supported_zero_flags);
>> + if (bottom_node) {
>> + bottom_bs = bdrv_lookup_bs(NULL, bottom_node, errp);
>> + if (!bottom_bs) {
>> + error_setg(errp, QERR_BASE_NOT_FOUND, bottom_node);
>
> QERR_BASE_NOT_FOUND is unrelated here. Also, I see a comment in qerror.h
> that such macros should not be used in new code. And don't forget to
> drop qerror.h include line.
>
I have been surprized because I don't have it in my branch and instead I do:
error_setg(errp, "Bottom node '%s' not found", bottom_node);
>> + qdict_del(options, "bottom");
>
> this may be moved above "bottom_bs = ..", to not call it after "if" in
> separate.
>
Please, see the "Re: [PATCH v11 04/13] copy-on-read: pass overlay base
node name to COR driver".
>> + return -EINVAL;
>> + }
>> + qdict_del(options, "bottom");
>> + }
>> state->active = true;
>> + state->bottom_bs = bottom_bs;
>> /*
>> * We don't need to call bdrv_child_refresh_perms() now as the
>> permissions
>>
>
>
23.10.2020 18:31, Andrey Shinkevich wrote:
>
> On 23.10.2020 17:45, Vladimir Sementsov-Ogievskiy wrote:
>> 22.10.2020 21:13, Andrey Shinkevich wrote:
>>> We are going to use the COR-filter for a block-stream job.
>>> To limit COR operations by the base node in the backing chain during
>>> stream job, pass the bottom node name, that is the first non-filter
>>> overlay of the base, to the copy-on-read driver as the base node itself
>>> may change due to possible concurrent jobs.
>>> The rest of the functionality will be implemented in the patch that
>>> follows.
>>>
>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>> ---
>>> block/copy-on-read.c | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
>>> index 618c4c4..3d8e4db 100644
>>> --- a/block/copy-on-read.c
>>> +++ b/block/copy-on-read.c
>>> @@ -24,18 +24,24 @@
>>> #include "block/block_int.h"
>>> #include "qemu/module.h"
>>> #include "qapi/error.h"
>>> +#include "qapi/qmp/qerror.h"
>>> +#include "qapi/qmp/qdict.h"
>>> #include "block/copy-on-read.h"
>>> typedef struct BDRVStateCOR {
>>> bool active;
>>> + BlockDriverState *bottom_bs;
>>> } BDRVStateCOR;
>>> static int cor_open(BlockDriverState *bs, QDict *options, int flags,
>>> Error **errp)
>>> {
>>> + BlockDriverState *bottom_bs = NULL;
>>> BDRVStateCOR *state = bs->opaque;
>>> + /* Find a bottom node name, if any */
>>> + const char *bottom_node = qdict_get_try_str(options, "bottom");
>>> bs->file = bdrv_open_child(NULL, options, "file", bs, &child_of_bds,
>>> BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
>>> @@ -51,7 +57,17 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
>>> ((BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK) &
>>> bs->file->bs->supported_zero_flags);
>>> + if (bottom_node) {
>>> + bottom_bs = bdrv_lookup_bs(NULL, bottom_node, errp);
>>> + if (!bottom_bs) {
>>> + error_setg(errp, QERR_BASE_NOT_FOUND, bottom_node);
>>
>> QERR_BASE_NOT_FOUND is unrelated here. Also, I see a comment in qerror.h that such macros should not be used in new code. And don't forget to drop qerror.h include line.
>>
>
> I have been surprized because I don't have it in my branch and instead I do:
> error_setg(errp, "Bottom node '%s' not found", bottom_node);
OK, with it:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
>>> + qdict_del(options, "bottom");
>>
>> this may be moved above "bottom_bs = ..", to not call it after "if" in separate.
>>
>
> Please, see the "Re: [PATCH v11 04/13] copy-on-read: pass overlay base node name to COR driver".
Hm, assume that it may free the string itself, OK then.
>
>>> + return -EINVAL;
>>> + }
>>> + qdict_del(options, "bottom");
>>> + }
>>> state->active = true;
>>> + state->bottom_bs = bottom_bs;
>>> /*
>>> * We don't need to call bdrv_child_refresh_perms() now as the permissions
>>>
>>
>>
--
Best regards,
Vladimir
© 2016 - 2026 Red Hat, Inc.