1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH 00/10] Enable direct use of secondary drivers

Daniel P. Berrangé posted 10 patches 7 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180126133537.31883-1-berrange@redhat.com
src/interface/interface_backend_netcf.c |  98 ++++++++-
src/interface/interface_backend_udev.c  |  97 ++++++++-
src/network/bridge_driver.c             | 185 +++++++++++++----
src/network/bridge_driver_platform.h    |   3 +
src/node_device/node_device_driver.c    |  73 ++++++-
src/node_device/node_device_driver.h    |   9 +
src/node_device/node_device_hal.c       |  18 ++
src/node_device/node_device_udev.c      |  19 ++
src/nwfilter/nwfilter_driver.c          |  83 ++++++++
src/secret/secret_driver.c              |  95 +++++++++
src/storage/storage_backend.h           |  45 ++--
src/storage/storage_backend_disk.c      |  30 +--
src/storage/storage_backend_fs.c        |  15 +-
src/storage/storage_backend_gluster.c   |   9 +-
src/storage/storage_backend_iscsi.c     |  24 +--
src/storage/storage_backend_logical.c   |  38 ++--
src/storage/storage_backend_mpath.c     |   5 +-
src/storage/storage_backend_rbd.c       |  53 ++---
src/storage/storage_backend_scsi.c      |  46 +++--
src/storage/storage_backend_sheepdog.c  |  33 ++-
src/storage/storage_backend_vstorage.c  |  10 +-
src/storage/storage_backend_zfs.c       |  15 +-
src/storage/storage_driver.c            | 351 ++++++++++++++++++++------------
src/storage/storage_util.c              | 146 ++++++-------
src/storage/storage_util.h              |  39 ++--
tests/storagevolxml2argvtest.c          |   7 +-
26 files changed, 1047 insertions(+), 499 deletions(-)
[libvirt] [PATCH 00/10] Enable direct use of secondary drivers
Posted by Daniel P. Berrangé 7 years, 3 months ago 
Currently the secondary drivers can only be used if you have a
connection to a primary hypervisor driver. This series introduces
explicit URIs that allow opening a connection that only talks to a
specific secondary driver. In the future these URIs will resolve to
individual daemons containing those drivers.

This also allows us to fix long standing problems with most code that
uses secrets internally. We need to pass a virConnectPtr into such code
but some call stacks don't have a connection available. In some cases we
open a temporary connection to the QEMU driver, but this is suboptimal
for deployments without the QEMU driver present.

Daniel P. Berrangé (10):
  storage: move driver registration back to end of the file
  storage: allow opening with storage:///system and storage:///session
    URIs
  network: move driver registration back to end of the file
  network: allow opening with network:///system and network:///session
    URIs
  nwfilter: allow opening with nwfilter:///system URI
  interface: allow opening with interface:///system and
    interface:///session URIs
  nodedev: allow opening with nodedev:///system and nodedev:///session
    URIs
  secret: allow opening with secret:///system and secret:///session URIs
  storage: open secret driver connection at time of use
  storage: remove virConnectPtr from all backend functions

 src/interface/interface_backend_netcf.c |  98 ++++++++-
 src/interface/interface_backend_udev.c  |  97 ++++++++-
 src/network/bridge_driver.c             | 185 +++++++++++++----
 src/network/bridge_driver_platform.h    |   3 +
 src/node_device/node_device_driver.c    |  73 ++++++-
 src/node_device/node_device_driver.h    |   9 +
 src/node_device/node_device_hal.c       |  18 ++
 src/node_device/node_device_udev.c      |  19 ++
 src/nwfilter/nwfilter_driver.c          |  83 ++++++++
 src/secret/secret_driver.c              |  95 +++++++++
 src/storage/storage_backend.h           |  45 ++--
 src/storage/storage_backend_disk.c      |  30 +--
 src/storage/storage_backend_fs.c        |  15 +-
 src/storage/storage_backend_gluster.c   |   9 +-
 src/storage/storage_backend_iscsi.c     |  24 +--
 src/storage/storage_backend_logical.c   |  38 ++--
 src/storage/storage_backend_mpath.c     |   5 +-
 src/storage/storage_backend_rbd.c       |  53 ++---
 src/storage/storage_backend_scsi.c      |  46 +++--
 src/storage/storage_backend_sheepdog.c  |  33 ++-
 src/storage/storage_backend_vstorage.c  |  10 +-
 src/storage/storage_backend_zfs.c       |  15 +-
 src/storage/storage_driver.c            | 351 ++++++++++++++++++++------------
 src/storage/storage_util.c              | 146 ++++++-------
 src/storage/storage_util.h              |  39 ++--
 tests/storagevolxml2argvtest.c          |   7 +-
 26 files changed, 1047 insertions(+), 499 deletions(-)

-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] Enable direct use of secondary drivers
Posted by Erik Skultety 7 years, 3 months ago 
On Fri, Jan 26, 2018 at 01:35:27PM +0000, Daniel P. Berrangé wrote:
> Currently the secondary drivers can only be used if you have a
> connection to a primary hypervisor driver. This series introduces
> explicit URIs that allow opening a connection that only talks to a
> specific secondary driver. In the future these URIs will resolve to
> individual daemons containing those drivers.
>
> This also allows us to fix long standing problems with most code that
> uses secrets internally. We need to pass a virConnectPtr into such code
> but some call stacks don't have a connection available. In some cases we
> open a temporary connection to the QEMU driver, but this is suboptimal
> for deployments without the QEMU driver present.
>
> Daniel P. Berrangé (10):
>   storage: move driver registration back to end of the file
>   storage: allow opening with storage:///system and storage:///session
>     URIs
>   network: move driver registration back to end of the file
>   network: allow opening with network:///system and network:///session
>     URIs
>   nwfilter: allow opening with nwfilter:///system URI
>   interface: allow opening with interface:///system and
>     interface:///session URIs
>   nodedev: allow opening with nodedev:///system and nodedev:///session
>     URIs
>   secret: allow opening with secret:///system and secret:///session URIs
>   storage: open secret driver connection at time of use
>   storage: remove virConnectPtr from all backend functions

Doc update would be nice as well, or are you planning on adding that as part of
separate series?

Erik

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] Enable direct use of secondary drivers
Posted by Daniel P. Berrangé 7 years, 3 months ago 
On Mon, Jan 29, 2018 at 12:46:11PM +0100, Erik Skultety wrote:
> On Fri, Jan 26, 2018 at 01:35:27PM +0000, Daniel P. Berrangé wrote:
> > Currently the secondary drivers can only be used if you have a
> > connection to a primary hypervisor driver. This series introduces
> > explicit URIs that allow opening a connection that only talks to a
> > specific secondary driver. In the future these URIs will resolve to
> > individual daemons containing those drivers.
> >
> > This also allows us to fix long standing problems with most code that
> > uses secrets internally. We need to pass a virConnectPtr into such code
> > but some call stacks don't have a connection available. In some cases we
> > open a temporary connection to the QEMU driver, but this is suboptimal
> > for deployments without the QEMU driver present.
> >
> > Daniel P. Berrangé (10):
> >   storage: move driver registration back to end of the file
> >   storage: allow opening with storage:///system and storage:///session
> >     URIs
> >   network: move driver registration back to end of the file
> >   network: allow opening with network:///system and network:///session
> >     URIs
> >   nwfilter: allow opening with nwfilter:///system URI
> >   interface: allow opening with interface:///system and
> >     interface:///session URIs
> >   nodedev: allow opening with nodedev:///system and nodedev:///session
> >     URIs
> >   secret: allow opening with secret:///system and secret:///session URIs
> >   storage: open secret driver connection at time of use
> >   storage: remove virConnectPtr from all backend functions
> 
> Doc update would be nice as well, or are you planning on adding that as part of
> separate series?

Good point, i'll look at that.  Since this series is acked, I might as well
post docs separately now.


Regards,
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 00/10] Enable direct use of secondary drivers
Posted by Michal Privoznik 7 years, 3 months ago 
On 01/26/2018 02:35 PM, Daniel P. Berrangé wrote:
> Currently the secondary drivers can only be used if you have a
> connection to a primary hypervisor driver. This series introduces
> explicit URIs that allow opening a connection that only talks to a
> specific secondary driver. In the future these URIs will resolve to
> individual daemons containing those drivers.
> 
> This also allows us to fix long standing problems with most code that
> uses secrets internally. We need to pass a virConnectPtr into such code
> but some call stacks don't have a connection available. In some cases we
> open a temporary connection to the QEMU driver, but this is suboptimal
> for deployments without the QEMU driver present.
> 
> Daniel P. Berrangé (10):
>   storage: move driver registration back to end of the file
>   storage: allow opening with storage:///system and storage:///session
>     URIs
>   network: move driver registration back to end of the file
>   network: allow opening with network:///system and network:///session
>     URIs
>   nwfilter: allow opening with nwfilter:///system URI
>   interface: allow opening with interface:///system and
>     interface:///session URIs
>   nodedev: allow opening with nodedev:///system and nodedev:///session
>     URIs
>   secret: allow opening with secret:///system and secret:///session URIs
>   storage: open secret driver connection at time of use
>   storage: remove virConnectPtr from all backend functions
> 
>  src/interface/interface_backend_netcf.c |  98 ++++++++-
>  src/interface/interface_backend_udev.c  |  97 ++++++++-
>  src/network/bridge_driver.c             | 185 +++++++++++++----
>  src/network/bridge_driver_platform.h    |   3 +
>  src/node_device/node_device_driver.c    |  73 ++++++-
>  src/node_device/node_device_driver.h    |   9 +
>  src/node_device/node_device_hal.c       |  18 ++
>  src/node_device/node_device_udev.c      |  19 ++
>  src/nwfilter/nwfilter_driver.c          |  83 ++++++++
>  src/secret/secret_driver.c              |  95 +++++++++
>  src/storage/storage_backend.h           |  45 ++--
>  src/storage/storage_backend_disk.c      |  30 +--
>  src/storage/storage_backend_fs.c        |  15 +-
>  src/storage/storage_backend_gluster.c   |   9 +-
>  src/storage/storage_backend_iscsi.c     |  24 +--
>  src/storage/storage_backend_logical.c   |  38 ++--
>  src/storage/storage_backend_mpath.c     |   5 +-
>  src/storage/storage_backend_rbd.c       |  53 ++---
>  src/storage/storage_backend_scsi.c      |  46 +++--
>  src/storage/storage_backend_sheepdog.c  |  33 ++-
>  src/storage/storage_backend_vstorage.c  |  10 +-
>  src/storage/storage_backend_zfs.c       |  15 +-
>  src/storage/storage_driver.c            | 351 ++++++++++++++++++++------------
>  src/storage/storage_util.c              | 146 ++++++-------
>  src/storage/storage_util.h              |  39 ++--
>  tests/storagevolxml2argvtest.c          |   7 +-
>  26 files changed, 1047 insertions(+), 499 deletions(-)
> 

ACK series, but there's a small problem in 09/10.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] Enable direct use of secondary drivers
Posted by Peter Krempa 7 years, 3 months ago 
On Fri, Jan 26, 2018 at 13:35:27 +0000, Daniel Berrange wrote:
> Currently the secondary drivers can only be used if you have a
> connection to a primary hypervisor driver. This series introduces
> explicit URIs that allow opening a connection that only talks to a
> specific secondary driver. In the future these URIs will resolve to
> individual daemons containing those drivers.

I'm so glad to see this, it felt awkward to hand off the connection
pointer through massive call chains. 

The only thing I'm afraid of in the future is that once the daemons are
split, if the user has a valid connection pointer, the code may still
fail if it fails to open a secondary connection to e.g. the storage
driver. All this while the original caller already had a valid pointer.

> 
> This also allows us to fix long standing problems with most code that
> uses secrets internally. We need to pass a virConnectPtr into such code
> but some call stacks don't have a connection available. In some cases we
> open a temporary connection to the QEMU driver, but this is suboptimal
> for deployments without the QEMU driver present.

That always grossed me out.

> 
> Daniel P. Berrangé (10):
>   storage: move driver registration back to end of the file
>   storage: allow opening with storage:///system and storage:///session
>     URIs
>   network: move driver registration back to end of the file
>   network: allow opening with network:///system and network:///session
>     URIs
>   nwfilter: allow opening with nwfilter:///system URI
>   interface: allow opening with interface:///system and
>     interface:///session URIs
>   nodedev: allow opening with nodedev:///system and nodedev:///session
>     URIs
>   secret: allow opening with secret:///system and secret:///session URIs

All of the patches above copy-paste code which has wrong coding style,
so all need to be fixed.

>   storage: open secret driver connection at time of use
>   storage: remove virConnectPtr from all backend functions

And the opening of the helper connection really needs a helper function.

ACK with the cosmetic tweaks

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] Enable direct use of secondary drivers
Posted by Daniel P. Berrangé 7 years, 3 months ago 
On Mon, Jan 29, 2018 at 11:58:11AM +0100, Peter Krempa wrote:
> On Fri, Jan 26, 2018 at 13:35:27 +0000, Daniel Berrange wrote:
> > Currently the secondary drivers can only be used if you have a
> > connection to a primary hypervisor driver. This series introduces
> > explicit URIs that allow opening a connection that only talks to a
> > specific secondary driver. In the future these URIs will resolve to
> > individual daemons containing those drivers.
> 
> I'm so glad to see this, it felt awkward to hand off the connection
> pointer through massive call chains. 
> 
> The only thing I'm afraid of in the future is that once the daemons are
> split, if the user has a valid connection pointer, the code may still
> fail if it fails to open a secondary connection to e.g. the storage
> driver. All this while the original caller already had a valid pointer.

Yep, that would be a new failure scenario, but I don't think it is too
serious in the greater scheme of all possible things that can go wrong.
Most likely problem would be that someone/something failed to start the
extra daemons. On most distros this won't be a problem, because systemd
socket activation will start then on-demand. 



Regards,
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 00/10] Enable direct use of secondary drivers
Posted by Daniel P. Berrangé 7 years, 3 months ago 
On Mon, Jan 29, 2018 at 11:58:11AM +0100, Peter Krempa wrote:

> > Daniel P. Berrangé (10):
> >   storage: move driver registration back to end of the file
> >   storage: allow opening with storage:///system and storage:///session
> >     URIs
> >   network: move driver registration back to end of the file
> >   network: allow opening with network:///system and network:///session
> >     URIs
> >   nwfilter: allow opening with nwfilter:///system URI
> >   interface: allow opening with interface:///system and
> >     interface:///session URIs
> >   nodedev: allow opening with nodedev:///system and nodedev:///session
> >     URIs
> >   secret: allow opening with secret:///system and secret:///session URIs
> 
> All of the patches above copy-paste code which has wrong coding style,
> so all need to be fixed.

Yes will fix.

> 
> >   storage: open secret driver connection at time of use
> >   storage: remove virConnectPtr from all backend functions
> 
> And the opening of the helper connection really needs a helper function.

I'm putting in this patch just before those two:

commit 409e665f3b1cb3049c588116713ffd37dd287e29
Author: Daniel P. Berrangé <berrange@redhat.com>
Date:   Wed Jan 31 18:21:52 2018 +0000

    driver: add some helpers for opening secondary driver connections
    
    Various parts of libvirt will want to open connections to secondary
    drivers. The right URI to use will depend on the context, so rather than
    duplicating that logic in various places, use some helper APIs. This
    will also make it easier for us to later pre-open/cache connections to
    avoid repeated opening & closing the same connectiong during autostart.
    
    Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/driver.c b/src/driver.c
index 04dd0a4431..a6a7ff925a 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -167,3 +167,33 @@ virDriverLoadModule(const char *name,
 
 
 /* XXX unload modules, but we can't until we can unregister libvirt drivers */
+
+virConnectPtr virGetConnectInterface(void)
+{
+    return virConnectOpen(geteuid() == 0 ? "interface:///system" : "interface:///session");
+}
+
+virConnectPtr virGetConnectNetwork(void)
+{
+    return virConnectOpen(geteuid() == 0 ? "network:///system" : "network:///session");
+}
+
+virConnectPtr virGetConnectNWFilter(void)
+{
+    return virConnectOpen(geteuid() == 0 ? "nwfilter:///system" : "nwfilter:///session");
+}
+
+virConnectPtr virGetConnectNodeDev(void)
+{
+    return virConnectOpen(geteuid() == 0 ? "nodedev:///system" : "nodedev:///session");
+}
+
+virConnectPtr virGetConnectSecret(void)
+{
+    return virConnectOpen(geteuid() == 0 ? "secret:///system" : "secret:///session");
+}
+
+virConnectPtr virGetConnectStorage(void)
+{
+    return virConnectOpen(geteuid() == 0 ? "storage:///system" : "storage:///session");
+}
diff --git a/src/driver.h b/src/driver.h
index 936c981603..fe0cec0923 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -105,4 +105,11 @@ int virDriverLoadModuleFull(const char *name,
                             const char *regfunc,
                             void **handle);
 
+virConnectPtr virGetConnectInterface(void);
+virConnectPtr virGetConnectNetwork(void);
+virConnectPtr virGetConnectNWFilter(void);
+virConnectPtr virGetConnectNodeDev(void);
+virConnectPtr virGetConnectSecret(void);
+virConnectPtr virGetConnectStorage(void);
+
 #endif /* __VIR_DRIVER_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 43deca9a52..c6e59e889a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1183,6 +1183,15 @@ virStorageVolClass;
 virStreamClass;
 
 
+# driver.h
+virGetConnectInterface;
+virGetConnectNetwork;
+virGetConnectNWFilter;
+virGetConnectNodeDev;
+virGetConnectSecret;
+virGetConnectStorage;
+
+
 # libvirt_internal.h
 virConnectSupportsFeature;
 virDomainMigrateBegin3;


Regards,
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

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.