[libvirt] [PATCH RFC 1/4] qemu_agent: move agent into util

[Jim Fehlig]

Sorry for the review delay.

On 02/08/2017 09:44 AM, Joao Martins wrote:
> As it could be shared with libxl which now allows channels to
> be created. Also changed filename to match others in the same
> directory namely to virqemuagent.{h,c}
>
> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
> ---
>  po/POTFILES.in               |    2 +-
>  src/Makefile.am              |    2 +-
>  src/libvirt_private.syms     |   21 +
>  src/qemu/qemu_agent.c        | 2248 ------------------------------------------
>  src/qemu/qemu_agent.h        |  123 ---
>  src/qemu/qemu_domain.h       |    2 +-
>  src/qemu/qemu_driver.c       |    2 +-
>  src/util/virqemuagent.c      | 2248 ++++++++++++++++++++++++++++++++++++++++++
>  src/util/virqemuagent.h      |  123 +++
>  tests/qemuagenttest.c        |    2 +-
>  tests/qemumonitortestutils.c |    2 +-
>  tests/qemumonitortestutils.h |    2 +-
>  12 files changed, 2399 insertions(+), 2378 deletions(-)
>  delete mode 100644 src/qemu/qemu_agent.c
>  delete mode 100644 src/qemu/qemu_agent.h
>  create mode 100644 src/util/virqemuagent.c
>  create mode 100644 src/util/virqemuagent.h

I hope others will opine on this change. It seems reasonable to me and I'm 
surprised the qemu driver only needed tiny changes to accommodate moving all 
this code.

[...]

> diff --git a/src/util/virqemuagent.h b/src/util/virqemuagent.h
> new file mode 100644
> index 0000000..2e81020
> --- /dev/null
> +++ b/src/util/virqemuagent.h
> @@ -0,0 +1,123 @@
> +/*
> + * virqemuagent.h: interaction with QEMU guest agent
> + *
> + * Copyright (C) 2006-2012 Red Hat, Inc.
> + * Copyright (C) 2006 Daniel P. Berrange
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + * Author: Daniel P. Berrange <berrange at redhat.com>
> + */
> +
> +
> +#ifndef __QEMU_AGENT_H__
> +# define __QEMU_AGENT_H__
> +
> +# include "internal.h"
> +# include "domain_conf.h"

ISTR a recent discussion on the list frowning on using code from src/conf in 
src/util, although virthostdev.h also includes domain_conf.h.

BTW, compilation fails here

In file included from util/virqemuagent.c:35:0:
util/virqemuagent.h:29:26: fatal error: domain_conf.h: No such file or directory
  # include "domain_conf.h"

> +
> +typedef struct _qemuAgent qemuAgent;
> +typedef qemuAgent *qemuAgentPtr;
> +
> +typedef struct _qemuAgentCallbacks qemuAgentCallbacks;
> +typedef qemuAgentCallbacks *qemuAgentCallbacksPtr;
> +struct _qemuAgentCallbacks {
> +    void (*destroy)(qemuAgentPtr mon,
> +                    virDomainObjPtr vm);
> +    void (*eofNotify)(qemuAgentPtr mon,
> +                      virDomainObjPtr vm);
> +    void (*errorNotify)(qemuAgentPtr mon,
> +                        virDomainObjPtr vm);
> +};
> +
> +
> +qemuAgentPtr qemuAgentOpen(virDomainObjPtr vm,
> +                           const virDomainChrSourceDef *config,
> +                           qemuAgentCallbacksPtr cb);
> +
> +void qemuAgentClose(qemuAgentPtr mon);

Other files in src/util prefix structs and functions with "vir". I'm not sure 
how picky folks are about that. If the consensus is towards the "vir" prefix, 
perhaps it would be easier done with a follow-up after the move?

Thanks a lot for working on this feature! It would be cool to get it in the 
3.2.0 release.

Regards,
Jim