Re: [PATCH v3 3/3] virt-shell: Move generic commands implementation to vsh.c

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

 



I talked to Martin and Michal about this and I agree with them that
moving the command structures as well as the commands is better idea
than leaving the structures in virsh.c so I'll send a v4 inline patch.

Erik

On 13/08/15 13:39, Erik Skultety wrote:
> Generic commands like 'help', 'cd', 'pwd',etc. can be reused by any
> client, so the clients should profit from this implementation rather
> than providing their own similar implementation (if it's not intensional
> and there's a reason for this)
> ---
>  tools/virsh.c | 343 +++++++++++++++-------------------------------------------
>  tools/vsh.c   | 155 ++++++++++++++++++++++++++
>  tools/vsh.h   |   7 ++
>  3 files changed, 250 insertions(+), 255 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 2d198cd..e123ea2 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -188,79 +188,6 @@ virshReconnect(vshControl *ctl)
>      priv->blockJobNoBytes = false;
>  }
>  
> -
> -/*
> - * "connect" command
> - */
> -static const vshCmdInfo info_connect[] = {
> -    {.name = "help",
> -     .data = N_("(re)connect to hypervisor")
> -    },
> -    {.name = "desc",
> -     .data = N_("Connect to local hypervisor. This is built-in "
> -                "command after shell start up.")
> -    },
> -    {.name = NULL}
> -};
> -
> -static const vshCmdOptDef opts_connect[] = {
> -    {.name = "name",
> -     .type = VSH_OT_STRING,
> -     .flags = VSH_OFLAG_EMPTY_OK,
> -     .help = N_("hypervisor connection URI")
> -    },
> -    {.name = "readonly",
> -     .type = VSH_OT_BOOL,
> -     .help = N_("read-only connection")
> -    },
> -    {.name = NULL}
> -};
> -
> -static bool
> -cmdConnect(vshControl *ctl, const vshCmd *cmd)
> -{
> -    bool ro = vshCommandOptBool(cmd, "readonly");
> -    const char *name = NULL;
> -    virshControlPtr priv = ctl->privData;
> -
> -    if (priv->conn) {
> -        int ret;
> -
> -        virConnectUnregisterCloseCallback(priv->conn, virshCatchDisconnect);
> -        ret = virConnectClose(priv->conn);
> -        if (ret < 0)
> -            vshError(ctl, "%s", _("Failed to disconnect from the hypervisor"));
> -        else if (ret > 0)
> -            vshError(ctl, "%s", _("One or more references were leaked after "
> -                                  "disconnect from the hypervisor"));
> -        priv->conn = NULL;
> -    }
> -
> -    VIR_FREE(ctl->name);
> -    if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
> -        return false;
> -
> -    ctl->name = vshStrdup(ctl, name);
> -
> -    priv->useGetInfo = false;
> -    priv->useSnapshotOld = false;
> -    priv->blockJobNoBytes = false;
> -    priv->readonly = ro;
> -
> -    priv->conn = virshConnect(ctl, ctl->name, priv->readonly);
> -
> -    if (!priv->conn) {
> -        vshError(ctl, "%s", _("Failed to connect to the hypervisor"));
> -        return false;
> -    }
> -
> -    if (virConnectRegisterCloseCallback(priv->conn, virshCatchDisconnect,
> -                                        NULL, NULL) < 0)
> -        vshError(ctl, "%s", _("Unable to register disconnect callback"));
> -
> -    return true;
> -}
> -
>  int virshStreamSink(virStreamPtr st ATTRIBUTE_UNUSED,
>                      const char *bytes, size_t nbytes, void *opaque)
>  {
> @@ -270,13 +197,9 @@ int virshStreamSink(virStreamPtr st ATTRIBUTE_UNUSED,
>  }
>  
>  /* ---------------
> - * Commands
> + * Command Info
>   * ---------------
>   */
> -
> -/*
> - * "help" command
> - */
>  static const vshCmdInfo info_help[] = {
>      {.name = "help",
>       .data = N_("print help")
> @@ -288,55 +211,6 @@ static const vshCmdInfo info_help[] = {
>      {.name = NULL}
>  };
>  
> -static const vshCmdOptDef opts_help[] = {
> -    {.name = "command",
> -     .type = VSH_OT_STRING,
> -     .help = N_("Prints global help, command specific help, or help for a group of related commands")
> -    },
> -    {.name = NULL}
> -};
> -
> -static bool
> -cmdHelp(vshControl *ctl, const vshCmd *cmd)
> - {
> -    const char *name = NULL;
> -
> -    if (vshCommandOptString(ctl, cmd, "command", &name) <= 0) {
> -        const vshCmdGrp *grp;
> -        const vshCmdDef *def;
> -
> -        vshPrint(ctl, "%s", _("Grouped commands:\n\n"));
> -
> -        for (grp = cmdGroups; grp->name; grp++) {
> -            vshPrint(ctl, _(" %s (help keyword '%s'):\n"), grp->name,
> -                     grp->keyword);
> -
> -            for (def = grp->commands; def->name; def++) {
> -                if (def->flags & VSH_CMD_FLAG_ALIAS)
> -                    continue;
> -                vshPrint(ctl, "    %-30s %s\n", def->name,
> -                         _(vshCmddefGetInfo(def, "help")));
> -            }
> -
> -            vshPrint(ctl, "\n");
> -        }
> -
> -        return true;
> -    }
> -
> -    if (vshCmddefSearch(name)) {
> -        return vshCmddefHelp(ctl, name);
> -    } else if (vshCmdGrpSearch(name)) {
> -        return vshCmdGrpHelp(ctl, name);
> -    } else {
> -        vshError(ctl, _("command or command group '%s' doesn't exist"), name);
> -        return false;
> -    }
> -}
> -
> -/*
> - * "cd" command
> - */
>  static const vshCmdInfo info_cd[] = {
>      {.name = "help",
>       .data = N_("change the current directory")
> @@ -347,45 +221,6 @@ static const vshCmdInfo info_cd[] = {
>      {.name = NULL}
>  };
>  
> -static const vshCmdOptDef opts_cd[] = {
> -    {.name = "dir",
> -     .type = VSH_OT_STRING,
> -     .help = N_("directory to switch to (default: home or else root)")
> -    },
> -    {.name = NULL}
> -};
> -
> -static bool
> -cmdCd(vshControl *ctl, const vshCmd *cmd)
> -{
> -    const char *dir = NULL;
> -    char *dir_malloced = NULL;
> -    bool ret = true;
> -    char ebuf[1024];
> -
> -    if (!ctl->imode) {
> -        vshError(ctl, "%s", _("cd: command valid only in interactive mode"));
> -        return false;
> -    }
> -
> -    if (vshCommandOptString(ctl, cmd, "dir", &dir) <= 0)
> -        dir = dir_malloced = virGetUserDirectory();
> -    if (!dir)
> -        dir = "/";
> -
> -    if (chdir(dir) == -1) {
> -        vshError(ctl, _("cd: %s: %s"),
> -                 virStrerror(errno, ebuf, sizeof(ebuf)), dir);
> -        ret = false;
> -    }
> -
> -    VIR_FREE(dir_malloced);
> -    return ret;
> -}
> -
> -/*
> - * "pwd" command
> - */
>  static const vshCmdInfo info_pwd[] = {
>      {.name = "help",
>       .data = N_("print the current directory")
> @@ -396,29 +231,6 @@ static const vshCmdInfo info_pwd[] = {
>      {.name = NULL}
>  };
>  
> -static bool
> -cmdPwd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
> -{
> -    char *cwd;
> -    bool ret = true;
> -    char ebuf[1024];
> -
> -    cwd = getcwd(NULL, 0);
> -    if (!cwd) {
> -        vshError(ctl, _("pwd: cannot get current directory: %s"),
> -                 virStrerror(errno, ebuf, sizeof(ebuf)));
> -        ret = false;
> -    } else {
> -        vshPrint(ctl, _("%s\n"), cwd);
> -        VIR_FREE(cwd);
> -    }
> -
> -    return ret;
> -}
> -
> -/*
> - * "echo" command
> - */
>  static const vshCmdInfo info_echo[] = {
>      {.name = "help",
>       .data = N_("echo arguments")
> @@ -429,6 +241,47 @@ static const vshCmdInfo info_echo[] = {
>      {.name = NULL}
>  };
>  
> +static const vshCmdInfo info_quit[] = {
> +    {.name = "help",
> +     .data = N_("quit this interactive terminal")
> +    },
> +    {.name = "desc",
> +     .data = ""
> +    },
> +    {.name = NULL}
> +};
> +
> +static const vshCmdInfo info_connect[] = {
> +    {.name = "help",
> +     .data = N_("(re)connect to hypervisor")
> +    },
> +    {.name = "desc",
> +     .data = N_("Connect to local hypervisor. This is built-in "
> +                "command after shell start up.")
> +    },
> +    {.name = NULL}
> +};
> +
> +/* ---------------
> + * Command Opts
> + * ---------------
> + */
> +static const vshCmdOptDef opts_help[] = {
> +    {.name = "command",
> +     .type = VSH_OT_STRING,
> +     .help = N_("Prints global help, command specific help, or help for a group of related commands")
> +    },
> +    {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_cd[] = {
> +    {.name = "dir",
> +     .type = VSH_OT_STRING,
> +     .help = N_("directory to switch to (default: home or else root)")
> +    },
> +    {.name = NULL}
> +};
> +
>  static const vshCmdOptDef opts_echo[] = {
>      {.name = "shell",
>       .type = VSH_OT_BOOL,
> @@ -453,80 +306,61 @@ static const vshCmdOptDef opts_echo[] = {
>      {.name = NULL}
>  };
>  
> -/* Exists mainly for debugging virsh, but also handy for adding back
> - * quotes for later evaluation.
> - */
> +static const vshCmdOptDef opts_connect[] = {
> +    {.name = "name",
> +     .type = VSH_OT_STRING,
> +     .flags = VSH_OFLAG_EMPTY_OK,
> +     .help = N_("hypervisor connection URI")
> +    },
> +    {.name = "readonly",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("read-only connection")
> +    },
> +    {.name = NULL}
> +};
> +
>  static bool
> -cmdEcho(vshControl *ctl, const vshCmd *cmd)
> +cmdConnect(vshControl *ctl, const vshCmd *cmd)
>  {
> -    bool shell = false;
> -    bool xml = false;
> -    int count = 0;
> -    const vshCmdOpt *opt = NULL;
> -    char *arg;
> -    virBuffer buf = VIR_BUFFER_INITIALIZER;
> -
> -    if (vshCommandOptBool(cmd, "shell"))
> -        shell = true;
> -    if (vshCommandOptBool(cmd, "xml"))
> -        xml = true;
> -
> -    while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
> -        char *str;
> -        virBuffer xmlbuf = VIR_BUFFER_INITIALIZER;
> -
> -        arg = opt->data;
> -
> -        if (count)
> -            virBufferAddChar(&buf, ' ');
> -
> -        if (xml) {
> -            virBufferEscapeString(&xmlbuf, "%s", arg);
> -            if (virBufferError(&xmlbuf)) {
> -                vshPrint(ctl, "%s", _("Failed to allocate XML buffer"));
> -                return false;
> -            }
> -            str = virBufferContentAndReset(&xmlbuf);
> -        } else {
> -            str = vshStrdup(ctl, arg);
> -        }
> +    bool ro = vshCommandOptBool(cmd, "readonly");
> +    const char *name = NULL;
> +    virshControlPtr priv = ctl->privData;
>  
> -        if (shell)
> -            virBufferEscapeShell(&buf, str);
> -        else
> -            virBufferAdd(&buf, str, -1);
> -        count++;
> -        VIR_FREE(str);
> +    if (priv->conn) {
> +        int ret;
> +
> +        virConnectUnregisterCloseCallback(priv->conn, virshCatchDisconnect);
> +        ret = virConnectClose(priv->conn);
> +        if (ret < 0)
> +            vshError(ctl, "%s", _("Failed to disconnect from the hypervisor"));
> +        else if (ret > 0)
> +            vshError(ctl, "%s", _("One or more references were leaked after "
> +                                  "disconnect from the hypervisor"));
> +        priv->conn = NULL;
>      }
>  
> -    if (virBufferError(&buf)) {
> -        vshPrint(ctl, "%s", _("Failed to allocate XML buffer"));
> +    VIR_FREE(ctl->connname);
> +    if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
> +        return false;
> +
> +    ctl->connname = vshStrdup(ctl, name);
> +
> +    priv->useGetInfo = false;
> +    priv->useSnapshotOld = false;
> +    priv->blockJobNoBytes = false;
> +    priv->readonly = ro;
> +
> +    priv->conn = virshConnect(ctl, ctl->connname, priv->readonly);
> +
> +    if (!priv->conn) {
> +        vshError(ctl, "%s", _("Failed to connect to the hypervisor"));
>          return false;
>      }
> -    arg = virBufferContentAndReset(&buf);
> -    if (arg)
> -        vshPrint(ctl, "%s", arg);
> -    VIR_FREE(arg);
> -    return true;
> -}
>  
> -/*
> - * "quit" command
> - */
> -static const vshCmdInfo info_quit[] = {
> -    {.name = "help",
> -     .data = N_("quit this interactive terminal")
> -    },
> -    {.name = "desc",
> -     .data = ""
> -    },
> -    {.name = NULL}
> -};
> +    if (virConnectRegisterCloseCallback(priv->conn, virshCatchDisconnect,
> +                                        NULL, NULL) < 0)
> +        vshError(ctl, "%s", _("Unable to register disconnect callback"));
>  
> -static bool
> -cmdQuit(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
> -{
> -    ctl->imode = false;
>      return true;
>  }
>  
> @@ -535,7 +369,6 @@ cmdQuit(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
>   * ---------------
>   */
>  
> -
>  static bool
>  virshConnectionUsability(vshControl *ctl, virConnectPtr conn)
>  {
> diff --git a/tools/vsh.c b/tools/vsh.c
> index 043d0fb..084cac5 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -2748,3 +2748,158 @@ vshDeinit(vshControl *ctl)
>          vshReadlineDeinit(ctl);
>      vshCloseLogFile(ctl);
>  }
> +
> +/* -----------------------------------------------
> + * Generic commands available to use by any client
> + * -----------------------------------------------
> + */
> +
> +bool
> +cmdHelp(vshControl *ctl, const vshCmd *cmd)
> + {
> +    const char *name = NULL;
> +
> +    if (vshCommandOptString(ctl, cmd, "command", &name) <= 0) {
> +        const vshCmdGrp *grp;
> +        const vshCmdDef *def;
> +
> +        vshPrint(ctl, "%s", _("Grouped commands:\n\n"));
> +
> +        for (grp = cmdGroups; grp->name; grp++) {
> +            vshPrint(ctl, _(" %s (help keyword '%s'):\n"), grp->name,
> +                     grp->keyword);
> +
> +            for (def = grp->commands; def->name; def++) {
> +                if (def->flags & VSH_CMD_FLAG_ALIAS)
> +                    continue;
> +                vshPrint(ctl, "    %-30s %s\n", def->name,
> +                         _(vshCmddefGetInfo(def, "help")));
> +            }
> +
> +            vshPrint(ctl, "\n");
> +        }
> +
> +        return true;
> +    }
> +
> +    if (vshCmddefSearch(name)) {
> +        return vshCmddefHelp(ctl, name);
> +    } else if (vshCmdGrpSearch(name)) {
> +        return vshCmdGrpHelp(ctl, name);
> +    } else {
> +        vshError(ctl, _("command or command group '%s' doesn't exist"), name);
> +        return false;
> +    }
> +}
> +
> +bool
> +cmdCd(vshControl *ctl, const vshCmd *cmd)
> +{
> +    const char *dir = NULL;
> +    char *dir_malloced = NULL;
> +    bool ret = true;
> +    char ebuf[1024];
> +
> +    if (!ctl->imode) {
> +        vshError(ctl, "%s", _("cd: command valid only in interactive mode"));
> +        return false;
> +    }
> +
> +    if (vshCommandOptString(ctl, cmd, "dir", &dir) <= 0)
> +        dir = dir_malloced = virGetUserDirectory();
> +    if (!dir)
> +        dir = "/";
> +
> +    if (chdir(dir) == -1) {
> +        vshError(ctl, _("cd: %s: %s"),
> +                 virStrerror(errno, ebuf, sizeof(ebuf)), dir);
> +        ret = false;
> +    }
> +
> +    VIR_FREE(dir_malloced);
> +    return ret;
> +}
> +
> +/* Exists mainly for debugging virsh, but also handy for adding back
> + * quotes for later evaluation.
> + */
> +bool
> +cmdEcho(vshControl *ctl, const vshCmd *cmd)
> +{
> +    bool shell = false;
> +    bool xml = false;
> +    int count = 0;
> +    const vshCmdOpt *opt = NULL;
> +    char *arg;
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> +    if (vshCommandOptBool(cmd, "shell"))
> +        shell = true;
> +    if (vshCommandOptBool(cmd, "xml"))
> +        xml = true;
> +
> +    while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
> +        char *str;
> +        virBuffer xmlbuf = VIR_BUFFER_INITIALIZER;
> +
> +        arg = opt->data;
> +
> +        if (count)
> +            virBufferAddChar(&buf, ' ');
> +
> +        if (xml) {
> +            virBufferEscapeString(&xmlbuf, "%s", arg);
> +            if (virBufferError(&xmlbuf)) {
> +                vshPrint(ctl, "%s", _("Failed to allocate XML buffer"));
> +                return false;
> +            }
> +            str = virBufferContentAndReset(&xmlbuf);
> +        } else {
> +            str = vshStrdup(ctl, arg);
> +        }
> +
> +        if (shell)
> +            virBufferEscapeShell(&buf, str);
> +        else
> +            virBufferAdd(&buf, str, -1);
> +        count++;
> +        VIR_FREE(str);
> +    }
> +
> +    if (virBufferError(&buf)) {
> +        vshPrint(ctl, "%s", _("Failed to allocate XML buffer"));
> +        return false;
> +    }
> +    arg = virBufferContentAndReset(&buf);
> +    if (arg)
> +        vshPrint(ctl, "%s", arg);
> +    VIR_FREE(arg);
> +    return true;
> +}
> +
> +bool
> +cmdPwd(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
> +{
> +    char *cwd;
> +    bool ret = true;
> +    char ebuf[1024];
> +
> +    cwd = getcwd(NULL, 0);
> +    if (!cwd) {
> +        vshError(ctl, _("pwd: cannot get current directory: %s"),
> +                 virStrerror(errno, ebuf, sizeof(ebuf)));
> +        ret = false;
> +    } else {
> +        vshPrint(ctl, _("%s\n"), cwd);
> +        VIR_FREE(cwd);
> +    }
> +
> +    return ret;
> +}
> +
> +bool
> +cmdQuit(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
> +{
> +    ctl->imode = false;
> +    return true;
> +}
> diff --git a/tools/vsh.h b/tools/vsh.h
> index 19862d0..cbc3455 100644
> --- a/tools/vsh.h
> +++ b/tools/vsh.h
> @@ -363,6 +363,13 @@ int vshEventStart(vshControl *ctl, int timeout_ms);
>  void vshEventTimeout(int timer, void *opaque);
>  int vshEventWait(vshControl *ctl);
>  
> +/* generic commands */
> +bool cmdHelp(vshControl *ctl, const vshCmd *cmd);
> +bool cmdCd(vshControl *ctl, const vshCmd *cmd);
> +bool cmdEcho(vshControl *ctl, const vshCmd *cmd);
> +bool cmdPwd(vshControl *ctl, const vshCmd *cmd);
> +bool cmdQuit(vshControl *ctl, const vshCmd *cmd);
> +
>  /* readline */
>  char * vshReadline(vshControl *ctl, const char *prompt);
>  
> 

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]