[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 18/18] nbd: BLOCK_STATUS for standard get_block_
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH 18/18] nbd: BLOCK_STATUS for standard get_block_status function: client part |
Date: |
Thu, 9 Feb 2017 10:00:07 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 02/03/2017 09:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Minimal realization: only first extent from the answer is used.
If you're only going to use one extent, should you implement
NBD_CMD_FLAG_REQ_ONE support to save the server some effort?
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
> block/nbd-client.c | 41 ++++++++++++++++++++++++++++++++++++++++-
> block/nbd-client.h | 5 +++++
> block/nbd.c | 3 +++
> include/block/nbd.h | 2 +-
> nbd/client.c | 23 ++++++++++++++++-------
> qemu-nbd.c | 2 +-
> 6 files changed, 66 insertions(+), 10 deletions(-)
> +int64_t coroutine_fn nbd_client_co_get_block_status(BlockDriverState *bs,
> + int64_t sector_num,
> + int nb_sectors, int
> *pnum,
> + BlockDriverState **file)
> +{
> + int64_t ret;
> + uint32_t nb_extents;
> + NBDExtent *extents;
> + NBDClientSession *client = nbd_get_client_session(bs);
> +
> + if (!client->block_status_ok) {
> + *pnum = nb_sectors;
> + ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
> + if (bs->drv->protocol_name) {
> + ret |= BDRV_BLOCK_OFFSET_VALID | (sector_num * BDRV_SECTOR_SIZE);
> + }
> + return ret;
> + }
Looks like a sane fallback when we don't have anything more accurate.
> +
> + ret = nbd_client_co_cmd_block_status(bs, sector_num << BDRV_SECTOR_BITS,
> + nb_sectors << BDRV_SECTOR_BITS,
> + &extents, &nb_extents);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + *pnum = extents[0].length >> BDRV_SECTOR_BITS;
> + ret = (extents[0].flags & NBD_STATE_HOLE ? 0 : BDRV_BLOCK_ALLOCATED) |
> + (extents[0].flags & NBD_STATE_ZERO ? BDRV_BLOCK_ZERO : 0);
> +
> + if ((ret & BDRV_BLOCK_ALLOCATED) && !(ret & BDRV_BLOCK_ZERO)) {
> + ret |= BDRV_BLOCK_DATA;
> + }
And this conversion looks correct.
> @@ -579,7 +617,8 @@ int nbd_client_init(BlockDriverState *bs,
> &client->size,
> &client->structured_reply,
> bitmap_name,
> - &client->bitmap_ok, errp);
> + &client->bitmap_ok,
> + &client->block_status_ok, errp);
That's a lot of parameters we're adding; I'm wondering if its smarter to
start passing things through via a struct. In fact, I have posted
patches previously (and need to rebase and repost them) that introduce
such a struct, in order to make the INFO extension viable.
> @@ -681,11 +681,19 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char
> *name, uint16_t *flags,
> false, NULL) == 0;
> }
>
> - if (!!structured_reply && *structured_reply && !!bitmap_name) {
> + if (!!structured_reply && *structured_reply) {
> int ret;
> - assert(!!bitmap_ok);
> - ret = nbd_receive_query_bitmap(ioc, name, bitmap_name,
> - bitmap_ok, errp) == 0;
> +
> + if (!!bitmap_name) {
> + assert(!!bitmap_ok);
> + ret = nbd_receive_query_bitmap(ioc, name, bitmap_name,
> + bitmap_ok, errp) == 0;
> + } else {
> + ret = nbd_receive_query_meta_context(ioc, name,
> + "base:allocation",
> + block_status_ok,
> + errp);
> + }
This looks weird trying to grab 'base:allocation' only if bitmap_name is
not provided. The logic will probably have to be different if we want
to allow a client to track both pieces of information at once.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH 13/18] nbd: add nbd_dirty_bitmap_load, (continued)
- [Qemu-block] [PATCH 18/18] nbd: BLOCK_STATUS for standard get_block_status function: client part, Vladimir Sementsov-Ogievskiy, 2017/02/03
- Re: [Qemu-block] [PATCH 18/18] nbd: BLOCK_STATUS for standard get_block_status function: client part,
Eric Blake <=
- [Qemu-block] [PATCH 01/18] nbd: rename NBD_REPLY_MAGIC to NBD_SIMPLE_REPLY_MAGIC, Vladimir Sementsov-Ogievskiy, 2017/02/03
- [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Vladimir Sementsov-Ogievskiy, 2017/02/03
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Eric Blake, 2017/02/07
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Vladimir Sementsov-Ogievskiy, 2017/02/09
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Eric Blake, 2017/02/09
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Vladimir Sementsov-Ogievskiy, 2017/02/10
- Re: [Qemu-block] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Eric Blake, 2017/02/13
- Re: [Qemu-block] [Qemu-devel] [PATCH 04/18] nbd/client: refactor nbd_receive_starttls, Eric Blake, 2017/02/20