Re: [PATCH 15/40] qemu: Convert 'priv->dbusVMStateIds' to a GSList

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 2/6/21 9:32 AM, Peter Krempa wrote:
> The conversion removes the use of virStringListAdd/virStringListRemove
> which try to add dynamic properties to a string list which is really
> inefficient.
>
> Storing the dbus VMState ids in a GSList is pretty straightforward and
> the slightly increased complexity of the code will be paid back by
> removing the string list helpers later.
>
> Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> ---
>   src/qemu/qemu_command.c      |  2 +-
>   src/qemu/qemu_dbus.c         | 19 ++++++++++++++++---
>   src/qemu/qemu_dbus.h         |  2 +-
>   src/qemu/qemu_domain.c       |  4 ++--
>   src/qemu/qemu_domain.h       |  2 +-
>   src/qemu/qemu_migration.c    | 10 ++++------
>   src/qemu/qemu_monitor.c      | 19 +++++++++++++------
>   src/qemu/qemu_monitor.h      |  2 +-
>   src/qemu/qemu_monitor_json.c |  5 ++---
>   src/qemu/qemu_monitor_json.h |  2 +-
>   src/qemu/qemu_slirp.c        |  7 ++-----
>   11 files changed, 44 insertions(+), 30 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 92036d26c0..ac6bec3389 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9605,7 +9605,7 @@ qemuBuildDBusVMStateCommandLine(virCommandPtr cmd,
>       g_autoptr(virJSONValue) props = NULL;
>       qemuDomainObjPrivatePtr priv = QEMU_DOMAIN_PRIVATE(vm);
>
> -    if (virStringListLength((const char **)priv->dbusVMStateIds) == 0)
> +    if (!priv->dbusVMStateIds)
>           return 0;
>
>       if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DBUS_VMSTATE)) {
> diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
> index ffcf83e5da..31ede2646f 100644
> --- a/src/qemu/qemu_dbus.c
> +++ b/src/qemu/qemu_dbus.c
> @@ -287,15 +287,28 @@ qemuDBusStart(virQEMUDriverPtr driver,
>   }
>
>
> -int
> +void
>   qemuDBusVMStateAdd(virDomainObjPtr vm, const char *id)
>   {
> -    return virStringListAdd(&QEMU_DOMAIN_PRIVATE(vm)->dbusVMStateIds, id);
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +
> +    priv->dbusVMStateIds = g_slist_append(priv->dbusVMStateIds, g_strdup(id));
>   }
>
>
>   void
>   qemuDBusVMStateRemove(virDomainObjPtr vm, const char *id)
>   {
> -    virStringListRemove(&QEMU_DOMAIN_PRIVATE(vm)->dbusVMStateIds, id);
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    GSList *next;
> +
> +    for (next = priv->dbusVMStateIds; next; next = next->next) {
> +        const char *elem = next->data;
> +
> +        if (STREQ(id, elem)) {
> +            priv->dbusVMStateIds = g_slist_remove_link(priv->dbusVMStateIds, next);
> +            g_slist_free_full(next, g_free);
> +            break;
> +        }
> +    }

It's rather sad that there is no better way to do this. I mean one would 
expect that removing an item from a singly linked list based on value 
not address of that value is pretty common.

Michal