[libvirt] [PATCH] virsh: don't override connection URI from argv

Currently, if a connection URI was specified on the command line by the '-c' switch, virsh connects to it, but after connecting overrides its value with the one it tries to obtain from the VIRSH_DEFAULT_CONNECT_URI environment variable. This makes virsh connecting to the wrong URI if it disconnects from the hypervisor and then tries to reconnect, and also leaks the original connname. Fix by calling virGetEnvBlockSUID() before virshParseArgv(). --- tools/virsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 0d8ec5c..e14410b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -962,14 +962,14 @@ main(int argc, char **argv) if (!vshInit(ctl, cmdGroups, NULL)) exit(EXIT_FAILURE); + ctl->connname = vshStrdup(ctl, virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")); + if (!virshParseArgv(ctl, argc, argv) || !virshInit(ctl)) { virshDeinit(ctl); exit(EXIT_FAILURE); } - ctl->connname = vshStrdup(ctl, virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")); - if (!ctl->imode) { ret = vshCommandRun(ctl, ctl->cmd); } else { -- 2.7.4

On Thu, Apr 21, 2016 at 09:38:30AM +0300, Roman Bogorodskiy wrote:
Currently, if a connection URI was specified on the command line by the '-c' switch, virsh connects to it, but after connecting overrides its value with the one it tries to obtain from the VIRSH_DEFAULT_CONNECT_URI environment variable.
This makes virsh connecting to the wrong URI if it disconnects from the hypervisor and then tries to reconnect, and also leaks the original connname.
Fix by calling virGetEnvBlockSUID() before virshParseArgv(). --- tools/virsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK. To those wondering why tests didn't catch that, be on the lookout for my tests amendment patch ;)

Martin Kletzander wrote:
On Thu, Apr 21, 2016 at 09:38:30AM +0300, Roman Bogorodskiy wrote:
Currently, if a connection URI was specified on the command line by the '-c' switch, virsh connects to it, but after connecting overrides its value with the one it tries to obtain from the VIRSH_DEFAULT_CONNECT_URI environment variable.
This makes virsh connecting to the wrong URI if it disconnects from the hypervisor and then tries to reconnect, and also leaks the original connname.
Fix by calling virGetEnvBlockSUID() before virshParseArgv(). --- tools/virsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK. To those wondering why tests didn't catch that, be on the lookout for my tests amendment patch ;)
Pushed, thanks! Roman Bogorodskiy

On Thu, Apr 21, 2016 at 09:38:30AM +0300, Roman Bogorodskiy wrote:
Currently, if a connection URI was specified on the command line by the '-c' switch, virsh connects to it, but after connecting overrides its value with the one it tries to obtain from the VIRSH_DEFAULT_CONNECT_URI environment variable.
This makes virsh connecting to the wrong URI if it disconnects from the hypervisor and then tries to reconnect, and also leaks the original connname.
Fix by calling virGetEnvBlockSUID() before virshParseArgv(). --- tools/virsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 0d8ec5c..e14410b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -962,14 +962,14 @@ main(int argc, char **argv) if (!vshInit(ctl, cmdGroups, NULL)) exit(EXIT_FAILURE);
+ ctl->connname = vshStrdup(ctl, virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")); +
This re-breaks what I tried to fix in: commit c0726e0708762e571a7883cb0558cfec32459669 virsh: read default connection uri from env later
if (!virshParseArgv(ctl, argc, argv) || !virshInit(ctl)) { virshDeinit(ctl); exit(EXIT_FAILURE); }
Would adding: if (!ctl->conname) here be enough? Jan
- ctl->connname = vshStrdup(ctl, virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")); - if (!ctl->imode) { ret = vshCommandRun(ctl, ctl->cmd); } else { -- 2.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Ján Tomko wrote:
On Thu, Apr 21, 2016 at 09:38:30AM +0300, Roman Bogorodskiy wrote:
Currently, if a connection URI was specified on the command line by the '-c' switch, virsh connects to it, but after connecting overrides its value with the one it tries to obtain from the VIRSH_DEFAULT_CONNECT_URI environment variable.
This makes virsh connecting to the wrong URI if it disconnects from the hypervisor and then tries to reconnect, and also leaks the original connname.
Fix by calling virGetEnvBlockSUID() before virshParseArgv(). --- tools/virsh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 0d8ec5c..e14410b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -962,14 +962,14 @@ main(int argc, char **argv) if (!vshInit(ctl, cmdGroups, NULL)) exit(EXIT_FAILURE);
+ ctl->connname = vshStrdup(ctl, virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")); +
This re-breaks what I tried to fix in: commit c0726e0708762e571a7883cb0558cfec32459669 virsh: read default connection uri from env later
if (!virshParseArgv(ctl, argc, argv) || !virshInit(ctl)) { virshDeinit(ctl); exit(EXIT_FAILURE); }
Would adding: if (!ctl->conname) here be enough?
Yeah, a fix like this also works for me. Please check '[PATCH] virsh: re-fix help printing without connection'.
Jan
- ctl->connname = vshStrdup(ctl, virGetEnvBlockSUID("VIRSH_DEFAULT_CONNECT_URI")); - if (!ctl->imode) { ret = vshCommandRun(ctl, ctl->cmd); } else { -- 2.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Roman Bogorodskiy
participants (3)
-
Ján Tomko
-
Martin Kletzander
-
Roman Bogorodskiy