[libvirt] [PATCH v3 0/5] Making libvirt aware of a server need
Hi guys, this is the third version of the patchset to add a new member to virConnectDriver in order to simplfy the check for a server. Now, libvirt does the work inside virConnectOpenInternal after getting the correct driver from the URI. v2: https://www.redhat.com/archives/libvir-list/2018-July/msg00501.html v1: https://www.redhat.com/archives/libvir-list/2018-July/msg00393.html Changes from v2: * patch 2: Move check for remoteOnly member just before calling connectOpen * (only patch 2 changed in v2) Changes from v1: * Adapted the code following ideas from Matthias Bolte Marcos Paulo de Souza (5): driver.h: Add remoteOnly member to virConnectDriver struct libvirt.c: Return error when remoteOnly is set but server is empty esx_driver: Set remoteOnly member of virConnectDriver hyperv_driver: Set remoteOnly member of virConnectDriver phyp_driver: Set remoteOnly member of virConnectDriver src/driver.h | 2 ++ src/esx/esx_driver.c | 8 +------- src/hyperv/hyperv_driver.c | 8 +------- src/libvirt.c | 7 +++++++ src/phyp/phyp_driver.c | 7 +------ 5 files changed, 12 insertions(+), 20 deletions(-) -- 2.17.1
This new flag will be set when a driver needs a remote URL in order to work, as ESX, HyperV and Phyp. Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> --- src/driver.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/driver.h b/src/driver.h index 0b1f7a2269..c4d94ba294 100644 --- a/src/driver.h +++ b/src/driver.h @@ -81,6 +81,8 @@ typedef virConnectDriver *virConnectDriverPtr; struct _virConnectDriver { /* Wether driver permits a server in the URI */ bool localOnly; + /* Wether driver needs a server in the URI */ + bool remoteOnly; /* * NULL terminated list of supported URI schemes. * - Single element { NULL } list indicates no supported schemes -- 2.17.1
Some drivers require a server in order to work, so this flag removes the burden of esach driver to check for an server by doing it in virConnectOpenInternal. Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> --- src/libvirt.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libvirt.c b/src/libvirt.c index 52f4dd2808..aeb8804714 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1029,6 +1029,13 @@ virConnectOpenInternal(const char *name, VIR_DEBUG("Matching any URI scheme for '%s'", ret->uri ? ret->uri->scheme : ""); } + /* before starting the new connection, check if the driver only works + * with a server, and so return an error if the server is missing */ + if (virConnectDriverTab[i]->remoteOnly && ret->uri && !ret->uri->server) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("URI is missing the server part")); + goto failed; + } + ret->driver = virConnectDriverTab[i]->hypervisorDriver; ret->interfaceDriver = virConnectDriverTab[i]->interfaceDriver; ret->networkDriver = virConnectDriverTab[i]->networkDriver; -- 2.17.1
ESX driver can't function without a server being informed, so this flag makes libvirt to check for a valid server before calling connectOpen. Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> --- src/esx/esx_driver.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 06e1238385..d937daac83 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -854,13 +854,6 @@ 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; - } - /* Require auth */ if (!auth || !auth->cb) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -5218,6 +5211,7 @@ static virConnectDriver esxConnectDriver = { .interfaceDriver = &esxInterfaceDriver, .networkDriver = &esxNetworkDriver, .storageDriver = &esxStorageDriver, + .remoteOnly = true, }; int -- 2.17.1
On 07/11/2018 01:31 AM, Marcos Paulo de Souza wrote:
ESX driver can't function without a server being informed, so this flag makes libvirt to check for a valid server before calling connectOpen.
Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> --- src/esx/esx_driver.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 06e1238385..d937daac83 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -854,13 +854,6 @@ 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; - } - /* Require auth */ if (!auth || !auth->cb) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -5218,6 +5211,7 @@ static virConnectDriver esxConnectDriver = { .interfaceDriver = &esxInterfaceDriver, .networkDriver = &esxNetworkDriver, .storageDriver = &esxStorageDriver, + .remoteOnly = true,
Nit pick, I think this remoteOnly should be placed a few lines upfront - to honour ordering laid down by _virConnectDriver declaration. The second reason is to group driver attributes setting (uri schemas, local/remote only) and drivers callback. Michal
HyperV driver can't function without a server being informed, so this flag makes libvirt to check for a valid server before calling connectOpen. Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> --- src/hyperv/hyperv_driver.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 95496bdf73..8bda334cf9 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -128,13 +128,6 @@ 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; - } - /* Require auth */ if (auth == NULL || auth->cb == NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -1664,6 +1657,7 @@ hypervDebugHandler(const char *message, debug_level_e level, static virConnectDriver hypervConnectDriver = { .uriSchemes = (const char *[]){ "hyperv", NULL }, .hypervisorDriver = &hypervHypervisorDriver, + .remoteOnly = true, }; int -- 2.17.1
Phyp driver can't function without a server being informed, so this flag makes libvirt to check for a valid server before calling connectOpen. Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com> --- src/phyp/phyp_driver.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 67ce7903ba..303a7151b7 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1141,12 +1141,6 @@ 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; - } - if (VIR_ALLOC(phyp_driver) < 0) goto failure; @@ -3764,6 +3758,7 @@ static virConnectDriver phypConnectDriver = { .hypervisorDriver = &phypHypervisorDriver, .interfaceDriver = &phypInterfaceDriver, .storageDriver = &phypStorageDriver, + .remoteOnly = true, }; int -- 2.17.1
On 07/11/2018 01:30 AM, Marcos Paulo de Souza wrote:
Hi guys,
this is the third version of the patchset to add a new member to virConnectDriver in order to simplfy the check for a server. Now, libvirt does the work inside virConnectOpenInternal after getting the correct driver from the URI.
v2: https://www.redhat.com/archives/libvir-list/2018-July/msg00501.html v1: https://www.redhat.com/archives/libvir-list/2018-July/msg00393.html
Changes from v2: * patch 2: Move check for remoteOnly member just before calling connectOpen * (only patch 2 changed in v2)
Changes from v1: * Adapted the code following ideas from Matthias Bolte
Marcos Paulo de Souza (5): driver.h: Add remoteOnly member to virConnectDriver struct libvirt.c: Return error when remoteOnly is set but server is empty esx_driver: Set remoteOnly member of virConnectDriver hyperv_driver: Set remoteOnly member of virConnectDriver phyp_driver: Set remoteOnly member of virConnectDriver
src/driver.h | 2 ++ src/esx/esx_driver.c | 8 +------- src/hyperv/hyperv_driver.c | 8 +------- src/libvirt.c | 7 +++++++ src/phyp/phyp_driver.c | 7 +------ 5 files changed, 12 insertions(+), 20 deletions(-)
ACKed and pushed. Michal
participants (2)
-
Marcos Paulo de Souza -
Michal Privoznik