[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemu_agent: support guest-info command



[I am pasting your patch here so I can point out some nits]

On 03.07.2012 02:56, MATSUDA, Daiki wrote:
> diff -uNrp libvirt-0.9.13.orig/daemon/remote.c libvirt-0.9.13/daemon/remote.c
> --- libvirt-0.9.13.orig/daemon/remote.c	2012-06-25 16:06:18.000000000 +0900
> +++ libvirt-0.9.13/daemon/remote.c	2012-07-02 10:02:54.400454470 +0900
> @@ -3923,6 +3923,42 @@ cleanup:
>      return rv;
>  }
>  
> +
> +static int
> +qemuDispatchGuestAgentCommand(virNetServerPtr server ATTRIBUTE_UNUSED,
> +                              virNetServerClientPtr client ATTRIBUTE_UNUSED,
> +                              virNetMessagePtr msg ATTRIBUTE_UNUSED,
> +                              virNetMessageErrorPtr rerr,
> +                              qemu_guest_agent_command_args *args,
> +                              qemu_guest_agent_command_ret *ret)
> +{
> +    virDomainPtr dom = NULL;
> +    int rv = -1;
> +    struct daemonClientPrivate *priv =
> +        virNetServerClientGetPrivateData(client);
> +
> +    if (!priv->conn) {
> +        virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
> +        goto cleanup;
> +    }
> +
> +    if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
> +        goto cleanup;
> +
> +    if (virDomainGuestAgentCommand(dom, args->cmd, &ret->result, args->flags) <0) {
> +        virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("Guest Agent Error"));

This is wrong as it overwrites any error reported by virDomainGuestAgentCommand.
Moreover, there should be space between '<' and '0' in if statement which is btw longer than 80 characters.

> +        goto cleanup;
> +    }
> +
> +    rv = 0;
> +cleanup:
> +    if (rv < 0)
> +        virNetMessageSaveError(rerr);
> +    if (dom)
> +        virDomainFree(dom);
> +    return rv;
> +}
> +
>  /*----- Helpers. -----*/
>  
>  /* get_nonnull_domain and get_nonnull_network turn an on-wire
> diff -uNrp libvirt-0.9.13.orig/include/libvirt/libvirt-qemu.h libvirt-0.9.13/include/libvirt/libvirt-qemu.h
> --- libvirt-0.9.13.orig/include/libvirt/libvirt-qemu.h	2012-03-06 22:59:21.000000000 +0900
> +++ libvirt-0.9.13/include/libvirt/libvirt-qemu.h	2012-07-02 10:02:54.401454277 +0900
> @@ -32,6 +32,9 @@ virDomainPtr virDomainQemuAttach(virConn
>                                   unsigned int pid_value,
>                                   unsigned int flags);
>  
> +int virDomainGuestAgentCommand(virDomainPtr domain, const char *cmd,
> +                               char **result, unsigned int flags);
> +

Do we really want this name? I mean in case of monitor we are qemu specific. So I think this should be either virDomainQemuAgentCommand or virDomainQemuGuestAgentCommand. We expose this as qemu-agent-command after all!

>  # ifdef __cplusplus
>  }
>  # endif
> diff -uNrp libvirt-0.9.13.orig/python/generator.py libvirt-0.9.13/python/generator.py
> --- libvirt-0.9.13.orig/python/generator.py	2012-06-25 16:06:18.000000000 +0900
> +++ libvirt-0.9.13/python/generator.py	2012-07-02 10:03:01.616454719 +0900
> @@ -429,6 +429,7 @@ skip_impl = (
>  
>  qemu_skip_impl = (
>      'virDomainQemuMonitorCommand',
> +    'virDomainGuestAgentCommand',
>  )
>  
>  
> diff -uNrp libvirt-0.9.13.orig/python/libvirt-qemu-override-api.xml libvirt-0.9.13/python/libvirt-qemu-override-api.xml
> --- libvirt-0.9.13.orig/python/libvirt-qemu-override-api.xml	2011-09-14 15:24:44.000000000 +0900
> +++ libvirt-0.9.13/python/libvirt-qemu-override-api.xml	2012-07-02 10:02:54.403454795 +0900
> @@ -8,5 +8,12 @@
>          <arg name='cmd' type='const char *' info='the command which will be passed to QEMU monitor'/>
>          <arg name='flags' type='unsigned int' info='an OR&apos;ed set of virDomainQemuMonitorCommandFlags'/>
>        </function>
> +      <function name='virDomainGuestAgentCommand' file='python-qemu'>
> +        <info>Send a Guest Agent command to domain</info>
> +        <return type='str *' info='the command output or None in case of error'/>
> +        <arg name='domain' type='virDomainPtr' info='pointer to the domain'/>
> +        <arg name='cmd' type='const char *' info='guest agent command on domain'/>
> +        <arg name='flags' type='unsigned int' info='execution flags'/>
> +      </function>
>    </symbols>
>  </api>
> diff -uNrp libvirt-0.9.13.orig/python/libvirt-qemu-override.c libvirt-0.9.13/python/libvirt-qemu-override.c
> --- libvirt-0.9.13.orig/python/libvirt-qemu-override.c	2012-03-30 11:45:28.000000000 +0900
> +++ libvirt-0.9.13/python/libvirt-qemu-override.c	2012-07-02 10:02:54.404454019 +0900
> @@ -82,6 +82,35 @@ libvirt_qemu_virDomainQemuMonitorCommand
>      return py_retval;
>  }
>  
> +static PyObject *
> +libvirt_qemu_virDomainGuestAgentCommand(PyObject *self ATTRIBUTE_UNUSED, PyObject *args)
> +{
> +    PyObject *py_retval;
> +    char *result = NULL;
> +    virDomainPtr domain;
> +    PyObject *pyobj_domain;
> +    unsigned int flags;
> +    char *cmd;
> +    int c_retval;
> +
> +    if (!PyArg_ParseTuple(args, (char *)"Ozi:virDomainGuestAgentCommand",
> +                          &pyobj_domain, &cmd, &flags))
> +        return NULL;
> +    domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain);
> +
> +    if (domain == NULL)
> +        return VIR_PY_NONE;
> +    LIBVIRT_BEGIN_ALLOW_THREADS;
> +    c_retval = virDomainGuestAgentCommand(domain, cmd, &result, flags);
> +    LIBVIRT_END_ALLOW_THREADS;
> +
> +    if (c_retval < 0)
> +        return VIR_PY_NONE;
> +
> +    py_retval = PyString_FromString(result);
> +    return py_retval;
> +}
> +
>  /************************************************************************
>   *									*
>   *			The registration stuff				*
> @@ -90,6 +119,7 @@ libvirt_qemu_virDomainQemuMonitorCommand
>  static PyMethodDef libvirtQemuMethods[] = {
>  #include "libvirt-qemu-export.c"
>      {(char *) "virDomainQemuMonitorCommand", libvirt_qemu_virDomainQemuMonitorCommand, METH_VARARGS, NULL},
> +    {(char *) "virDomainGuestAgentCommand", libvirt_qemu_virDomainGuestAgentCommand, METH_VARARGS, NULL},
>      {NULL, NULL, 0, NULL}
>  };
>  
> diff -uNrp libvirt-0.9.13.orig/src/driver.h libvirt-0.9.13/src/driver.h
> --- libvirt-0.9.13.orig/src/driver.h	2012-06-25 16:06:18.000000000 +0900
> +++ libvirt-0.9.13/src/driver.h	2012-07-02 10:02:54.405454285 +0900
> @@ -860,6 +860,10 @@ typedef char *
>                                 const char *uri,
>                                 unsigned int flags);
>  
> +typedef int
> +    (*virDrvDomainGuestAgentCommand)(virDomainPtr domain, const char *cmd,
> +                                     char **result, unsigned int flags);
> +
>  /**
>   * _virDriver:
>   *
> @@ -1041,6 +1045,7 @@ struct _virDriver {
>      virDrvDomainGetDiskErrors           domainGetDiskErrors;
>      virDrvDomainSetMetadata             domainSetMetadata;
>      virDrvDomainGetMetadata             domainGetMetadata;
> +    virDrvDomainGuestAgentCommand       qemuDomainGuestAgentCommand;
>  };

I'd move these two as close as possible to virDrvDomainQemuMonitorCommand so we have similar commands grouped.

>  
>  typedef int
> diff -uNrp libvirt-0.9.13.orig/src/libvirt-qemu.c libvirt-0.9.13/src/libvirt-qemu.c
> --- libvirt-0.9.13.orig/src/libvirt-qemu.c	2012-05-31 23:23:22.000000000 +0900
> +++ libvirt-0.9.13/src/libvirt-qemu.c	2012-07-02 10:02:54.407454078 +0900
> @@ -185,3 +185,48 @@ error:
>      virDispatchError(conn);
>      return NULL;
>  }
> +
> +/**
> + * virDomainGuestAgentCommand:
> + * @domain: a domain object
> + * @cmd: the guest agent command string
> + * @result: a string returned by @cmd
> + * @flags: execution flags
> + *
> + * Execution Guest Agent Command
> + *
> + * Returns 0 if succeeded, -1 in failing.
> + */
> +int
> +virDomainGuestAgentCommand(virDomainPtr domain,
> +                          const char *cmd,
> +                          char **result,
> +                          unsigned int flags)
> +{
> +    virConnectPtr conn;
> +

VIR_DEBUG() or equivalent is missing here. Each API must log arguments it was called with.

> +    if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> +        virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }
> +
> +    conn = domain->conn;
> +
> +    virCheckNonNullArgGoto(result, error);
> +
> +    if (conn->driver->qemuDomainGuestAgentCommand) {
> +        int ret;
> +        ret = conn->driver->qemuDomainGuestAgentCommand(domain, cmd, result, flags);

Long line.

> +        if (ret < 0)
> +            goto error;
> +        return ret;
> +    }
> +
> +    virLibConnError(conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
> +
> +error:
> +    /* Copy to connection error object for back compatability */
> +    virDispatchError(domain->conn);
> +    return -1;
> +}
> diff -uNrp libvirt-0.9.13.orig/src/libvirt_qemu.syms libvirt-0.9.13/src/libvirt_qemu.syms
> --- libvirt-0.9.13.orig/src/libvirt_qemu.syms	2011-07-14 12:00:39.000000000 +0900
> +++ libvirt-0.9.13/src/libvirt_qemu.syms	2012-07-02 10:02:54.407454078 +0900
> @@ -19,3 +19,8 @@ LIBVIRT_QEMU_0.9.4 {
>      global:
>          virDomainQemuAttach;
>  } LIBVIRT_QEMU_0.8.3;
> +
> +LIBVIRT_QEMU_0.9.14 {
> +    global:
> +        virDomainGuestAgentCommand;
> +} LIBVIRT_QEMU_0.9.4;
> diff -uNrp libvirt-0.9.13.orig/src/qemu/qemu_agent.c libvirt-0.9.13/src/qemu/qemu_agent.c
> --- libvirt-0.9.13.orig/src/qemu/qemu_agent.c	2012-06-25 16:06:18.000000000 +0900
> +++ libvirt-0.9.13/src/qemu/qemu_agent.c	2012-07-02 10:02:54.408488163 +0900
> @@ -1410,3 +1410,29 @@ qemuAgentSuspend(qemuAgentPtr mon,
>      virJSONValueFree(reply);
>      return ret;
>  }
> +
> +int qemuAgentGuestAgentCommand(qemuAgentPtr mon,
> +                               const char *cmd,
> +                               char **result)
> +{
> +    int ret = -1;
> +    virJSONValuePtr jcmd;
> +    virJSONValuePtr reply = NULL;
> +
> +    jcmd = qemuAgentMakeCommand(cmd, NULL);
> +    if (!jcmd)
> +        return ret;

This is confusing. In case of qemu-monitor-command we require user to pass whole command string:
    {'execute':'some-dummy-command'}
which is correct as we don't lock them in {'execute':'%s'} scheme. Remember, we want them to
use commands/features not yet exposed by libvirt. So if qemu will ever come up with different
command scheme this API will remain broken. In addition - and this is even more serious - there is
no way of specifying arguments to a command, e.g. guest-sync take an integer:
     {'execute':'guest-sync', 'arguments':{'id':123}}
Therefore I think we need to drop qemuAgentMakeCommand() here and require users to do
the very same here as they do in qemu-monitor-command.

> +
> +    ret = qemuAgentCommand(mon, jcmd, &reply);
> +
> +    if (ret == 0) {
> +        ret = qemuAgentCheckError(jcmd, reply);
> +        *result = virJSONValueToString(reply);
> +    } else {
> +        *result = NULL;
> +    }
> +
> +    virJSONValueFree(jcmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
> diff -uNrp libvirt-0.9.13.orig/src/qemu/qemu_agent.h libvirt-0.9.13/src/qemu/qemu_agent.h
> --- libvirt-0.9.13.orig/src/qemu/qemu_agent.h	2012-06-25 16:06:18.000000000 +0900
> +++ libvirt-0.9.13/src/qemu/qemu_agent.h	2012-07-02 10:02:54.409454807 +0900
> @@ -80,4 +80,8 @@ int qemuAgentFSThaw(qemuAgentPtr mon);
>  
>  int qemuAgentSuspend(qemuAgentPtr mon,
>                       unsigned int target);
> +
> +int qemuAgentGuestAgentCommand(qemuAgentPtr mon,
> +                               const char *cmd,
> +                               char **result);
>  #endif /* __QEMU_AGENT_H__ */
> diff -uNrp libvirt-0.9.13.orig/src/qemu/qemu_driver.c libvirt-0.9.13/src/qemu/qemu_driver.c
> --- libvirt-0.9.13.orig/src/qemu/qemu_driver.c	2012-06-27 10:44:39.000000000 +0900
> +++ libvirt-0.9.13/src/qemu/qemu_driver.c	2012-07-02 10:02:54.419455949 +0900
> @@ -13158,6 +13158,78 @@ qemuListAllDomains(virConnectPtr conn,
>      return ret;
>  }
>  
> +static int
> +qemuDomainGuestAgentCommand(virDomainPtr domain,
> +                            const char *cmd,
> +                            char **result,
> +                            unsigned int flags ATTRIBUTE_UNUSED)
> +{
> +    struct qemud_driver *driver = domain->conn->privateData;
> +    virDomainObjPtr vm;
> +    int ret = -1;
> +    qemuDomainObjPrivatePtr priv;

virCheckFlags(0, -1); what means flags can't be ATTRIBUTE_UNUSED.

> +
> +    qemuDriverLock(driver);
> +    vm = virDomainFindByUUID(&driver->domains, domain->uuid);
> +
> +    if (!vm) {
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +        virUUIDFormat(domain->uuid, uuidstr);
> +        qemuReportError(VIR_ERR_NO_DOMAIN,
> +                         _("no domain with matching uuid '%s'"), uuidstr);
> +        goto cleanup;
> +    }

Do we really want to hold driver locked through whole API?

> +
> +    if (!virDomainObjIsActive(vm)) {
> +        qemuReportError(VIR_ERR_OPERATION_INVALID,
> +                        "%s", _("domain is not running"));
> +        goto cleanup;
> +    }
> +
> +    if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
> +        goto cleanup;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                        _("domain is not running"));
> +        goto endjob;
> +    }
> +
> +    priv = vm->privateData;
> +
> +    qemuDomainObjTaint(driver, vm, VIR_DOMAIN_TAINT_CUSTOM_MONITOR, -1);
> +
> +    if (priv->agentError) {
> +        qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                        _("QEMU guest agent is not "
> +                          "available due to an error"));
> +        goto cleanup;
> +    }
> +
> +    if (!priv->agent) {
> +        qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                        _("QEMU guest agent is not configured"));
> +        goto cleanup;
> +    }
> +
> +    qemuDomainObjEnterAgent(driver, vm);
> +    ret = qemuAgentGuestAgentCommand(priv->agent, cmd, result);
> +    qemuDomainObjExitAgent(driver, vm);
> +
> +    VIR_DEBUG ("qemu-agent-command ret: '%d' domain: '%s' string: %s", ret, vm->def->name, *result);
> +
> +endjob:
> +    if (qemuDomainObjEndJob(driver, vm) == 0) {
> +        vm = NULL;
> +    }
> +
> +cleanup:
> +    if (vm)
> +        virDomainObjUnlock(vm);
> +    qemuDriverUnlock(driver);
> +    return ret;
> +}
> +
>  static virDriver qemuDriver = {
>      .no = VIR_DRV_QEMU,
>      .name = QEMU_DRIVER_NAME,
> @@ -13323,6 +13380,7 @@ static virDriver qemuDriver = {
>      .domainPMSuspendForDuration = qemuDomainPMSuspendForDuration, /* 0.9.11 */
>      .domainPMWakeup = qemuDomainPMWakeup, /* 0.9.11 */
>      .domainGetCPUStats = qemuDomainGetCPUStats, /* 0.9.11 */
> +    .qemuDomainGuestAgentCommand = qemuDomainGuestAgentCommand, /* 0.9.14 */
>  };
>  
>  
> diff -uNrp libvirt-0.9.13.orig/src/remote/qemu_protocol.x libvirt-0.9.13/src/remote/qemu_protocol.x
> --- libvirt-0.9.13.orig/src/remote/qemu_protocol.x	2012-03-06 22:59:21.000000000 +0900
> +++ libvirt-0.9.13/src/remote/qemu_protocol.x	2012-07-02 10:02:54.421454763 +0900
> @@ -47,6 +47,16 @@ struct qemu_domain_attach_ret {
>      remote_nonnull_domain dom;
>  };
>  
> +struct qemu_guest_agent_command_args {
> +    remote_nonnull_domain dom;
> +    remote_nonnull_string cmd;
> +    u_int flags;
> +};
> +
> +struct qemu_guest_agent_command_ret {
> +    remote_nonnull_string result;
> +};
> +
>  /* Define the program number, protocol version and procedure numbers here. */
>  const QEMU_PROGRAM = 0x20008087;
>  const QEMU_PROTOCOL_VERSION = 1;
> @@ -61,5 +71,6 @@ enum qemu_procedure {
>       * are some exceptions to this rule, e.g. domainDestroy. Other APIs MAY
>       * be marked as high priority. If in doubt, it's safe to choose low. */
>      QEMU_PROC_MONITOR_COMMAND = 1, /* skipgen skipgen priority:low */
> -    QEMU_PROC_DOMAIN_ATTACH = 2 /* autogen autogen priority:low */
> +    QEMU_PROC_DOMAIN_ATTACH = 2, /* autogen autogen priority:low */
> +    QEMU_PROC_GUEST_AGENT_COMMAND = 3 /* skipgen skipgen priority:low */
>  };
> diff -uNrp libvirt-0.9.13.orig/src/remote/remote_driver.c libvirt-0.9.13/src/remote/remote_driver.c
> --- libvirt-0.9.13.orig/src/remote/remote_driver.c	2012-06-25 16:06:18.000000000 +0900
> +++ libvirt-0.9.13/src/remote/remote_driver.c	2012-07-02 10:02:54.425455211 +0900
> @@ -5127,6 +5127,44 @@ make_nonnull_domain_snapshot (remote_non
>      make_nonnull_domain(&snapshot_dst->dom, snapshot_src->domain);
>  }
>  
> +static int
> +remoteQemuDomainGuestAgentCommand (virDomainPtr domain, const char *cmd,
> +                                   char **result, unsigned int flags)
> +{
> +    int rv = -1;
> +    qemu_guest_agent_command_args args;
> +    qemu_guest_agent_command_ret ret;
> +    struct private_data *priv = domain->conn->privateData;
> +
> +    remoteDriverLock(priv);
> +
> +    make_nonnull_domain(&args.dom, domain);
> +    args.cmd = (char *)cmd;
> +    args.flags = flags;
> +
> +    memset (&ret, 0, sizeof ret);

We require sizeof to be used as function:
memset(&ret, 0, sizeof(ret));

> +
> +    if (call (domain->conn, priv, REMOTE_CALL_QEMU, QEMU_PROC_GUEST_AGENT_COMMAND,
> +              (xdrproc_t) xdr_qemu_guest_agent_command_args, (char *) &args,
> +              (xdrproc_t) xdr_qemu_guest_agent_command_ret, (char *) &ret) == -1)
> +        goto done;

Again long line, but it's among whole file so I think I can close one eye..

> +
> +    *result = strdup(ret.result);
> +    if (*result == NULL) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    rv = 0;
> +
> +cleanup:
> +    xdr_free ((xdrproc_t) xdr_qemu_guest_agent_command_ret, (char *) &ret);
> +
> +done:
> +    remoteDriverUnlock(priv);
> +    return rv;
> +}
> +
>  /*----------------------------------------------------------------------*/
>  
>  unsigned long remoteVersion(void)
> @@ -5303,6 +5339,7 @@ static virDriver remote_driver = {
>      .domainGetDiskErrors = remoteDomainGetDiskErrors, /* 0.9.10 */
>      .domainSetMetadata = remoteDomainSetMetadata, /* 0.9.10 */
>      .domainGetMetadata = remoteDomainGetMetadata, /* 0.9.10 */
> +    .qemuDomainGuestAgentCommand = remoteQemuDomainGuestAgentCommand, /* 0.9.14 */
>  };
>  
>  static virNetworkDriver network_driver = {
> diff -uNrp libvirt-0.9.13.orig/tools/virsh.c libvirt-0.9.13/tools/virsh.c
> --- libvirt-0.9.13.orig/tools/virsh.c	2012-06-28 12:05:04.000000000 +0900
> +++ libvirt-0.9.13/tools/virsh.c	2012-07-02 10:02:54.441454424 +0900
> @@ -160,6 +160,7 @@ typedef enum {
>  #define VSH_CMD_GRP_SNAPSHOT         "Snapshot"
>  #define VSH_CMD_GRP_HOST_AND_HV      "Host and Hypervisor"
>  #define VSH_CMD_GRP_VIRSH            "Virsh itself"
> +#define VSH_CMD_GRP_GUESTAGENT       "QEMU Guest Agent"

Why this? I mean, qemu-monitor-command is under VSH_CMD_GRP_HOST_AND_HV so I think we should keep things consistent.

>  
>  /*
>   * Command Option Flags
> @@ -18138,6 +18139,68 @@ cleanup:
>      return ret;
>  }
>  
> +/*
> + * "qemu-agent-command" command
> + */
> +static const vshCmdInfo info_qemu_agent_command[] = {
> +    {"help", N_("Qemu Guest Agent Command")},
> +    {"desc", N_("Qemu Guest Agent Command")},
> +    {NULL, NULL}
> +};
> +
> +static const vshCmdOptDef opts_qemu_agent_command[] = {
> +    {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
> +    {"cmd", VSH_OT_ARGV, VSH_OFLAG_REQ, N_("command")},
> +    {NULL, 0, 0, NULL}
> +};
> +
> +static bool
> +cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
> +{
> +    virDomainPtr dom = NULL;
> +    bool ret = false;
> +    char *guest_agent_cmd = NULL;
> +    char *result = NULL;
> +    unsigned int flags = 0;
> +    const vshCmdOpt *opt = NULL;
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    bool pad = false;
> +
> +    if (!vshConnectionUsability(ctl, ctl->conn))
> +        goto cleanup;
> +
> +    dom = vshCommandOptDomain(ctl, cmd, NULL);
> +    if (dom == NULL)
> +        goto cleanup;
> +
> +    while ((opt = vshCommandOptArgv(cmd, opt))) {
> +        if (pad)
> +            virBufferAddChar(&buf, ' ');
> +        pad = true;
> +        virBufferAdd(&buf, opt->data, -1);
> +    }
> +    if (virBufferError(&buf)) {
> +        vshPrint(ctl, "%s", _("Failed to collect command"));
> +        goto cleanup;
> +    }
> +    guest_agent_cmd = virBufferContentAndReset(&buf);
> +
> +    if (virDomainGuestAgentCommand(dom, guest_agent_cmd, &result, flags) < 0)
> +        goto cleanup;
> +
> +    printf("%s\n", result);
> +
> +    ret = true;
> +
> +cleanup:
> +    VIR_FREE(result);
> +    VIR_FREE(guest_agent_cmd);
> +    if (dom)
> +        virDomainFree(dom);
> +
> +    return ret;
> +}
> +
>  static const vshCmdDef domManagementCmds[] = {
>      {"attach-device", cmdAttachDevice, opts_attach_device,
>       info_attach_device, 0},
> @@ -18453,6 +18516,11 @@ static const vshCmdDef hostAndHypervisor
>      {NULL, NULL, NULL, NULL, 0}
>  };
>  
> +static const vshCmdDef guestagentCmds[] = {
> +    {"qemu-agent-command", cmdQemuAgentCommand, opts_qemu_agent_command, info_qemu_agent_command, 0},
> +    {NULL, NULL, NULL, NULL, 0}
> +};
> +
>  static const vshCmdGrp cmdGroups[] = {
>      {VSH_CMD_GRP_DOM_MANAGEMENT, "domain", domManagementCmds},
>      {VSH_CMD_GRP_DOM_MONITORING, "monitor", domMonitoringCmds},
> @@ -18465,6 +18533,7 @@ static const vshCmdGrp cmdGroups[] = {
>      {VSH_CMD_GRP_SNAPSHOT, "snapshot", snapshotCmds},
>      {VSH_CMD_GRP_STORAGE_POOL, "pool", storagePoolCmds},
>      {VSH_CMD_GRP_STORAGE_VOL, "volume", storageVolCmds},
> +    {VSH_CMD_GRP_GUESTAGENT, "guestagent", guestagentCmds},
>      {VSH_CMD_GRP_VIRSH, "virsh", virshCmds},
>      {NULL, NULL, NULL}
>


Plus, you need to update src/qemu_protocol-structs as well


For future version, can you please utilize git and git-send-email? It makes review simpler. Thanks.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]