Re: [libvirt] [PATCH v4 3/5] qemu: Implement virDomainGetHostnameFlags
On 1/10/20 2:05 PM, Erik Skultety wrote:
On Thu, Jan 09, 2020 at 01:45:58PM +0100, Michal Privoznik wrote:
> From: Julio Faracco <jcfaracco(a)gmail.com>
>
> We have to keep the default - querying the agent if no flag is
> set.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
> ---
...
> +
> +static int
> +qemuDomainGetHostnameLease(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + char **hostname)
...
> +
> + if ((n_leases = virNetworkGetDHCPLeases(network, macaddr,
> + &leases, 0)) < 0)
> + goto endjob;
> +
> + for (j = 0; j < n_leases; j++) {
> + virNetworkDHCPLeasePtr lease = leases[j];
> + if (lease->hostname && !*hostname)
> + *hostname = g_strdup(lease->hostname);
Not a big deal, but why not doing a break once you extract the first hostname?
We need to free all leases to avoid memleak. Instead of doing that in
two separate loops (one where we'd look for the hostname, the other just
for freeing), I find it simpler if done in a single loop.
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
Michal