[libvirt] [PATCH v1 1/3] qemu_conf.c: introduce qemuAddRemoveSharedHostdevInternal

[Michal Privoznik]

On 9/4/19 1:06 AM, Daniel Henrique Barboza wrote:
> qemuAddSharedHostdev() has a code similar to
> qemuRemoveSharedHostdev(), with exception of one line that
> defines the operation (add or remove).
> 
> This patch introduces a new function that aggregates the common
> code, using a flag to switch between the operations, avoiding
> code repetition.
> 
> No functional change was made.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
> ---
>   src/qemu/qemu_conf.c | 93 ++++++++++++++++++--------------------------
>   1 file changed, 37 insertions(+), 56 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index d771bb6916..a583440807 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1722,9 +1722,33 @@ qemuGetHostdevPath(virDomainHostdevDefPtr hostdev)
>   
>   
>   static int
> -qemuAddSharedHostdev(virQEMUDriverPtr driver,
> -                     virDomainHostdevDefPtr hostdev,
> -                     const char *name)
> +qemuSharedDeviceEntryRemove(virQEMUDriverPtr driver,
> +                            const char *key,
> +                            const char *name)
> +{
> +    qemuSharedDeviceEntryPtr entry = NULL;
> +    int idx;
> +
> +    if (!(entry = virHashLookup(driver->sharedDevices, key)))
> +        return -1;
> +
> +    /* Nothing to do if the shared disk is not recored in the table. */
> +    if (!qemuSharedDeviceEntryDomainExists(entry, name, &idx))
> +        return 0;
> +
> +    if (entry->ref != 1)
> +        VIR_DELETE_ELEMENT(entry->domains, idx, entry->ref);
> +    else
> +        ignore_value(virHashRemoveEntry(driver->sharedDevices, key));
> +
> +    return 0;
> +}
> +
> +
> +static int
> +qemuAddRemoveSharedHostdevInternal(virQEMUDriverPtr driver,
> +                                   virDomainHostdevDefPtr hostdev,
> +                                   const char *name, bool addDevice)

How about qemuSharedHostdevAddRemoveInternal() for a name? It follows 
"object" + "operation" naming schema better.

>   {
>       char *dev_path = NULL;
>       char *key = NULL;

Also, this can be turned into VIR_AUTOFREE().

Michal