[libvirt] [PATCH] virsh: let 'connect' without args remember -c option

[Osier Yang]

On 2012年06月09日 07:42, Eric Blake wrote:
> If a user invokes 'virsh -c $URI', then within that batch shell,
> they probably want 'connect' to revert to $URI rather than the
> normal default URI you get for passing in NULL.
>
> In particular, I had a setup where qemu:///session was failing,
> but took 20 seconds to fail; since 'make -C tests check TESTS=virsh-all'
> exercises the command 'virsh -c test:///default connect' without
> arguments, it was locking up for 20 seconds trying to connect to
> qemu, even though the testsuite specifically wants to limit itself
> to the test:///default URI.
>
> * tools/virsh.c (__vshControl): Add member.
> (main, vshParseArgv): Set it.
> (vshDeinit): Clean it up.
> (cmdConnect): Use it to reopen to original connection.
> ---
>
> This doesn't fix the root cause of this problem:
> https://www.redhat.com/archives/libvir-list/2012-June/msg00237.html
> but at least it makes my 'make check' runs faster while
> I still investigate the root problem.
>
>   tools/virsh.c |   10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 0453b95..b28dc49 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -241,6 +241,7 @@ typedef struct __vshCmd {
>    */
>   typedef struct __vshControl {
>       char *name;                 /* connection name */
> +    char *default_name;         /* -c or env-var default when name is NULL */
>       virConnectPtr conn;         /* connection to hypervisor (MAY BE NULL) */
>       vshCmd *cmd;                /* the current command */
>       char *cmdstr;               /* string with command */
> @@ -856,7 +857,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
>           vshError(ctl, "%s", _("Please specify valid connection URI"));
>           return false;
>       }
> -    ctl->name = vshStrdup(ctl, name);
> +    ctl->name = vshStrdup(ctl, name ? name : ctl->default_name);
>
>       ctl->useGetInfo = false;
>       ctl->useSnapshotOld = false;
> @@ -19922,6 +19923,7 @@ vshDeinit(vshControl *ctl)
>       vshReadlineDeinit(ctl);
>       vshCloseLogFile(ctl);
>       VIR_FREE(ctl->name);
> +    VIR_FREE(ctl->default_name);
>       if (ctl->conn) {
>           int ret;
>           if ((ret = virConnectClose(ctl->conn)) != 0) {
> @@ -20176,7 +20178,8 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
>               ctl->timing = true;
>               break;
>           case 'c':
> -            ctl->name = vshStrdup(ctl, optarg);
> +            VIR_FREE(ctl->default_name);
> +            ctl->default_name = vshStrdup(ctl, optarg);
>               break;
>           case 'v':
>               if (STRNEQ_NULLABLE(optarg, "long")) {
> @@ -20211,6 +20214,7 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
>               exit(EXIT_FAILURE);
>           }
>       }
> +    ctl->name = vshStrdup(ctl, ctl->default_name);
>
>       if (argc>  optind) {
>           /* parse command */
> @@ -20268,7 +20272,7 @@ main(int argc, char **argv)
>           progname++;
>
>       if ((defaultConn = getenv("VIRSH_DEFAULT_CONNECT_URI"))) {
> -        ctl->name = vshStrdup(ctl, defaultConn);
> +        ctl->default_name = vshStrdup(ctl, defaultConn);
>       }
>
>       if (!vshParseArgv(ctl, argc, argv)) {

Isn't the following patch enough for the fix? The problem is just
caused by ctl->name is freed even if no --name is specified for
command 'connect'. Or I missed something obviously?

diff --git a/tools/virsh.c b/tools/virsh.c
index abcfbff..0c8b44d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -851,12 +851,15 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd)
          ctl->conn = NULL;
      }

-    VIR_FREE(ctl->name);
      if (vshCommandOptString(cmd, "name", &name) < 0) {
          vshError(ctl, "%s", _("Please specify valid connection URI"));
          return false;
      }
-    ctl->name = vshStrdup(ctl, name);
+
+    if (name) {
+        VIR_FREE(ctl->name);
+        ctl->name = vshStrdup(ctl, name);
+    }

      ctl->useGetInfo = false;
      ctl->useSnapshotOld = false;

Regards,
Osier