Re: [PATCH v4 5/9] blockdev: Split off basic bitmap operations for qemu-img

[Vladimir Sementsov-Ogievskiy]

13.05.2020 04:16, Eric Blake wrote:
> Upcoming patches want to add some basic bitmap manipulation abilities
> to qemu-img.  But blockdev.o is too heavyweight to link into qemu-img
> (among other things, it would drag in block jobs and transaction
> support - qemu-img does offline manipulation, where atomicity is less
> important because there are no concurrent modifications to compete
> with), so it's time to split off the bare bones of what we will need
> into a new file block/monitor/bitmap-qmp-cmds.o.
>
> This is sufficient to expose 6 QMP commands for use by qemu-img (add,
> remove, clear, enable, disable, merge), as well as move the three
> helper functions touched in the previous patch.  Regarding
> MAINTAINERS, the new file is automatically part of block core, but
> also makes sense as related to other dirty bitmap files.
>
> Signed-off-by: Eric Blake <address@hidden>
> Reviewed-by: Max Reitz <address@hidden>
> ---
>   Makefile.objs                   |   3 +-
>   block/monitor/bitmap-qmp-cmds.c | 323 ++++++++++++++++++++++++++++++++
>   blockdev.c                      | 284 ----------------------------
>   MAINTAINERS                     |   1 +
>   block/monitor/Makefile.objs     |   1 +
>   5 files changed, 326 insertions(+), 286 deletions(-)
>   create mode 100644 block/monitor/bitmap-qmp-cmds.c
>
> diff --git a/Makefile.objs b/Makefile.objs
> index a7c967633acf..99774cfd2545 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -13,9 +13,8 @@ chardev-obj-y = chardev/
>
>   authz-obj-y = authz/
>
> -block-obj-y = nbd/
> +block-obj-y = block/ block/monitor/ nbd/ scsi/
>   block-obj-y += block.o blockjob.o job.o
> -block-obj-y += block/ scsi/
>   block-obj-y += qemu-io-cmds.o
>   block-obj-$(CONFIG_REPLICATION) += replication.o
>
> diff --git a/block/monitor/bitmap-qmp-cmds.c b/block/monitor/bitmap-qmp-cmds.c
> new file mode 100644
> index 000000000000..748e1e682483
> --- /dev/null
> +++ b/block/monitor/bitmap-qmp-cmds.c

Hmm, shouldn't transaction bitmap actions be moved here too? May be, not in 
these series..

> @@ -0,0 +1,323 @@
> +/*
> + * QEMU host block device bitmaps

A bit conflicts with tha fact that they are not of block-device level but of 
node-level.

May be just "Block dirty bitmap qmp commands" ?

> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard

Does it really apply here? block-dirty-bitmap-add was added in 2015.. May be 
Red Hat and Virtuozzo copyrights would be more appropriate.

> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + *
> + * This file incorporates work covered by the following copyright and
> + * permission notice:
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "sysemu/blockdev.h"
> +#include "block/block.h"
> +#include "block/block_int.h"
> +#include "qapi/qapi-commands-block.h"
> +#include "qapi/error.h"

compiles for with only four:

  #include "qemu/osdep.h"
                                                                                 
  #include "block/block_int.h"
  #include "qapi/qapi-commands-block.h"
  #include "qapi/error.h"

with at least extra includes dropped:
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>

--
Best regards,
Vladimir