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