On 28.08.20 18:52, Andrey Shinkevich wrote:
To limit the guest's COR operations by the base node in the backing
chain during stream job, pass the base file name to the copy-on-read
Does it have to be a filename? That sounds really bad to me.
driver. 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 | 41 ++++++++++++++++++++++++++++++++++++++++-
block/copy-on-read.h | 1 +
2 files changed, 41 insertions(+), 1 deletion(-)
Furthermore, I believe that this option should become an externally
visible option for the copy-on-read filter (i.e., part of its
BlockdevOptions) – but that definitely won’t be viable if @base contains
a filename.
Can’t we let the stream job invoke bdrv_find_backing_image() to
translate a filename into a node name that’s then passed to the COR filter?
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 0ede7aa..1f858bb 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -24,19 +24,45 @@
#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 *base_bs;
} BDRVStateCOR;
+/*
+ * Non-zero pointers are the caller's responsibility.
+ */
+static BlockDriverState *get_base_by_name(BlockDriverState *bs,
+ const char *base_name, Error **errp)
+{
+ BlockDriverState *base_bs = NULL;
+ AioContext *aio_context;
+
+ base_bs = bdrv_find_backing_image(bs, base_name);
+ if (base_bs == NULL) {
+ error_setg(errp, QERR_BASE_NOT_FOUND, base_name);
+ return NULL;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+ assert(bdrv_get_aio_context(base_bs) == aio_context);
+ aio_context_release(aio_context);
Er. OK. But why? Isn’t this just guaranteed by the block layer? I
don’t think we need an explicit assertion for this, especially if it
means having to acquire an AioContext.
Furthermore, I don’t even know why we’d need the AioContext. On one
hand, we don’t need to acquire a context just to get it or compare it;
on the other, this I would have thought that .bdrv_open() runs in the
BDS’s AioContext anyway (or the caller already has it acquired at least).
Max