[libvirt] [PATCH 3/4] backup: Add new qemu monitor bitmap

[Peter Krempa]

On Mon, Jun 10, 2019 at 21:49:51 -0500, Eric Blake wrote:
> On 6/7/19 2:08 AM, Peter Krempa wrote:
> > On Thu, Jun 06, 2019 at 08:41:26 -0500, Eric Blake wrote:
> >> On 6/6/19 7:48 AM, Peter Krempa wrote:
> >>> On Wed, Jun 05, 2019 at 22:01:09 -0500, Eric Blake wrote:
> >>>> The upcoming virDomainBackup() API needs to take advantage of various
> >>>> qcow2 bitmap manipulations as the basis to virDomainCheckpoints and
> >>>> incremental backups.  Add four functions to expose
> >>>> block-dirty-bitmap-{add,enable,disable,merge} (this is the
> >>>> recently-added QEMU_CAPS_BITMAP_MERGE capability).
> >>>>
> >>>> Signed-off-by: Eric Blake <eblake at redhat.com>
> >>>> ---
> >>>>  src/qemu/qemu_monitor.h      |  19 ++++++
> >>>>  src/qemu/qemu_monitor_json.h |  17 +++++
> >>>>  src/qemu/qemu_monitor.c      |  51 +++++++++++++++
> >>>>  src/qemu/qemu_monitor_json.c | 119 +++++++++++++++++++++++++++++++++++
> >>>>  4 files changed, 206 insertions(+)
> >>>
> >>> I'd suggest you add GEN_TEST_FUNC/DO_TEST_GEN tests for those. It's
> >>> simple to add and gives you checking of the arguments against the QAPI
> >>> schema for free.
> >>
> >> Will do. Do you need to see the amended patch with that added, or should
> >> I go ahead and push once I have it working?
> > 
> > ACK if you add those with all the fields excercised.
> 
> Here's what I'm squashing in. The MergeBitmaps test depends on my
> cleanup to use TestNewSchema everywhere. Also, I noticed that if we
> wanted to use VIR_AUTOPTR(qemuMonitorTest) it might make a lot of the
> file easier to read, but that should be a separate patch (including fixing:
> qemumonitorjsontest.c:1431:5: error: cleanup argument not a function
>      VIR_AUTOPTR(qemuMonitorTest) test = NULL;

As I've pointed out in the RFC patch you've sent, adding the automatic
freeing function is a good idea, but refactoring old code does seem to
be a waste of time.

You certainly can use the new format in the code you are adding though.

>      ^~~~~~~~~~~
> )
> 
> diff --git i/tests/qemumonitorjsontest.c w/tests/qemumonitorjsontest.c
> index 2e7e976e9b..c07d8bf548 100644
> --- i/tests/qemumonitorjsontest.c
> +++ w/tests/qemumonitorjsontest.c
> @@ -1375,6 +1375,9 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen,
> "foodev", true)
>  GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev")
>  GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumRemove, "foodev")
>  GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumInsert, "foodev", "newnode")
> +GEN_TEST_FUNC(qemuMonitorJSONAddBitmap, "node", "bitmap", true)
> +GEN_TEST_FUNC(qemuMonitorJSONEnableBitmap, "node", "bitmap")
> +GEN_TEST_FUNC(qemuMonitorJSONDeleteBitmap, "node", "bitmap")
> 
>  static int
>  testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque)
> @@ -1419,6 +1422,44 @@
> testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque)
>      return ret;
>  }
> 
> +static int
> +testQemuMonitorJSONqemuMonitorJSONMergeBitmaps(const void *opaque)
> +{
> +    const testGenericData *data = opaque;
> +    virDomainXMLOptionPtr xmlopt = data->xmlopt;
> +    qemuMonitorTestPtr test = qemuMonitorTestNewSchema(xmlopt,
> data->schema);
> +    int ret = -1;
> +    VIR_AUTOPTR(virJSONValue) arr = NULL;
> +
> +
> +    if (!test)
> +        return -1;
> +
> +    if (!(arr = virJSONValueNewArray()))
> +        goto cleanup;
> +
> +    if (virJSONValueArrayAppendString(arr, "b1") < 0 ||
> +        virJSONValueArrayAppendString(arr, "b2") < 0)
> +        goto cleanup;
> +
> +    if (qemuMonitorTestAddItem(test, "block-dirty-bitmap-merge",
> +                               "{\"return\":{}}") < 0)
> +        goto cleanup;
> +
> +    if (qemuMonitorJSONMergeBitmaps(qemuMonitorTestGetMonitor(test),
> +                                    "node", "dst", &arr) < 0)
> +        goto cleanup;
> +
> +    if (arr)
> +        goto cleanup;
> +
> +    ret = 0;
> +
> + cleanup:
> +    qemuMonitorTestFree(test);
> +    return ret;
> +}
> +
>  static bool
>  testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct
> qemuMonitorQueryCpusEntry *a,
>                                                   struct
> qemuMonitorQueryCpusEntry *b)
> @@ -3109,6 +3150,9 @@ mymain(void)
>      DO_TEST_GEN(qemuMonitorJSONBlockdevTrayClose);
>      DO_TEST_GEN(qemuMonitorJSONBlockdevMediumRemove);
>      DO_TEST_GEN(qemuMonitorJSONBlockdevMediumInsert);
> +    DO_TEST_GEN(qemuMonitorJSONAddBitmap);
> +    DO_TEST_GEN(qemuMonitorJSONEnableBitmap);
> +    DO_TEST_GEN(qemuMonitorJSONDeleteBitmap);
>      DO_TEST(qemuMonitorJSONGetBalloonInfo);
>      DO_TEST(qemuMonitorJSONGetBlockInfo);
>      DO_TEST(qemuMonitorJSONGetAllBlockStatsInfo);
> @@ -3125,6 +3169,7 @@ mymain(void)
>      DO_TEST(qemuMonitorJSONSendKeyHoldtime);
>      DO_TEST(qemuMonitorSupportsActiveCommit);
>      DO_TEST(qemuMonitorJSONNBDServerStart);
> +    DO_TEST(qemuMonitorJSONMergeBitmaps);
> 
>      DO_TEST_CPU_DATA("host");
>      DO_TEST_CPU_DATA("full");

ACK to the squash-in regardless of if you decide to use AUTOPTR for the
test monitor in testQemuMonitorJSONqemuMonitorJSONMergeBitmaps.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190611/03b806bc/attachment-0001.sig>