[libvirt] [PATCH 0/5] Making libvirt aware of a server need
Hi guys, this is basically a second version of [1], but I rewrote the patch following Matthias ideas [2]. It really seems a cleaner change, but, feel free to suggest any other change that could make the code better. I did a lot of tests using virsh tool, so when using the ESX driver without a proper server, it returns a error. I could not test hyperv and phyp. Thanks, [1]: https://www.redhat.com/archives/libvir-list/2018-July/msg00393.html [2]: https://www.redhat.com/archives/libvir-list/2018-July/msg00400.html 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 | 5 +++++ src/phyp/phyp_driver.c | 7 +------ 5 files changed, 10 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 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libvirt.c b/src/libvirt.c index 52f4dd2808..9ae4e774eb 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1005,6 +1005,11 @@ virConnectOpenInternal(const char *name, continue; } + if (virConnectDriverTab[i]->remoteOnly && ret->uri && !ret->uri->server) { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("URI is missing the server part")); + goto failed; + } + /* Filter drivers based on declared URI schemes */ if (virConnectDriverTab[i]->uriSchemes) { bool matchScheme = false; -- 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
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 Mon, Jul 09, 2018 at 01:22:18PM -0300, Marcos Paulo de Souza wrote:
Hi guys,
this is basically a second version of [1], but I rewrote the patch following Matthias ideas [2]. It really seems a cleaner change, but, feel free to suggest any other change that could make the code better.
I did a lot of tests using virsh tool, so when using the ESX driver without a proper server, it returns a error. I could not test hyperv and phyp.
Thanks,
[1]: https://www.redhat.com/archives/libvir-list/2018-July/msg00393.html [2]: https://www.redhat.com/archives/libvir-list/2018-July/msg00400.html
This patchset is still not ready, I found a problem in my approach in libvirt.c. Will resent a v3 once this is fixed. Thanks,
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 | 5 +++++ src/phyp/phyp_driver.c | 7 +------ 5 files changed, 10 insertions(+), 20 deletions(-)
-- 2.17.1
-- Thanks, Marcos
participants (1)
-
Marcos Paulo de Souza