This will improve an error message later on.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
src/remote/remote_daemon_dispatch.c | 2 +-
src/remote/remote_sockets.c | 11 +++++++++--
src/remote/remote_sockets.h | 2 +-
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 7daf503b517a..497469e80657 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -2015,7 +2015,7 @@ remoteDispatchProbeURI(bool readonly,
suffix = "session";
} else {
- if (remoteProbeSystemDriverFromSocket(readonly, &driver) < 0)
+ if (remoteProbeSystemDriverFromSocket(readonly, &driver, NULL) < 0)
return -1;
suffix = "system";
diff --git a/src/remote/remote_sockets.c b/src/remote/remote_sockets.c
index 4ab3d72933e2..c21970cd31e7 100644
--- a/src/remote/remote_sockets.c
+++ b/src/remote/remote_sockets.c
@@ -204,7 +204,7 @@ remoteProbeSessionDriverFromBinary(char **driver)
int
-remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
+remoteProbeSystemDriverFromSocket(bool readonly, char **driver, char **first_socket)
{
/* Order these the same as virDriverLoadModule
* calls in daemonInitialize, so we replicate
@@ -232,6 +232,9 @@ remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
};
ssize_t i;
+ if (first_socket)
+ *first_socket = NULL;
+
for (i = 0; i < (ssize_t) G_N_ELEMENTS(drivers); i++) {
g_autofree char *sockname =
g_strdup_printf("%s/libvirt/virt%sd-%s", RUNSTATEDIR,
@@ -243,6 +246,9 @@ remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
return 0;
}
+ if (first_socket && !*first_socket)
+ *first_socket = g_steal_pointer(&sockname);
+
VIR_DEBUG("Missing sock %s for driver %s", sockname, drivers[i]);
}
@@ -338,7 +344,8 @@ remoteGetUNIXSocket(remoteDriverTransport transport,
return NULL;
} else {
if (remoteProbeSystemDriverFromSocket(flags & REMOTE_DRIVER_OPEN_RO,
- &guessdriver) < 0)
+ &guessdriver,
+ NULL) < 0)
return NULL;
}
driver = guessdriver;
diff --git a/src/remote/remote_sockets.h b/src/remote/remote_sockets.h
index 00e654d46c2a..6505639c92a3 100644
--- a/src/remote/remote_sockets.h
+++ b/src/remote/remote_sockets.h
@@ -65,7 +65,7 @@ remoteSplitURIScheme(virURI *uri,
int
remoteProbeSessionDriverFromBinary(char **driver);
int
-remoteProbeSystemDriverFromSocket(bool readonly, char **driver);
+remoteProbeSystemDriverFromSocket(bool readonly, char **driver, char **first_socket);
int
remoteProbeSessionDriverFromSocket(bool readonly, char **driver);
--
2.43.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Tue, Nov 28, 2023 at 01:02:39PM +0100, Martin Kletzander wrote:
> This will improve an error message later on.
>
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
> src/remote/remote_daemon_dispatch.c | 2 +-
> src/remote/remote_sockets.c | 11 +++++++++--
> src/remote/remote_sockets.h | 2 +-
> 3 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
> index 7daf503b517a..497469e80657 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -2015,7 +2015,7 @@ remoteDispatchProbeURI(bool readonly,
>
> suffix = "session";
> } else {
> - if (remoteProbeSystemDriverFromSocket(readonly, &driver) < 0)
> + if (remoteProbeSystemDriverFromSocket(readonly, &driver, NULL) < 0)
> return -1;
>
> suffix = "system";
> diff --git a/src/remote/remote_sockets.c b/src/remote/remote_sockets.c
> index 4ab3d72933e2..c21970cd31e7 100644
> --- a/src/remote/remote_sockets.c
> +++ b/src/remote/remote_sockets.c
> @@ -204,7 +204,7 @@ remoteProbeSessionDriverFromBinary(char **driver)
>
>
> int
> -remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
> +remoteProbeSystemDriverFromSocket(bool readonly, char **driver, char **first_socket)
> {
> /* Order these the same as virDriverLoadModule
> * calls in daemonInitialize, so we replicate
> @@ -232,6 +232,9 @@ remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
> };
> ssize_t i;
>
> + if (first_socket)
> + *first_socket = NULL;
> +
> for (i = 0; i < (ssize_t) G_N_ELEMENTS(drivers); i++) {
> g_autofree char *sockname =
> g_strdup_printf("%s/libvirt/virt%sd-%s", RUNSTATEDIR,
> @@ -243,6 +246,9 @@ remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
> return 0;
> }
>
> + if (first_socket && !*first_socket)
> + *first_socket = g_steal_pointer(&sockname);
> +
> VIR_DEBUG("Missing sock %s for driver %s", sockname, drivers[i]);
> }
>
> @@ -338,7 +344,8 @@ remoteGetUNIXSocket(remoteDriverTransport transport,
> return NULL;
> } else {
> if (remoteProbeSystemDriverFromSocket(flags & REMOTE_DRIVER_OPEN_RO,
> - &guessdriver) < 0)
> + &guessdriver,
> + NULL) < 0)
> return NULL;
> }
> driver = guessdriver;
> diff --git a/src/remote/remote_sockets.h b/src/remote/remote_sockets.h
> index 00e654d46c2a..6505639c92a3 100644
> --- a/src/remote/remote_sockets.h
> +++ b/src/remote/remote_sockets.h
> @@ -65,7 +65,7 @@ remoteSplitURIScheme(virURI *uri,
> int
> remoteProbeSessionDriverFromBinary(char **driver);
> int
> -remoteProbeSystemDriverFromSocket(bool readonly, char **driver);
> +remoteProbeSystemDriverFromSocket(bool readonly, char **driver, char **first_socket);
> int
> remoteProbeSessionDriverFromSocket(bool readonly, char **driver);
>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Tue, Nov 28, 2023 at 13:02:39 +0100, Martin Kletzander wrote:
> This will improve an error message later on.
>
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
> src/remote/remote_daemon_dispatch.c | 2 +-
> src/remote/remote_sockets.c | 11 +++++++++--
> src/remote/remote_sockets.h | 2 +-
> 3 files changed, 11 insertions(+), 4 deletions(-)
[...]
> diff --git a/src/remote/remote_sockets.c b/src/remote/remote_sockets.c
> index 4ab3d72933e2..c21970cd31e7 100644
> --- a/src/remote/remote_sockets.c
> +++ b/src/remote/remote_sockets.c
[...]
> @@ -243,6 +246,9 @@ remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
> return 0;
> }
>
> + if (first_socket && !*first_socket)
> + *first_socket = g_steal_pointer(&sockname);
You steal 'sockname' here ...
> +
> VIR_DEBUG("Missing sock %s for driver %s", sockname, drivers[i]);
... and here it's still used.
For making the code future proof, it'll be most likely better to simply
copy the string instead of rearranging it to be able to steal it.
> }
>
With the bug above addressed:
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Tue, Nov 28, 2023 at 14:04:12 +0100, Peter Krempa wrote:
> On Tue, Nov 28, 2023 at 13:02:39 +0100, Martin Kletzander wrote:
> > This will improve an error message later on.
> >
> > Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> > ---
> > src/remote/remote_daemon_dispatch.c | 2 +-
> > src/remote/remote_sockets.c | 11 +++++++++--
> > src/remote/remote_sockets.h | 2 +-
> > 3 files changed, 11 insertions(+), 4 deletions(-)
>
> [...]
>
> > diff --git a/src/remote/remote_sockets.c b/src/remote/remote_sockets.c
> > index 4ab3d72933e2..c21970cd31e7 100644
> > --- a/src/remote/remote_sockets.c
> > +++ b/src/remote/remote_sockets.c
>
> [...]
>
> > @@ -243,6 +246,9 @@ remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
> > return 0;
> > }
> >
> > + if (first_socket && !*first_socket)
> > + *first_socket = g_steal_pointer(&sockname);
Actually, the commit message seems to imply that you want the first
probed socket to be here, but if the socket file exists, this code won't
be reached. This is fine for the usage though I suppose.
I'll also have to see how this is used, because based on the logic, the
first_socket, value will be set to whatever was frist compiled from the
following list:
const char *drivers[] = {
#ifdef WITH_LIBXL
"xen",
#endif
#ifdef WITH_QEMU
"qemu",
#endif
#ifdef WITH_LXC
"lxc",
#endif
#ifdef WITH_VBOX
"vbox",
#endif
#ifdef WITH_BHYVE
"bhyve",
#endif
#ifdef WITH_VZ
"vz",
#endif
};
Thus reallistically after this patch it will return the first missing
socket from the above list whichever got compiled in given version.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Tue, Nov 28, 2023 at 02:15:59PM +0100, Peter Krempa wrote:
>On Tue, Nov 28, 2023 at 14:04:12 +0100, Peter Krempa wrote:
>> On Tue, Nov 28, 2023 at 13:02:39 +0100, Martin Kletzander wrote:
>> > This will improve an error message later on.
>> >
>> > Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> > ---
>> > src/remote/remote_daemon_dispatch.c | 2 +-
>> > src/remote/remote_sockets.c | 11 +++++++++--
>> > src/remote/remote_sockets.h | 2 +-
>> > 3 files changed, 11 insertions(+), 4 deletions(-)
>>
>> [...]
>>
>> > diff --git a/src/remote/remote_sockets.c b/src/remote/remote_sockets.c
>> > index 4ab3d72933e2..c21970cd31e7 100644
>> > --- a/src/remote/remote_sockets.c
>> > +++ b/src/remote/remote_sockets.c
>>
>> [...]
>>
>> > @@ -243,6 +246,9 @@ remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
>> > return 0;
>> > }
>> >
>> > + if (first_socket && !*first_socket)
>> > + *first_socket = g_steal_pointer(&sockname);
>
>Actually, the commit message seems to imply that you want the first
>probed socket to be here, but if the socket file exists, this code won't
>be reached. This is fine for the usage though I suppose.
>
Yes, that's the point, enhance the error message. If any socket is
found, there is no error message from this particular function.
>I'll also have to see how this is used, because based on the logic, the
>first_socket, value will be set to whatever was frist compiled from the
>following list:
>
>const char *drivers[] = {
>#ifdef WITH_LIBXL
> "xen",
>#endif
>#ifdef WITH_QEMU
> "qemu",
>#endif
>#ifdef WITH_LXC
> "lxc",
>#endif
>#ifdef WITH_VBOX
> "vbox",
>#endif
>#ifdef WITH_BHYVE
> "bhyve",
>#endif
>#ifdef WITH_VZ
> "vz",
>#endif
> };
>
>
>Thus reallistically after this patch it will return the first missing
>socket from the above list whichever got compiled in given version.
>
Yes, exactly my point. I had various patches suggesting all the sockets
etc., but when someone is using neither uri_default nor
VIRSH_DEFAULT_CONNECT_URI nor LIBVIRT_DEFAULT_URI nor -c (some of them
of course only in the virsh case) then constructing a paragraph of
suggestions in a convoluted way seems more gross than this patch, which
strictly enhances the error message users see.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
On Tue, Nov 28, 2023 at 02:04:12PM +0100, Peter Krempa wrote:
>On Tue, Nov 28, 2023 at 13:02:39 +0100, Martin Kletzander wrote:
>> This will improve an error message later on.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>> src/remote/remote_daemon_dispatch.c | 2 +-
>> src/remote/remote_sockets.c | 11 +++++++++--
>> src/remote/remote_sockets.h | 2 +-
>> 3 files changed, 11 insertions(+), 4 deletions(-)
>
>[...]
>
>> diff --git a/src/remote/remote_sockets.c b/src/remote/remote_sockets.c
>> index 4ab3d72933e2..c21970cd31e7 100644
>> --- a/src/remote/remote_sockets.c
>> +++ b/src/remote/remote_sockets.c
>
>[...]
>
>> @@ -243,6 +246,9 @@ remoteProbeSystemDriverFromSocket(bool readonly, char **driver)
>> return 0;
>> }
>>
>> + if (first_socket && !*first_socket)
>> + *first_socket = g_steal_pointer(&sockname);
>
>You steal 'sockname' here ...
>
>> +
>> VIR_DEBUG("Missing sock %s for driver %s", sockname, drivers[i]);
>
>... and here it's still used.
>
>For making the code future proof, it'll be most likely better to simply
>copy the string instead of rearranging it to be able to steal it.
>
Yup, makes sense, thanks.
>> }
>>
>
>With the bug above addressed:
>
>Reviewed-by: Peter Krempa <pkrempa@redhat.com>
>
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
© 2016 - 2026 Red Hat, Inc.