[libvirt] [PATCH 1/9] libvirtd: Alter refcnt processing for domain server objects

[Marc Hartmayer]

On Fri, Jan 19, 2018 at 06:23 PM +0100, John Ferlan <jferlan at redhat.com> wrote:
> Once virNetDaemonAddServer returns, the @srv or @srvAdm have
> either been added to the @dmn list increasing the refcnt or
> there was an error. In either case, we can lower the refcnt
> from virNetServerNew, but not set to NULL. Thus "using" the
> hash table reference when adding programs later or allowing
> the immediate free of the object on error.
>
> The @dmn dispose function (virNetDaemonDispose) will handle
> the Unref of each object during the virHashFree when the
> object is removed from the hash table.
>
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  daemon/libvirtd.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 6d3b83355..0afd1dd82 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -1065,6 +1065,7 @@ int main(int argc, char **argv) {
>      char *remote_config_file = NULL;
>      int statuswrite = -1;
>      int ret = 1;
> +    int rc;
>      int pid_file_fd = -1;
>      char *pid_file = NULL;
>      char *sock_file = NULL;
> @@ -1319,7 +1320,11 @@ int main(int argc, char **argv) {
>          goto cleanup;
>      }
>
> -    if (virNetDaemonAddServer(dmn, srv) < 0) {
> +    /* Add @srv to @dmn servers hash table and Unref to allow removal from
> +     * hash table at @dmn disposal to decide when to free @srv */
> +    rc = virNetDaemonAddServer(dmn, srv);

Reviewed-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>

Wouldn’t it be cleaner to initialize the server and then add it to the
daemon? (disadvantage: it makes the correct unreferencing a little more
complicated).

<pseudo_code>
create_and_initialize_daemon
create_and_add_programs_server
virNetDaemonAddServer()
…
</pseudo_code>

> +    virObjectUnref(srv);
> +    if (rc < 0) {
>          ret = VIR_DAEMON_ERR_INIT;
>          goto cleanup;
>      }
> @@ -1393,7 +1398,11 @@ int main(int argc, char **argv) {
>          goto cleanup;
>      }
>
> -    if (virNetDaemonAddServer(dmn, srvAdm) < 0) {
> +    /* Add @srvAdm to @dmn servers hash table and Unref to allow removal from
> +     * hash table at @dmn disposal to decide when to free @srvAdm */
> +    rc = virNetDaemonAddServer(dmn, srvAdm);
> +    virObjectUnref(srvAdm);
> +    if (rc < 0) {
>          ret = VIR_DAEMON_ERR_INIT;
>          goto cleanup;
>      }
> @@ -1516,11 +1525,9 @@ int main(int argc, char **argv) {
>      }
>
>      virObjectUnref(adminProgram);
> -    virObjectUnref(srvAdm);
>      virObjectUnref(qemuProgram);
>      virObjectUnref(lxcProgram);
>      virObjectUnref(remoteProgram);
> -    virObjectUnref(srv);
>      virObjectUnref(dmn);
>
>      virNetlinkShutdown();
> --
> 2.13.6
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
Beste Grüße / Kind regards
   Marc Hartmayer

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294