[libvirt] [PATCH] virsh: Honour --readonly with cmdConnect and no name

Martin Kletzander posted 1 patch 6 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/7de9870f9f9cfefd5cf3234ca439687f09ded290.1503067204.git.mkletzan@redhat.com
tools/virsh.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[libvirt] [PATCH] virsh: Honour --readonly with cmdConnect and no name
Posted by Martin Kletzander 6 years, 8 months ago
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 tools/virsh.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 94bb7ff6c93a..9883e87df2b5 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -218,7 +218,13 @@ virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force)
 {
     bool connected = false;
     virshControlPtr priv = ctl->privData;
-    bool ro = name ? readonly : priv->readonly;
+
+    /* If the flag was not specified, then it depends on whether we are
+     * reconnecting to the default URI (in which case we want to keep the
+     * readonly flag as it was) or to a specified URI in which case it
+     * should stay false */
+    if (!readonly && !name)
+        readonly = priv->readonly;
 
     if (priv->conn) {
         int ret;
@@ -233,7 +239,7 @@ virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force)
                                   "disconnect from the hypervisor"));
     }
 
-    priv->conn = virshConnect(ctl, name ? name : ctl->connname, ro);
+    priv->conn = virshConnect(ctl, name ? name : ctl->connname, readonly);
 
     if (!priv->conn) {
         if (disconnected)
-- 
2.14.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Honour --readonly with cmdConnect and no name
Posted by Martin Kletzander 6 years, 8 months ago
On Fri, Aug 18, 2017 at 04:40:04PM +0200, Martin Kletzander wrote:
>Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>---
> tools/virsh.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1436042

(already added into the commit locally)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Honour --readonly with cmdConnect and no name
Posted by Erik Skultety 6 years, 8 months ago
On Fri, Aug 18, 2017 at 04:40:04PM +0200, Martin Kletzander wrote:
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  tools/virsh.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 94bb7ff6c93a..9883e87df2b5 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -218,7 +218,13 @@ virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force)
>  {
>      bool connected = false;
>      virshControlPtr priv = ctl->privData;
> -    bool ro = name ? readonly : priv->readonly;
> +
> +    /* If the flag was not specified, then it depends on whether we are
> +     * reconnecting to the default URI (in which case we want to keep the

Probably I'm just thinking about it too much, but "default URI" means the URI
to use when none specified - you can define it by the following: uri_default in
libvirt.conf, LIBVIRT_DEFAULT_URI env or let libvirt use the hardcoded default
- the thing is that I can have the default defined as "qemu:///system" but use
lxc for the moment and this would still work nicely, so maybe "last known
URI"?? But as I said, maybe I'm just spending ridiculously too much time
thinking about it.

ACK

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Honour --readonly with cmdConnect and no name
Posted by Martin Kletzander 6 years, 8 months ago
On Mon, Aug 21, 2017 at 11:29:15AM +0200, Erik Skultety wrote:
>On Fri, Aug 18, 2017 at 04:40:04PM +0200, Martin Kletzander wrote:
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  tools/virsh.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index 94bb7ff6c93a..9883e87df2b5 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -218,7 +218,13 @@ virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force)
>>  {
>>      bool connected = false;
>>      virshControlPtr priv = ctl->privData;
>> -    bool ro = name ? readonly : priv->readonly;
>> +
>> +    /* If the flag was not specified, then it depends on whether we are
>> +     * reconnecting to the default URI (in which case we want to keep the
>
>Probably I'm just thinking about it too much, but "default URI" means the URI
>to use when none specified - you can define it by the following: uri_default in
>libvirt.conf, LIBVIRT_DEFAULT_URI env or let libvirt use the hardcoded default
>- the thing is that I can have the default defined as "qemu:///system" but use
>lxc for the moment and this would still work nicely, so maybe "last known
>URI"?? But as I said, maybe I'm just spending ridiculously too much time
>thinking about it.

No, you're right, it's reconnecting to the current URI, just with a new
flag, so it should say "current".  Will fix before pushing.

>
>ACK
>
>Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list