[Qemu-devel] [PATCH v2] QemuOpt: add unit tests

[Qemu-devel] [PATCH v2] QemuOpt: add unit tests

[Leandro Dorileo]

Cover basic aspects and API usage for QemuOpt. The current implementation
covers the API's planned to be changed by Chunyan Liu in his QEMUOptionParameter
replacement/cleanup job.

Other APIs should be covered in future improvements.

Signed-off-by: Leandro Dorileo <l@dorileo.org>

Changes:
  v2:
     + fixed comments;
     + make use of g_assert_cmpstr();
     + use error_abort instead of a local_err for qemu_opts_absorb_qdict();
     + asserts on QemuOptsList (empty and list name);
     + added test_qemu_opt_unset();
     + asserts on qemu_opt_*_set() return;
     + added test_qemu_opts_reset();
     + added test_qemu_opts_set();
---
 tests/Makefile         |   3 +
 tests/test-qemu-opts.c | 455 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 458 insertions(+)
 create mode 100644 tests/test-qemu-opts.c

diff --git a/tests/Makefile b/tests/Makefile
index 471b4c8..4814283 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -60,6 +60,8 @@ check-unit-y += tests/test-qdev-global-props$(EXESUF)
 check-unit-y += tests/check-qom-interface$(EXESUF)
 gcov-files-check-qom-interface-y = qom/object.c
 check-unit-y += tests/test-vmstate$(EXESUF)
+check-unit-y += tests/test-qemu-opts$(EXESUF)
+gcov-files-test-qemu-opts-y = qom/test-qemu-opts.c
 
 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
@@ -272,6 +274,7 @@ tests/qom-test$(EXESUF): tests/qom-test.o
 tests/blockdev-test$(EXESUF): tests/blockdev-test.o $(libqos-pc-obj-y)
 tests/qdev-monitor-test$(EXESUF): tests/qdev-monitor-test.o $(libqos-pc-obj-y)
 tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o
+tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a
 
 # QTest rules
 
diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
new file mode 100644
index 0000000..abd2fb8
--- /dev/null
+++ b/tests/test-qemu-opts.c
@@ -0,0 +1,455 @@
+/*
+ * QemuOpts unit-tests.
+ *
+ * Copyright (C) 2014 Leandro Dorileo <l@dorileo.org>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include "qapi/error.h"
+#include "qapi/qmp/qstring.h"
+#include "qemu/config-file.h"
+
+#include <glib.h>
+#include <string.h>
+
+static QemuOptsList opts_list_01 = {
+    .name = "opts_list_01",
+    .head = QTAILQ_HEAD_INITIALIZER(opts_list_01.head),
+    .desc = {
+        {
+            .name = "str1",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "str2",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "str3",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "number1",
+            .type = QEMU_OPT_NUMBER,
+        },
+        { /* end of list */ }
+    },
+};
+
+static QemuOptsList opts_list_02 = {
+    .name = "opts_list_02",
+    .head = QTAILQ_HEAD_INITIALIZER(opts_list_02.head),
+    .desc = {
+        {
+            .name = "str1",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "bool1",
+            .type = QEMU_OPT_BOOL,
+        },{
+            .name = "str2",
+            .type = QEMU_OPT_STRING,
+        },{
+            .name = "size1",
+            .type = QEMU_OPT_SIZE,
+        },
+        { /* end of list */ }
+    },
+};
+
+QemuOptsList opts_list_03 = {
+    .name = "opts_list_03",
+    .head = QTAILQ_HEAD_INITIALIZER(opts_list_03.head),
+    .desc = {
+        /* no elements => accept any params */
+        { /* end of list */ }
+    },
+};
+
+static void register_opts(void)
+{
+    qemu_add_opts(&opts_list_01);
+    qemu_add_opts(&opts_list_02);
+    qemu_add_opts(&opts_list_03);
+}
+
+static void test_find_unknown_opts(void)
+{
+    QemuOptsList *list;
+
+    register_opts();
+
+    /* should not return anything, we don't have an "unknown" option */
+    list = qemu_find_opts("unknown");
+    g_assert(list == NULL);
+}
+
+static void test_qemu_find_opts(void)
+{
+    QemuOptsList *list;
+
+    register_opts();
+
+    /* we have an "opts_list_01" option, should return it */
+    list = qemu_find_opts("opts_list_01");
+    g_assert(list != NULL);
+    g_assert_cmpstr(list->name, ==, "opts_list_01");
+}
+
+static void test_qemu_opts_create(void)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+
+    register_opts();
+
+    list = qemu_find_opts("opts_list_01");
+    g_assert(list != NULL);
+    g_assert(QTAILQ_EMPTY(&list->head));
+    g_assert_cmpstr(list->name, ==, "opts_list_01");
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+
+    /* create the opts */
+    opts = qemu_opts_create(list, NULL, 0, &error_abort);
+    g_assert(opts != NULL);
+    g_assert(!QTAILQ_EMPTY(&list->head));
+
+    /* now we've create the opts, must find it */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts != NULL);
+
+    qemu_opts_del(opts);
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+}
+
+static void test_qemu_opt_get(void)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const char *opt = NULL;
+
+    register_opts();
+
+    list = qemu_find_opts("opts_list_01");
+    g_assert(list != NULL);
+    g_assert(QTAILQ_EMPTY(&list->head));
+    g_assert_cmpstr(list->name, ==, "opts_list_01");
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+
+    /* create the opts */
+    opts = qemu_opts_create(list, NULL, 0, &error_abort);
+    g_assert(opts != NULL);
+    g_assert(!QTAILQ_EMPTY(&list->head));
+
+    /* haven't set anything to str2 yet */
+    opt = qemu_opt_get(opts, "str2");
+    g_assert(opt == NULL);
+
+    qemu_opt_set(opts, "str2", "value");
+
+    /* now we have set str2, should know about it */
+    opt = qemu_opt_get(opts, "str2");
+    g_assert_cmpstr(opt, ==, "value");
+
+    qemu_opt_set(opts, "str2", "value2");
+
+    /* having reset the value, the returned should be the reset one */
+    opt = qemu_opt_get(opts, "str2");
+    g_assert_cmpstr(opt, ==, "value2");
+
+    qemu_opts_del(opts);
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+}
+
+static void test_qemu_opt_get_bool(void)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    bool opt;
+    int ret;
+
+    register_opts();
+
+    list = qemu_find_opts("opts_list_02");
+    g_assert(list != NULL);
+    g_assert(QTAILQ_EMPTY(&list->head));
+    g_assert_cmpstr(list->name, ==, "opts_list_02");
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+
+    /* create the opts */
+    opts = qemu_opts_create(list, NULL, 0, &error_abort);
+    g_assert(opts != NULL);
+    g_assert(!QTAILQ_EMPTY(&list->head));
+
+    /* haven't set anything to bool1 yet, so defval should be returned */
+    opt = qemu_opt_get_bool(opts, "bool1", false);
+    g_assert(opt == false);
+
+    ret = qemu_opt_set_bool(opts, "bool1", true);
+    g_assert(ret == 0);
+
+    /* now we have set bool1, should know about it */
+    opt = qemu_opt_get_bool(opts, "bool1", false);
+    g_assert(opt == true);
+
+    /* having reset the value, opt should be the reset one not defval */
+    ret = qemu_opt_set_bool(opts, "bool1", false);
+    g_assert(ret == 0);
+
+    opt = qemu_opt_get_bool(opts, "bool1", true);
+    g_assert(opt == false);
+
+    qemu_opts_del(opts);
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+}
+
+static void test_qemu_opt_get_number(void)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    uint64_t opt;
+    int ret;
+
+    register_opts();
+
+    list = qemu_find_opts("opts_list_01");
+    g_assert(list != NULL);
+    g_assert(QTAILQ_EMPTY(&list->head));
+    g_assert_cmpstr(list->name, ==, "opts_list_01");
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+
+    /* create the opts */
+    opts = qemu_opts_create(list, NULL, 0, &error_abort);
+    g_assert(opts != NULL);
+    g_assert(!QTAILQ_EMPTY(&list->head));
+
+    /* haven't set anything to number1 yet, so defval should be returned */
+    opt = qemu_opt_get_number(opts, "number1", 5);
+    g_assert(opt == 5);
+
+    ret = qemu_opt_set_number(opts, "number1", 10);
+    g_assert(ret == 0);
+
+    /* now we have set number1, should know about it */
+    opt = qemu_opt_get_number(opts, "number1", 5);
+    g_assert(opt == 10);
+
+    /* having reset it, the returned should be the reset one not defval */
+    ret = qemu_opt_set_number(opts, "number1", 15);
+    g_assert(ret == 0);
+
+    opt = qemu_opt_get_number(opts, "number1", 5);
+    g_assert(opt == 15);
+
+    qemu_opts_del(opts);
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+}
+
+static void test_qemu_opt_get_size(void)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    uint64_t opt;
+    QDict *dict;
+
+    register_opts();
+
+    list = qemu_find_opts("opts_list_02");
+    g_assert(list != NULL);
+    g_assert(QTAILQ_EMPTY(&list->head));
+    g_assert_cmpstr(list->name, ==, "opts_list_02");
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+
+    /* create the opts */
+    opts = qemu_opts_create(list, NULL, 0, &error_abort);
+    g_assert(opts != NULL);
+    g_assert(!QTAILQ_EMPTY(&list->head));
+
+    /* haven't set anything to size1 yet, so defval should be returned */
+    opt = qemu_opt_get_size(opts, "size1", 5);
+    g_assert(opt == 5);
+
+    dict = qdict_new();
+    g_assert(dict != NULL);
+
+    qdict_put(dict, "size1", qstring_from_str("10"));
+
+    qemu_opts_absorb_qdict(opts, dict, &error_abort);
+    g_assert(error_abort == NULL);
+
+    /* now we have set size1, should know about it */
+    opt = qemu_opt_get_size(opts, "size1", 5);
+    g_assert(opt == 10);
+
+    /* reset value */
+    qdict_put(dict, "size1", qstring_from_str("15"));
+
+    qemu_opts_absorb_qdict(opts, dict, &error_abort);
+    g_assert(error_abort == NULL);
+
+    /* test the reset value */
+    opt = qemu_opt_get_size(opts, "size1", 5);
+    g_assert(opt == 15);
+
+    qdict_del(dict, "size1");
+    g_free(dict);
+
+    qemu_opts_del(opts);
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+}
+
+static void test_qemu_opt_unset(void)
+{
+    QemuOpts *opts;
+    const char *value;
+    int ret;
+
+    /* dinamically initialized (parsed) opts */
+    opts = qemu_opts_parse(&opts_list_03, "key=value", 0);
+    g_assert(opts != NULL);
+
+    /* check default/parsed value */
+    value = qemu_opt_get(opts, "key");
+    g_assert_cmpstr(value, ==, "value");
+
+    /* reset it to value2 */
+    qemu_opt_set(opts, "key", "value2");
+
+    value = qemu_opt_get(opts, "key");
+    g_assert_cmpstr(value, ==, "value2");
+
+    /* unset, valid only for "accept any" */
+    ret = qemu_opt_unset(opts, "key");
+    g_assert(ret == 0);
+
+    /* after reset the value should be the parsed/default one */
+    value = qemu_opt_get(opts, "key");
+    g_assert_cmpstr(value, ==, "value");
+
+    qemu_opts_del(opts);
+}
+
+static void test_qemu_opts_reset(void)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    uint64_t opt;
+    int ret;
+
+    register_opts();
+
+    list = qemu_find_opts("opts_list_01");
+    g_assert(list != NULL);
+    g_assert(QTAILQ_EMPTY(&list->head));
+    g_assert_cmpstr(list->name, ==, "opts_list_01");
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+
+    /* create the opts */
+    opts = qemu_opts_create(list, NULL, 0, &error_abort);
+    g_assert(opts != NULL);
+    g_assert(!QTAILQ_EMPTY(&list->head));
+
+    /* haven't set anything to number1 yet, so defval should be returned */
+    opt = qemu_opt_get_number(opts, "number1", 5);
+    g_assert(opt == 5);
+
+    ret = qemu_opt_set_number(opts, "number1", 10);
+    g_assert(ret == 0);
+
+    /* now we have set number1, should know about it */
+    opt = qemu_opt_get_number(opts, "number1", 5);
+    g_assert(opt == 10);
+
+    qemu_opts_reset(list);
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+}
+
+static void test_qemu_opts_set(void)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    int ret;
+    const char *opt;
+
+    register_opts();
+
+    list = qemu_find_opts("opts_list_01");
+    g_assert(list != NULL);
+    g_assert(QTAILQ_EMPTY(&list->head));
+    g_assert_cmpstr(list->name, ==, "opts_list_01");
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+
+    /* implicitly create opts and set str3 value */
+    ret = qemu_opts_set(list, NULL, "str3", "value");
+    g_assert(ret == 0);
+    g_assert(!QTAILQ_EMPTY(&list->head));
+
+    /* get the just created opts */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts != NULL);
+
+    /* check the str3 value */
+    opt = qemu_opt_get(opts, "str3");
+    g_assert_cmpstr(opt, ==, "value");
+
+    qemu_opts_del(opts);
+
+    /* should not find anything at this point */
+    opts = qemu_opts_find(list, NULL);
+    g_assert(opts == NULL);
+}
+
+int main(int argc, char *argv[])
+{
+    g_test_init(&argc, &argv, NULL);
+    g_test_add_func("/qemu-opts/find_unknown_opts", test_find_unknown_opts);
+    g_test_add_func("/qemu-opts/find_opts", test_qemu_find_opts);
+    g_test_add_func("/qemu-opts/opts_create", test_qemu_opts_create);
+    g_test_add_func("/qemu-opts/opt_get", test_qemu_opt_get);
+    g_test_add_func("/qemu-opts/opt_get_bool", test_qemu_opt_get_bool);
+    g_test_add_func("/qemu-opts/opt_get_number", test_qemu_opt_get_number);
+    g_test_add_func("/qemu-opts/opt_get_size", test_qemu_opt_get_size);
+    g_test_add_func("/qemu-opts/opt_unset", test_qemu_opt_unset);
+    g_test_add_func("/qemu-opts/opts_reset", test_qemu_opts_reset);
+    g_test_add_func("/qemu-opts/opts_set", test_qemu_opts_set);
+    g_test_run();
+    return 0;
+}
-- 
1.9.0

Re: [Qemu-devel] [PATCH v2] QemuOpt: add unit tests

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]

On 03/17/2014 05:10 PM, Leandro Dorileo wrote:
> Cover basic aspects and API usage for QemuOpt. The current implementation
> covers the API's planned to be changed by Chunyan Liu in his QEMUOptionParameter
> replacement/cleanup job.
> 
> Other APIs should be covered in future improvements.
> 
> Signed-off-by: Leandro Dorileo <l@dorileo.org>

Right here is where you should stick a --- marker.

> 
> Changes:
>   v2:
>      + fixed comments;
>      + make use of g_assert_cmpstr();
>      + use error_abort instead of a local_err for qemu_opts_absorb_qdict();
>      + asserts on QemuOptsList (empty and list name);
>      + added test_qemu_opt_unset();
>      + asserts on qemu_opt_*_set() return;
>      + added test_qemu_opts_reset();
>      + added test_qemu_opts_set();
> ---

It's okay to have a duplicate one; but the main point is that the v2
changelog is useful to reviewers but not to the git log; and anything
after the --- marker gets omitted by 'git am' when a maintainer accepts
your patch into their pull request.

>  tests/Makefile         |   3 +
>  tests/test-qemu-opts.c | 455 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 458 insertions(+)
>  create mode 100644 tests/test-qemu-opts.c
> 
> +static void test_qemu_find_opts(void)
> +{
> +    QemuOptsList *list;
> +
> +    register_opts();
> +
> +    /* we have an "opts_list_01" option, should return it */
> +    list = qemu_find_opts("opts_list_01");
> +    g_assert(list != NULL);
> +    g_assert_cmpstr(list->name, ==, "opts_list_01");
> +}
> +
> +static void test_qemu_opts_create(void)
> +{
> +    QemuOptsList *list;
> +    QemuOpts *opts;
> +
> +    register_opts();
> +
> +    list = qemu_find_opts("opts_list_01");
> +    g_assert(list != NULL);
> +    g_assert(QTAILQ_EMPTY(&list->head));
> +    g_assert_cmpstr(list->name, ==, "opts_list_01");

This test starts as a duplicate of test_qemu_find_opts; I guess that's
okay (having both tests means a bit more insight into what broke if one
fails but the other passes).

> +
> +static void test_qemu_opt_unset(void)
> +{
> +    QemuOpts *opts;
> +    const char *value;
> +    int ret;
> +
> +    /* dinamically initialized (parsed) opts */

s/dinamically/dynamically/

That's trivially fixable, so feel free to add:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2] QemuOpt: add unit tests

[Leandro Dorileo]

Hi Eric,

On Fri, Mar 21, 2014 at 08:37:40AM -0600, Eric Blake wrote:
> On 03/17/2014 05:10 PM, Leandro Dorileo wrote:
> > Cover basic aspects and API usage for QemuOpt. The current implementation
> > covers the API's planned to be changed by Chunyan Liu in his QEMUOptionParameter
> > replacement/cleanup job.
> > 
> > Other APIs should be covered in future improvements.
> > 
> > Signed-off-by: Leandro Dorileo <l@dorileo.org>
> 
> Right here is where you should stick a --- marker.
> 
> > 
> > Changes:
> >   v2:
> >      + fixed comments;
> >      + make use of g_assert_cmpstr();
> >      + use error_abort instead of a local_err for qemu_opts_absorb_qdict();
> >      + asserts on QemuOptsList (empty and list name);
> >      + added test_qemu_opt_unset();
> >      + asserts on qemu_opt_*_set() return;
> >      + added test_qemu_opts_reset();
> >      + added test_qemu_opts_set();
> > ---
> 
> It's okay to have a duplicate one; but the main point is that the v2
> changelog is useful to reviewers but not to the git log; and anything
> after the --- marker gets omitted by 'git am' when a maintainer accepts
> your patch into their pull request.

I would say that I even know about the --- marker, but have misplaced it... :(

> 
> >  tests/Makefile         |   3 +
> >  tests/test-qemu-opts.c | 455 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 458 insertions(+)
> >  create mode 100644 tests/test-qemu-opts.c
> > 
> > +static void test_qemu_find_opts(void)
> > +{
> > +    QemuOptsList *list;
> > +
> > +    register_opts();
> > +
> > +    /* we have an "opts_list_01" option, should return it */
> > +    list = qemu_find_opts("opts_list_01");
> > +    g_assert(list != NULL);
> > +    g_assert_cmpstr(list->name, ==, "opts_list_01");
> > +}
> > +
> > +static void test_qemu_opts_create(void)
> > +{
> > +    QemuOptsList *list;
> > +    QemuOpts *opts;
> > +
> > +    register_opts();
> > +
> > +    list = qemu_find_opts("opts_list_01");
> > +    g_assert(list != NULL);
> > +    g_assert(QTAILQ_EMPTY(&list->head));
> > +    g_assert_cmpstr(list->name, ==, "opts_list_01");
> 
> This test starts as a duplicate of test_qemu_find_opts; I guess that's
> okay (having both tests means a bit more insight into what broke if one
> fails but the other passes).
> 
> > +
> > +static void test_qemu_opt_unset(void)
> > +{
> > +    QemuOpts *opts;
> > +    const char *value;
> > +    int ret;
> > +
> > +    /* dinamically initialized (parsed) opts */
> 
> s/dinamically/dynamically/
> 
> That's trivially fixable, so feel free to add:

Ok...

> 
> Reviewed-by: Eric Blake <eblake@redhat.com>


Thanks for reviewing...

-- 
Leandro Dorileo

Re: [Qemu-devel] [PATCH v2] QemuOpt: add unit tests

[Laszlo Ersek]

On 03/21/14 15:56, Leandro Dorileo wrote:
> Hi Eric,
> 
> On Fri, Mar 21, 2014 at 08:37:40AM -0600, Eric Blake wrote:
>> On 03/17/2014 05:10 PM, Leandro Dorileo wrote:
>>> Cover basic aspects and API usage for QemuOpt. The current implementation
>>> covers the API's planned to be changed by Chunyan Liu in his QEMUOptionParameter
>>> replacement/cleanup job.
>>>
>>> Other APIs should be covered in future improvements.
>>>
>>> Signed-off-by: Leandro Dorileo <l@dorileo.org>
>>
>> Right here is where you should stick a --- marker.
>>
>>>
>>> Changes:
>>>   v2:
>>>      + fixed comments;
>>>      + make use of g_assert_cmpstr();
>>>      + use error_abort instead of a local_err for qemu_opts_absorb_qdict();
>>>      + asserts on QemuOptsList (empty and list name);
>>>      + added test_qemu_opt_unset();
>>>      + asserts on qemu_opt_*_set() return;
>>>      + added test_qemu_opts_reset();
>>>      + added test_qemu_opts_set();
>>> ---
>>
>> It's okay to have a duplicate one; but the main point is that the v2
>> changelog is useful to reviewers but not to the git log; and anything
>> after the --- marker gets omitted by 'git am' when a maintainer accepts
>> your patch into their pull request.
> 
> I would say that I even know about the --- marker, but have misplaced it... :(

Off-topic: I suggest to include a reference to git-notes(1) in our patch
submission guidelines.
- git-notes(1) lets you manage such v(n)->v(n+1) changelogs inside git,
- the notes are pushable,
- they are carried across rebases,
- they are *not* part of the commit messages (consequently, they are not
part of the commit hashes either),
- they are (can be) correctly displayed by git-log, git-show, gitk, and
git-format-patch (notably, in the last case, under the --- separator)

When you start using git-notes, you don't understand how you could exist
without it.

Git-notes(1) takes some minimal configuration before use; the web offers
easily searchable, good advice.

Thanks,
Laszlo

Re: [Qemu-devel] [PATCH v2] QemuOpt: add unit tests

[Leandro Dorileo]

Hi Laszlo,

On Fri, Mar 21, 2014 at 04:30:32PM +0100, Laszlo Ersek wrote:
> On 03/21/14 15:56, Leandro Dorileo wrote:
> > Hi Eric,
> > 
> > On Fri, Mar 21, 2014 at 08:37:40AM -0600, Eric Blake wrote:
> >> On 03/17/2014 05:10 PM, Leandro Dorileo wrote:
> >>> Cover basic aspects and API usage for QemuOpt. The current implementation
> >>> covers the API's planned to be changed by Chunyan Liu in his QEMUOptionParameter
> >>> replacement/cleanup job.
> >>>
> >>> Other APIs should be covered in future improvements.
> >>>
> >>> Signed-off-by: Leandro Dorileo <l@dorileo.org>
> >>
> >> Right here is where you should stick a --- marker.
> >>
> >>>
> >>> Changes:
> >>>   v2:
> >>>      + fixed comments;
> >>>      + make use of g_assert_cmpstr();
> >>>      + use error_abort instead of a local_err for qemu_opts_absorb_qdict();
> >>>      + asserts on QemuOptsList (empty and list name);
> >>>      + added test_qemu_opt_unset();
> >>>      + asserts on qemu_opt_*_set() return;
> >>>      + added test_qemu_opts_reset();
> >>>      + added test_qemu_opts_set();
> >>> ---
> >>
> >> It's okay to have a duplicate one; but the main point is that the v2
> >> changelog is useful to reviewers but not to the git log; and anything
> >> after the --- marker gets omitted by 'git am' when a maintainer accepts
> >> your patch into their pull request.
> > 
> > I would say that I even know about the --- marker, but have misplaced it... :(
> 
> Off-topic: I suggest to include a reference to git-notes(1) in our patch
> submission guidelines.
> - git-notes(1) lets you manage such v(n)->v(n+1) changelogs inside git,
> - the notes are pushable,
> - they are carried across rebases,
> - they are *not* part of the commit messages (consequently, they are not
> part of the commit hashes either),
> - they are (can be) correctly displayed by git-log, git-show, gitk, and
> git-format-patch (notably, in the last case, under the --- separator)
> 
> When you start using git-notes, you don't understand how you could exist
> without it.

Definitely, I wasn't aware of git-notes. Thanks...

> 
> Git-notes(1) takes some minimal configuration before use; the web offers
> easily searchable, good advice.
> 
> Thanks,
> Laszlo

-- 
Leandro Dorileo