On Fri, Dec 30, 2011 at 10:03 AM, Marcelo Tosatti <mtosa...@redhat.com> wrote:
> +int bdrv_is_allocated_base(BlockDriverState *top,
> + BlockDriverState *base,
> + int64_t sector_num,
> + int nb_sectors, int *pnum)
Since this function runs in coroutine context it should be marked:
int coroutine_fn bdrv_co_is_allocated_base(...)
The _co_ in the filename is just a convention to identify block layer
functions that execute in coroutine context. The coroutine_fn
annotation is useful if we ever want static checker support that
verifies that coroutine functions are always called from coroutine
context.
> +{
> + BlockDriverState *intermediate;
> + int ret, n;
> +
> + ret = bdrv_co_is_allocated(top, sector_num, nb_sectors, &n);
> + if (ret) {
> + *pnum = n;
> + return ret;
> + }
> +
> + /*
> + * Is the unallocated chunk [sector_num, n] also
> + * unallocated between base and top?
> + */
> + intermediate = top->backing_hd;
This reaches into BlockDriverState->backing_hd. In practice this is
probably the simplest and best way to do it. But if we're going to do
this then do we even need the BlockDriver .bdrv_find_backing_file()
abstraction? We could implement generic code which traverses
->backing_hd in block.c and if you don't use
BlockDriverState->backing_hd then you're out of luck.
> @@ -129,7 +141,10 @@ retry:
> bdrv_disable_zero_detection(bs);
>
> if (sector_num == end && ret == 0) {
> - bdrv_change_backing_file(bs, NULL, NULL);
> + const char *base_id = NULL;
> + if (base)
> + base_id = s->backing_file_id;
> + bdrv_change_backing_file(bs, base_id, NULL);
This makes me want to move the bdrv_change_backing_file() call out to
blockdev.c where we would perform it on successful completion. That
way we don't need to pass base_id into stream.c at all. A streaming
block job would no longer automatically change the backing file on
completion.
> @@ -166,6 +182,8 @@ int stream_start(BlockDriverState *bs, B
>
> s = block_job_create(&stream_job_type, bs, cb, opaque);
> s->base = base;
> + if (base_id)
> + strncpy(s->backing_file_id, base_id, sizeof(s->backing_file_id) - 1);
cutils.c:pstrcpy() is nicer than strncpy(), it does not need the size - 1.
