Thanks for your review @John.
I will wait for 4.8.0 to push all changes in one shot (including news.xml).

Julio Cesar Faracco
Em qua, 29 de ago de 2018 às 17:15, John Ferlan <jfer...@redhat.com> escreveu:
>
>
>
> On 08/21/2018 10:39 PM, Julio Faracco wrote:
> > This commit add the support to use the function qemuAgentGetHostname()
>
> s/add the/adds/
>
> > for obtain the domain hostname using QEMU-GA command.
>
> s/for/to/
>
> >
> > Signed-off-by: Julio Faracco <jcfara...@gmail.com>
> > ---
> >  src/qemu/qemu_driver.c | 40 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 21e9e87ddd..5f53cbea15 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -19351,6 +19351,45 @@ qemuConnectGetCPUModelNames(virConnectPtr conn,
> >      return virCPUGetModels(arch, models);
> >  }
> >
>
> Nit - we prefer 2 blank lines between functions for new code.
>
> > +static char *
> > +qemuDomainGetHostname(virDomainPtr dom,
> > +                      unsigned int flags)
> > +{
> > +    virQEMUDriverPtr driver = dom->conn->privateData;
> > +    virDomainObjPtr vm = NULL;
> > +    qemuAgentPtr agent;
> > +    char *hostname = NULL;
> > +
> > +    virCheckFlags(0, NULL);
> > +
> > +    if (!(vm = qemuDomObjFromDomain(dom)))
> > +        return NULL;
> > +
> > +    if (virDomainGetHostnameEnsureACL(dom->conn, vm->def) < 0)
> > +        goto cleanup;
> > +
> > +    if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
> > +        goto cleanup;
> > +
> > +    if (virDomainObjCheckActive(vm) < 0)
> > +        goto endjob;
> > +
> > +    if (!qemuDomainAgentAvailable(vm, true))
> > +        goto endjob;
> > +
> > +    agent = qemuDomainObjEnterAgent(vm);
> > +    ignore_value(qemuAgentGetHostname(agent, &hostname));
> > +    qemuDomainObjExitAgent(vm, agent);
> > +
> > + endjob:
> > +    qemuDomainObjEndAgentJob(vm);
> > +
> > + cleanup:
> > +    virDomainObjEndAPI(&vm);
> > +    return hostname;
> > +}
> > +
> > +
>
> Ironically the 2 blanks lines were done here ;-)
>
> >  static int
> >  qemuDomainGetTime(virDomainPtr dom,
> >                    long long *seconds,
> > @@ -21955,6 +21994,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
> >      .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */
> >      .domainFSFreeze = qemuDomainFSFreeze, /* 1.2.5 */
> >      .domainFSThaw = qemuDomainFSThaw, /* 1.2.5 */
> > +    .domainGetHostname = qemuDomainGetHostname, /* 4.7.0 */
>
> This'll be 4.8.0.
>
> Again, simple enough for me to adjust before pushing once 4.8.0 opens.
>
> You'll still need to post the news.xml changes once it opens and maybe
> you can work up patches to work through those things I pointed out in
> the review of patch 1.
>
>     Reviewed-by: John Ferlan <jfer...@redhat.com>
>
> John
>
> >      .domainGetTime = qemuDomainGetTime, /* 1.2.5 */
> >      .domainSetTime = qemuDomainSetTime, /* 1.2.5 */
> >      .nodeGetFreePages = qemuNodeGetFreePages, /* 1.2.6 */
> >

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to