[libvirt] [PATCH] remote: remove the only use of LIBVIRTD_CONFIGURATION_FILE

Fabiano Fidêncio posted 1 patch 4 years, 9 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190704061621.22790-1-fidencio@redhat.com
src/remote/remote_daemon.c | 4 ----
1 file changed, 4 deletions(-)
[libvirt] [PATCH] remote: remove the only use of LIBVIRTD_CONFIGURATION_FILE
Posted by Fabiano Fidêncio 4 years, 9 months ago
86fbce56f27e removed the constant, but didn't actually adjust the only
place where the constant was used.

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
---
This patch fixes a build breakage.

It's either this, or reverting 86fbce56f27e.
---
 src/remote/remote_daemon.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index fdc9e4333a..3cd7b16163 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -899,9 +899,6 @@ daemonUsage(const char *argv0, bool privileged)
                 _("\n"
                   "  Default paths:\n"
                   "\n"
-                  "    Configuration file (unless overridden by -f):\n"
-                  "      %s\n"
-                  "\n"
                   "    Sockets:\n"
                   "      %s\n"
                   "      %s\n"
@@ -914,7 +911,6 @@ daemonUsage(const char *argv0, bool privileged)
                   "    PID file (unless overridden by -p):\n"
                   "      %s/run/libvirtd.pid\n"
                   "\n"),
-                LIBVIRTD_CONFIGURATION_FILE,
                 LIBVIRTD_PRIV_UNIX_SOCKET,
                 LIBVIRTD_PRIV_UNIX_SOCKET_RO,
                 LIBVIRT_CACERT,
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: remove the only use of LIBVIRTD_CONFIGURATION_FILE
Posted by Bjoern Walk 4 years, 9 months ago
Fabiano Fidêncio <fidencio@redhat.com> [2019-07-04, 08:16AM +0200]:
> 86fbce56f27e removed the constant, but didn't actually adjust the only
> place where the constant was used.
> 
> Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
> ---
> This patch fixes a build breakage.
> 
> It's either this, or reverting 86fbce56f27e.
> ---
>  src/remote/remote_daemon.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> index fdc9e4333a..3cd7b16163 100644
> --- a/src/remote/remote_daemon.c
> +++ b/src/remote/remote_daemon.c
> @@ -899,9 +899,6 @@ daemonUsage(const char *argv0, bool privileged)
>                  _("\n"
>                    "  Default paths:\n"
>                    "\n"
> -                  "    Configuration file (unless overridden by -f):\n"
> -                  "      %s\n"
> -                  "\n"
>                    "    Sockets:\n"
>                    "      %s\n"
>                    "      %s\n"
> @@ -914,7 +911,6 @@ daemonUsage(const char *argv0, bool privileged)
>                    "    PID file (unless overridden by -p):\n"
>                    "      %s/run/libvirtd.pid\n"
>                    "\n"),
> -                LIBVIRTD_CONFIGURATION_FILE,
>                  LIBVIRTD_PRIV_UNIX_SOCKET,
>                  LIBVIRTD_PRIV_UNIX_SOCKET_RO,
>                  LIBVIRT_CACERT,

So now with this and the original patch we just removed a piece of
useful information? Doesn't make sense to me.

-- 
IBM Systems
Linux on Z & Virtualization Development
--------------------------------------------------
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--------------------------------------------------
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: remove the only use of LIBVIRTD_CONFIGURATION_FILE
Posted by Boris Fiuczynski 4 years, 9 months ago
This patch is sort of incomplete since the unprivileged usage path still 
shows the default path for the configuration file that this patch would 
remove for the privileged path.
I think the simplest solution is to revert 86fbce56f27e.

On 7/4/19 8:16 AM, Fabiano Fidêncio wrote:
> 86fbce56f27e removed the constant, but didn't actually adjust the only
> place where the constant was used.
> 
> Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
> ---
> This patch fixes a build breakage.
> 
> It's either this, or reverting 86fbce56f27e.
> ---
>   src/remote/remote_daemon.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> index fdc9e4333a..3cd7b16163 100644
> --- a/src/remote/remote_daemon.c
> +++ b/src/remote/remote_daemon.c
> @@ -899,9 +899,6 @@ daemonUsage(const char *argv0, bool privileged)
>                   _("\n"
>                     "  Default paths:\n"
>                     "\n"
> -                  "    Configuration file (unless overridden by -f):\n"
> -                  "      %s\n"
> -                  "\n"
>                     "    Sockets:\n"
>                     "      %s\n"
>                     "      %s\n"
> @@ -914,7 +911,6 @@ daemonUsage(const char *argv0, bool privileged)
>                     "    PID file (unless overridden by -p):\n"
>                     "      %s/run/libvirtd.pid\n"
>                     "\n"),
> -                LIBVIRTD_CONFIGURATION_FILE,
>                   LIBVIRTD_PRIV_UNIX_SOCKET,
>                   LIBVIRTD_PRIV_UNIX_SOCKET_RO,
>                   LIBVIRT_CACERT,
> 


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: remove the only use of LIBVIRTD_CONFIGURATION_FILE
Posted by Peter Krempa 4 years, 9 months ago
On Thu, Jul 04, 2019 at 08:16:21 +0200, Fabiano Fidêncio wrote:
> 86fbce56f27e removed the constant, but didn't actually adjust the only
> place where the constant was used.
> 
> Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
> ---
> This patch fixes a build breakage.
> 
> It's either this, or reverting 86fbce56f27e.

I've reverted it now sice the premise that the constant was unused is
wrong. A separate patch can e.g. squash this change and the removal of
the constant with a better justification perhaps. Obviously that will
need to go through review again to consider the merit of the
justification.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: remove the only use of LIBVIRTD_CONFIGURATION_FILE
Posted by Daniel P. Berrangé 4 years, 9 months ago
On Thu, Jul 04, 2019 at 09:28:01AM +0200, Peter Krempa wrote:
> On Thu, Jul 04, 2019 at 08:16:21 +0200, Fabiano Fidêncio wrote:
> > 86fbce56f27e removed the constant, but didn't actually adjust the only
> > place where the constant was used.
> > 
> > Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
> > ---
> > This patch fixes a build breakage.
> > 
> > It's either this, or reverting 86fbce56f27e.
> 
> I've reverted it now sice the premise that the constant was unused is
> wrong. A separate patch can e.g. squash this change and the removal of
> the constant with a better justification perhaps. Obviously that will
> need to go through review again to consider the merit of the
> justification.

Yes, I screwed up. I removed use of this constant in my pending branch
but forgot this. I notice the constant declaration was unused. It genuinely
was unused when first introduced, but later it was made use of. Unfortunately
when searching the history I jumped straight over the place which introduced
its use, making it appear to me as if it was never used :-(

Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] remote: remove the only use of LIBVIRTD_CONFIGURATION_FILE
Posted by Fabiano Fidêncio 4 years, 9 months ago
On Thu, Jul 4, 2019 at 9:28 AM Peter Krempa <pkrempa@redhat.com> wrote:
>
> On Thu, Jul 04, 2019 at 08:16:21 +0200, Fabiano Fidêncio wrote:
> > 86fbce56f27e removed the constant, but didn't actually adjust the only
> > place where the constant was used.
> >
> > Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
> > ---
> > This patch fixes a build breakage.
> >
> > It's either this, or reverting 86fbce56f27e.
>
> I've reverted it now sice the premise that the constant was unused is
> wrong. A separate patch can e.g. squash this change and the removal of
> the constant with a better justification perhaps. Obviously that will
> need to go through review again to consider the merit of the
> justification.

Fair enough! Thanks for taking care of this!

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