8:06 p.m.
Hello,
I am trying to add a generic way to check for server, specially in drivers that
needs the server to be declared. If there is better way to put the check, or a
better way to check this setting, let me know.
Thanks in advance!
Marcos Paulo de Souza (4):
driver.h: Add macro VIR_DRV_CONN_CHECK_SERVER
esx_driver: Use VIR_DRV_CONN_CHECK_SERVER macro
hyperv_driver: Use VIR_DRV_CONN_CHECK_SERVER macro
phyp_driver: Use VIR_DRV_CONN_CHECK_SERVER macro
src/driver.h | 9 +++++++++
src/esx/esx_driver.c | 7 +------
src/hyperv/hyperv_driver.c | 7 +------
src/phyp/phyp_driver.c | 6 +-----
4 files changed, 12 insertions(+), 17 deletions(-)
--
2.17.1
8:06 p.m.
New subject: [libvirt] [RFC PATCH 1/4] driver.h: Add macro VIR_DRV_CONN_CHECK_SERVER
This new macro will check if the server was passed to connectOpen
function. Some drivers as esx, hyperv and phyp need a server address to
connect to.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org(a)gmail.com>
---
src/driver.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/driver.h b/src/driver.h
index 0b1f7a2269..1f9d7fd26b 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -60,6 +60,15 @@ typedef enum {
((drv)->connectSupportsFeature ? \
(drv)->connectSupportsFeature((conn), (feature)) > 0 : 0)
+/* Check if the connection of the driver has filled the server address */
+# define VIR_DRV_CONN_CHECK_SERVER \
+ do { \
+ if (!conn->uri->server) { \
+ virReportError(VIR_ERR_INVALID_ARG, "%s", \
+ _("URI is missing the server part")); \
+ return VIR_DRV_OPEN_ERROR; \
+ } \
+ } while (0)
# define __VIR_DRIVER_H_INCLUDES___
--
2.17.1
1:31 a.m.
New subject: [libvirt] [RFC PATCH 1/4] driver.h: Add macro VIR_DRV_CONN_CHECK_SERVER
On Sat, Jul 07, 2018 at 22:06:53 -0300, Marcos Paulo de Souza wrote:
This new macro will check if the server was passed to connectOpen
function. Some drivers as esx, hyperv and phyp need a server address to
connect to.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org(a)gmail.com>
---
src/driver.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/driver.h b/src/driver.h
index 0b1f7a2269..1f9d7fd26b 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -60,6 +60,15 @@ typedef enum {
((drv)->connectSupportsFeature ? \
(drv)->connectSupportsFeature((conn), (feature)) > 0 : 0)
+/* Check if the connection of the driver has filled the server address */
+# define VIR_DRV_CONN_CHECK_SERVER \
'conn' should be passed in as an argument. Without that it is less
obvious what is happening here.
P.S: Yes I'm aware that we have e.g. virCheckFlags that checks the
'flags' variable in any context, but we don't have to introduce more of
it.
10:42 a.m.
New subject: [libvirt] [RFC PATCH 1/4] driver.h: Add macro VIR_DRV_CONN_CHECK_SERVER
On Mon, Jul 09, 2018 at 08:31:05AM +0200, Peter Krempa wrote:
On Sat, Jul 07, 2018 at 22:06:53 -0300, Marcos Paulo de Souza wrote:
> This new macro will check if the server was passed to connectOpen
> function. Some drivers as esx, hyperv and phyp need a server address to
> connect to.
>
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org(a)gmail.com>
> ---
> src/driver.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/src/driver.h b/src/driver.h
> index 0b1f7a2269..1f9d7fd26b 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -60,6 +60,15 @@ typedef enum {
> ((drv)->connectSupportsFeature ? \
> (drv)->connectSupportsFeature((conn), (feature)) > 0 : 0)
>
> +/* Check if the connection of the driver has filled the server address */
> +# define VIR_DRV_CONN_CHECK_SERVER \
'conn' should be passed in as an argument. Without that it is less
obvious what is happening here.
P.S: Yes I'm aware that we have e.g. virCheckFlags that checks the
'flags' variable in any context, but we don't have to introduce more of
it.
How about to follow Matthias idea, and add a new member to virConnectDriver?
(https://www.redhat.com/archives/libvir-list/2018-July/msg00400.html)
I am already preparing a new patchset following his idea.
--
Thanks,
Marcos
8:06 p.m.
New subject: [libvirt] [RFC PATCH 2/4] esx_driver: Use VIR_DRV_CONN_CHECK_SERVER macro
ESX needs a server to connect to, so use this macro in order to reuse
code.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org(a)gmail.com>
---
src/esx/esx_driver.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 06e1238385..89ac7c430f 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -854,12 +854,7 @@ esxConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
conn->uri->path, conn->uri->scheme);
}
- /* Require server part */
- if (!conn->uri->server) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("URI is missing the server part"));
- return VIR_DRV_OPEN_ERROR;
- }
+ VIR_DRV_CONN_CHECK_SERVER;
/* Require auth */
if (!auth || !auth->cb) {
--
2.17.1
8:06 p.m.
New subject: [libvirt] [RFC PATCH 3/4] hyperv_driver: Use VIR_DRV_CONN_CHECK_SERVER macro
hyperv needs a server to connect to, so use this macro in order to reuse
code.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org(a)gmail.com>
---
src/hyperv/hyperv_driver.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 95496bdf73..a419f0ccee 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -128,12 +128,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
- /* Require server part */
- if (conn->uri->server == NULL) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("URI is missing the server part"));
- return VIR_DRV_OPEN_ERROR;
- }
+ VIR_DRV_CONN_CHECK_SERVER;
/* Require auth */
if (auth == NULL || auth->cb == NULL) {
--
2.17.1
8:06 p.m.
New subject: [libvirt] [RFC PATCH 4/4] phyp_driver: Use VIR_DRV_CONN_CHECK_SERVER macro
phyp needs a server to connect to, so use this macro in order to reuse
code.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org(a)gmail.com>
---
src/phyp/phyp_driver.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 67ce7903ba..beca1c6fc7 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1141,11 +1141,7 @@ phypConnectOpen(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
- if (conn->uri->server == NULL) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("Missing server name in phyp://
URI"));
- return VIR_DRV_OPEN_ERROR;
- }
+ VIR_DRV_CONN_CHECK_SERVER;
if (VIR_ALLOC(phyp_driver) < 0)
goto failure;
--
2.17.1
3:07 p.m.
2018-07-08 3:06 GMT+02:00 Marcos Paulo de Souza <marcos.souza.org(a)gmail.com>:
Hello,
I am trying to add a generic way to check for server, specially in drivers that
needs the server to be declared. If there is better way to put the check, or a
better way to check this setting, let me know.
Thanks in advance!
I'd realize this in a different way.
The virConnectDriver struct has a localOnly flag that is used to
indicate whether a connection driver permits a server in the URI. Then
virConnectOpenInternal uses this flag during its driver auto-probing
phase.
I suggest expanding this concept and adding a remoteOnly flag to the
virConnectDriver struct. Instead of checking the URI server part in
the driver's own open function let virConnectOpenInternal do this
check based on the remoteOnly flag.
--
Matthias Bolte
http://photron.blogspot.com
2328
days inactive
2329
days old
7 comments
3 participants
participants (3)
-
Marcos Paulo de Souza -
Matthias Bolte -
Peter Krempa