Re: [PATCH v6 2/4] copy-on-read: add filter append/drop functions
On 23.08.2020 22:35, Andrey Shinkevich wrote:
On 19.08.2020 13:21, Vladimir Sementsov-Ogievskiy wrote:
19.08.2020 00:24, Andrey Shinkevich wrote:
Provide API for the COR-filter insertion/removal.
Also, drop the filter child permissions for an inactive state when the
filter node is being removed.
Signed-off-by: Andrey Shinkevich
---
block/copy-on-read.c | 103
+++
block/copy-on-read.h | 36 ++
2 files changed, 139 insertions(+)
create mode 100644 block/copy-on-read.h
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index cb03e0f..150d9b7 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -23,11 +23,21 @@
...
+BlockDriverState *bdrv_cor_filter_append(BlockDriverState *bs,
+ const char *filter_node_name,
+ Error **errp)
+{
+ BlockDriverState *cor_filter_bs;
+ BDRVStateCOR *state;
+ Error *local_err = NULL;
+
+ cor_filter_bs = create_filter_node(bs, filter_node_name, errp);
+ if (cor_filter_bs == NULL) {
+ error_prepend(errp, "Could not create filter node: ");
+ return NULL;
+ }
+
+ if (!filter_node_name) {
+ cor_filter_bs->implicit = true;
+ }
+
+ bdrv_drained_begin(bs);
+ bdrv_replace_node(bs, cor_filter_bs, _err);
+ bdrv_drained_end(bs);
+
+ if (local_err) {
+ bdrv_unref(cor_filter_bs);
+ error_propagate(errp, local_err);
+ return NULL;
+ }
+
+ state = cor_filter_bs->opaque;
+ state->active = true;
hmm stop. it's already active, after create_filter_node, so you don't
need to set it again, isn't it?
I will remove the extra assignment from the cor_open() for consistancy.
Andrey
No, I won't. It is wrong to do that. COR-operations wouldn't work.
Andrey
+
+ return cor_filter_bs;
+}
+
...
Re: [PATCH v6 2/4] copy-on-read: add filter append/drop functions
On 19.08.2020 13:21, Vladimir Sementsov-Ogievskiy wrote:
19.08.2020 00:24, Andrey Shinkevich wrote:
Provide API for the COR-filter insertion/removal.
Also, drop the filter child permissions for an inactive state when the
filter node is being removed.
Signed-off-by: Andrey Shinkevich
---
block/copy-on-read.c | 103
+++
block/copy-on-read.h | 36 ++
2 files changed, 139 insertions(+)
create mode 100644 block/copy-on-read.h
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index cb03e0f..150d9b7 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -23,11 +23,21 @@
#include "qemu/osdep.h"
#include "block/block_int.h"
#include "qemu/module.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "block/copy-on-read.h"
+
+
+typedef struct BDRVStateCOR {
+ bool active;
+} BDRVStateCOR;
static int cor_open(BlockDriverState *bs, QDict *options, int
flags,
Error **errp)
{
+ BDRVStateCOR *state = bs->opaque;
+
bs->file = bdrv_open_child(NULL, options, "file", bs,
_of_bds,
BDRV_CHILD_FILTERED |
BDRV_CHILD_PRIMARY,
false, errp);
@@ -42,6 +52,8 @@ 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);
+ state->active = true;
We don't need to call bdrv_child_refresh_perms() here, as permissions
will be
updated soon, when the filter node get its parent, yes?
Let's add at least a comment on this.
+
return 0;
}
@@ -57,6 +69,17 @@ static void cor_child_perm(BlockDriverState *bs,
BdrvChild *c,
uint64_t perm, uint64_t shared,
uint64_t *nperm, uint64_t *nshared)
{
+ BDRVStateCOR *s = bs->opaque;
+
+ if (!s->active) {
+ /*
+ * While the filter is being removed
+ */
+ *nperm = 0;
+ *nshared = BLK_PERM_ALL;
+ return;
+ }
+
*nperm = perm & PERM_PASSTHROUGH;
*nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
@@ -135,6 +158,7 @@ static void cor_lock_medium(BlockDriverState
*bs, bool locked)
static BlockDriver bdrv_copy_on_read = {
.format_name = "copy-on-read",
+ .instance_size = sizeof(BDRVStateCOR),
.bdrv_open = cor_open,
.bdrv_child_perm = cor_child_perm,
@@ -159,4 +183,83 @@ static void bdrv_copy_on_read_init(void)
bdrv_register(_copy_on_read);
}
+
+static BlockDriverState *create_filter_node(BlockDriverState *bs,
+ const char
*filter_node_name,
+ Error **errp)
+{
+ QDict *opts = qdict_new();
+
+ qdict_put_str(opts, "driver", "copy-on-read");
+ qdict_put_str(opts, "file", bdrv_get_node_name(bs));
+ if (filter_node_name) {
+ qdict_put_str(opts, "node-name", filter_node_name);
+ }
+
+ return bdrv_open(NULL, NULL, opts, BDRV_O_RDWR, errp);
+}
+
+
+BlockDriverState *bdrv_cor_filter_append(BlockDriverState *bs,
+ const char *filter_node_name,
+ Error **errp)
+{
+ BlockDriverState *cor_filter_bs;
+ BDRVStateCOR *state;
+ Error *local_err = NULL;
+
+ cor_filter_bs = create_filter_node(bs, filter_node_name, errp);
+ if (cor_filter_bs == NULL) {
+ error_prepend(errp, "Could not create filter node: ");
+ return NULL;
+ }
+
+ if (!filter_node_name) {
+ cor_filter_bs->implicit = true;
+ }
+
+ bdrv_drained_begin(bs);
+ bdrv_replace_node(bs, cor_filter_bs, _err);
+ bdrv_drained_end(bs);
+
+ if (local_err) {
+ bdrv_unref(cor_filter_bs);
+ error_propagate(errp, local_err);
+ return NULL;
+ }
+
+ state = cor_filter_bs->opaque;
+ state->active = true;
hmm stop. it's already active, after create_filter_node, so you don't
need to set it again, isn't it?
I will remove the extra assignment from the cor_open() for consistancy.
Andrey
+
+ return cor_filter_bs;
+}
+
+
+void bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs)
+{
+ BdrvChild *child;
+ BlockDriverState *bs;
+ BDRVStateCOR *s = cor_filter_bs->opaque;
+
+ child = bdrv_filter_child(cor_filter_bs);
+ if (!child) {
+ return;
+ }
+ bs = child->bs;
+
+ /* Retain the BDS until we complete the graph change. */
+ bdrv_ref(bs);
+ /* Hold a guest back from writing while permissions are being
reset. */
+ bdrv_drained_begin(bs);
+ /* Drop permissions before the graph change. */
+ s->active = false;
+ bdrv_child_refresh_perms(cor_filter_bs, child, _abort);
+ bdrv_replace_node(cor_filter_bs, bs, _abort);
+
+
Re: [PATCH v6 2/4] copy-on-read: add filter append/drop functions
19.08.2020 00:24, Andrey Shinkevich wrote:
Provide API for the COR-filter insertion/removal.
Also, drop the filter child permissions for an inactive state when the
filter node is being removed.
Signed-off-by: Andrey Shinkevich
---
block/copy-on-read.c | 103 +++
block/copy-on-read.h | 36 ++
2 files changed, 139 insertions(+)
create mode 100644 block/copy-on-read.h
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index cb03e0f..150d9b7 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -23,11 +23,21 @@
#include "qemu/osdep.h"
#include "block/block_int.h"
#include "qemu/module.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
+#include "block/copy-on-read.h"
+
+
+typedef struct BDRVStateCOR {
+bool active;
+} BDRVStateCOR;
static int cor_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
+BDRVStateCOR *state = bs->opaque;
+
bs->file = bdrv_open_child(NULL, options, "file", bs, _of_bds,
BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY,
false, errp);
@@ -42,6 +52,8 @@ 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);
+state->active = true;
We don't need to call bdrv_child_refresh_perms() here, as permissions will be
updated soon, when the filter node get its parent, yes?
Let's add at least a comment on this.
+
return 0;
}
@@ -57,6 +69,17 @@ static void cor_child_perm(BlockDriverState *bs, BdrvChild *c,
uint64_t perm, uint64_t shared,
uint64_t *nperm, uint64_t *nshared)
{
+BDRVStateCOR *s = bs->opaque;
+
+if (!s->active) {
+/*
+ * While the filter is being removed
+ */
+*nperm = 0;
+*nshared = BLK_PERM_ALL;
+return;
+}
+
*nperm = perm & PERM_PASSTHROUGH;
*nshared = (shared & PERM_PASSTHROUGH) | PERM_UNCHANGED;
@@ -135,6 +158,7 @@ static void cor_lock_medium(BlockDriverState *bs, bool locked)
static BlockDriver bdrv_copy_on_read = {
.format_name= "copy-on-read",
+.instance_size = sizeof(BDRVStateCOR),
.bdrv_open = cor_open,
.bdrv_child_perm= cor_child_perm,
@@ -159,4 +183,83 @@ static void bdrv_copy_on_read_init(void)
bdrv_register(_copy_on_read);
}
+
+static BlockDriverState *create_filter_node(BlockDriverState *bs,
+const char *filter_node_name,
+Error **errp)
+{
+QDict *opts = qdict_new();
+
+qdict_put_str(opts, "driver", "copy-on-read");
+qdict_put_str(opts, "file", bdrv_get_node_name(bs));
+if (filter_node_name) {
+qdict_put_str(opts, "node-name", filter_node_name);
+}
+
+return bdrv_open(NULL, NULL, opts, BDRV_O_RDWR, errp);
+}
+
+
+BlockDriverState *bdrv_cor_filter_append(BlockDriverState *bs,
+ const char *filter_node_name,
+ Error **errp)
+{
+BlockDriverState *cor_filter_bs;
+BDRVStateCOR *state;
+Error *local_err = NULL;
+
+cor_filter_bs = create_filter_node(bs, filter_node_name, errp);
+if (cor_filter_bs == NULL) {
+error_prepend(errp, "Could not create filter node: ");
+return NULL;
+}
+
+if (!filter_node_name) {
+cor_filter_bs->implicit = true;
+}
+
+bdrv_drained_begin(bs);
+bdrv_replace_node(bs, cor_filter_bs, _err);
+bdrv_drained_end(bs);
+
+if (local_err) {
+bdrv_unref(cor_filter_bs);
+error_propagate(errp, local_err);
+return NULL;
+}
+
+state = cor_filter_bs->opaque;
+state->active = true;
hmm stop. it's already active, after create_filter_node, so you don't need to
set it again, isn't it?
+
+return cor_filter_bs;
+}
+
+
+void bdrv_cor_filter_drop(BlockDriverState *cor_filter_bs)
+{
+BdrvChild *child;
+BlockDriverState *bs;
+BDRVStateCOR *s = cor_filter_bs->opaque;
+
+child = bdrv_filter_child(cor_filter_bs);
+if (!child) {
+return;
+}
+bs = child->bs;
+
+/* Retain the BDS until we complete the graph change. */
+bdrv_ref(bs);
+/* Hold a guest back from writing while permissions are being reset. */
+bdrv_drained_begin(bs);
+/* Drop permissions before the graph change. */
+s->active = false;
+bdrv_child_refresh_perms(cor_filter_bs, child, _abort);
+bdrv_replace_node(cor_filter_bs, bs, _abort);
+
+bdrv_drained_end(bs);
+bdrv_unref(bs);
+bdrv_unref(cor_filter_bs);
+}
+
+
block_init(bdrv_copy_on_read_init);
diff --git a/block/copy-on-read.h
