[libvirt] [PATCH v4 00/15] Implement alternative metadata locking

Michal Privoznik posted 15 patches 6 years ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1542193371.git.mprivozn@redhat.com
cfg.mk                             |   4 +-
src/libvirt_private.syms           |   1 +
src/locking/lock_daemon_dispatch.c |  11 +-
src/locking/lock_driver.h          |  12 -
src/locking/lock_driver_lockd.c    | 421 ++++++++++-------------------
src/locking/lock_driver_lockd.h    |   1 -
src/locking/lock_driver_sanlock.c  |  44 +--
src/locking/lock_manager.c         |  10 +-
src/lxc/lxc_controller.c           |   3 +-
src/lxc/lxc_driver.c               |   2 +-
src/qemu/qemu_conf.c               |   1 -
src/qemu/qemu_conf.h               |   2 +-
src/qemu/qemu_domain.c             |   7 +
src/qemu/qemu_domain.h             |   3 +
src/qemu/qemu_driver.c             |   3 -
src/qemu/qemu_extdevice.c          |  16 +-
src/qemu/qemu_extdevice.h          |   4 +-
src/qemu/qemu_process.c            |   9 +-
src/qemu/qemu_security.c           |  87 ++++--
src/qemu/qemu_security.h           |   4 +-
src/qemu/qemu_tpm.c                |  24 +-
src/qemu/qemu_tpm.h                |   4 +-
src/security/security_dac.c        |  54 ++--
src/security/security_driver.h     |   3 +-
src/security/security_manager.c    | 259 +++++++++---------
src/security/security_manager.h    |  22 +-
src/security/security_selinux.c    |  53 ++--
src/security/security_stack.c      |   5 +-
src/util/virlockspace.c            |  15 +-
src/util/virlockspace.h            |   4 -
src/util/virprocess.c              |  82 ++++--
src/util/virprocess.h              |  16 ++
tests/seclabeltest.c               |   2 +-
tests/securityselinuxlabeltest.c   |   2 +-
tests/securityselinuxtest.c        |   2 +-
tests/testutilsqemu.c              |   2 +-
tests/virlockspacetest.c           |  29 +-
37 files changed, 573 insertions(+), 650 deletions(-)
[libvirt] [PATCH v4 00/15] Implement alternative metadata locking
Posted by Michal Privoznik 6 years ago
v4 of:

https://www.redhat.com/archives/libvir-list/2018-October/msg00861.html

diff to v3:
- Introduced a config knob to enable/disable metadata locking (except
  not really). We want to have a knob that enables/disables remembering
  of original owner. This knob in turn enables metadata locking. The
  reason is that metadata locking on its own doesn't make any sense.
  Anyway, the qemu.conf change is not done (it'll be done in upcoming
  patch set that implements original owner remembering), so if you want
  to see these patches in action you'll need to apply the following
  patch:

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 32da9a7351..0080b0d021 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -347,6 +347,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
     if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST)))
         goto error;
 
+    cfg->rememberOwner = true;
+
     if (privileged &&
         qemuDomainNamespaceAvailable(QEMU_DOMAIN_NS_MOUNT) &&
         virBitmapSetBit(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) < 0)


- I've fixed small issues raised in review of v3.
Note that patches 01 and 02 are ACKed already but I'm sending them for
completeness (probably doesn't make much sense to merge them while this
is still under review, does it?).


Michal Prívozník (15):
  virprocess: Introduce virProcessRunInFork
  virprocess: Make virProcessRunInMountNamespace use virProcessRunInFork
  qemu_tpm: Pass virDomainObjPtr instead of virDomainDefPtr
  qemu_domain: Track if domain remembers original owner
  virSecurityManagerTransactionCommit: Do metadata locking iff enabled
    in config
  security_manager: Rework metadata locking
  Revert "security_manager: Load lock plugin on init"
  Revert "qemu_conf: Introduce metadata_lock_manager"
  Revert "lock_manager: Allow disabling configFile for
    virLockManagerPluginNew"
  Revert "lock_driver: Introduce VIR_LOCK_MANAGER_ACQUIRE_ROLLBACK"
  Revert "lock_driver: Introduce
    VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA"
  Revert "_virLockManagerLockDaemonPrivate: Move @hasRWDisks into dom
    union"
  Revert "lock_driver: Introduce new
    VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON"
  Revert "lock_driver_lockd: Introduce
    VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_METADATA flag"
  Revert "virlockspace: Allow caller to specify start and length offset
    in virLockSpaceAcquireResource"

 cfg.mk                             |   4 +-
 src/libvirt_private.syms           |   1 +
 src/locking/lock_daemon_dispatch.c |  11 +-
 src/locking/lock_driver.h          |  12 -
 src/locking/lock_driver_lockd.c    | 421 ++++++++++-------------------
 src/locking/lock_driver_lockd.h    |   1 -
 src/locking/lock_driver_sanlock.c  |  44 +--
 src/locking/lock_manager.c         |  10 +-
 src/lxc/lxc_controller.c           |   3 +-
 src/lxc/lxc_driver.c               |   2 +-
 src/qemu/qemu_conf.c               |   1 -
 src/qemu/qemu_conf.h               |   2 +-
 src/qemu/qemu_domain.c             |   7 +
 src/qemu/qemu_domain.h             |   3 +
 src/qemu/qemu_driver.c             |   3 -
 src/qemu/qemu_extdevice.c          |  16 +-
 src/qemu/qemu_extdevice.h          |   4 +-
 src/qemu/qemu_process.c            |   9 +-
 src/qemu/qemu_security.c           |  87 ++++--
 src/qemu/qemu_security.h           |   4 +-
 src/qemu/qemu_tpm.c                |  24 +-
 src/qemu/qemu_tpm.h                |   4 +-
 src/security/security_dac.c        |  54 ++--
 src/security/security_driver.h     |   3 +-
 src/security/security_manager.c    | 259 +++++++++---------
 src/security/security_manager.h    |  22 +-
 src/security/security_selinux.c    |  53 ++--
 src/security/security_stack.c      |   5 +-
 src/util/virlockspace.c            |  15 +-
 src/util/virlockspace.h            |   4 -
 src/util/virprocess.c              |  82 ++++--
 src/util/virprocess.h              |  16 ++
 tests/seclabeltest.c               |   2 +-
 tests/securityselinuxlabeltest.c   |   2 +-
 tests/securityselinuxtest.c        |   2 +-
 tests/testutilsqemu.c              |   2 +-
 tests/virlockspacetest.c           |  29 +-
 37 files changed, 573 insertions(+), 650 deletions(-)

-- 
2.18.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 00/15] Implement alternative metadata locking
Posted by John Ferlan 6 years ago

On 11/14/18 7:44 AM, Michal Privoznik wrote:
> v4 of:
> 
> https://www.redhat.com/archives/libvir-list/2018-October/msg00861.html
> 
> diff to v3:
> - Introduced a config knob to enable/disable metadata locking (except
>   not really). We want to have a knob that enables/disables remembering
>   of original owner. This knob in turn enables metadata locking. The
>   reason is that metadata locking on its own doesn't make any sense.
>   Anyway, the qemu.conf change is not done (it'll be done in upcoming
>   patch set that implements original owner remembering), so if you want
>   to see these patches in action you'll need to apply the following
>   patch:
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 32da9a7351..0080b0d021 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -347,6 +347,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>      if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST)))
>          goto error;
>  
> +    cfg->rememberOwner = true;
> +
>      if (privileged &&
>          qemuDomainNamespaceAvailable(QEMU_DOMAIN_NS_MOUNT) &&
>          virBitmapSetBit(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) < 0)
> 
> 
> - I've fixed small issues raised in review of v3.
> Note that patches 01 and 02 are ACKed already but I'm sending them for
> completeness (probably doesn't make much sense to merge them while this
> is still under review, does it?).
> 
> 
> Michal Prívozník (15):
>   virprocess: Introduce virProcessRunInFork
>   virprocess: Make virProcessRunInMountNamespace use virProcessRunInFork
>   qemu_tpm: Pass virDomainObjPtr instead of virDomainDefPtr
>   qemu_domain: Track if domain remembers original owner
>   virSecurityManagerTransactionCommit: Do metadata locking iff enabled
>     in config
>   security_manager: Rework metadata locking
>   Revert "security_manager: Load lock plugin on init"
>   Revert "qemu_conf: Introduce metadata_lock_manager"
>   Revert "lock_manager: Allow disabling configFile for
>     virLockManagerPluginNew"
>   Revert "lock_driver: Introduce VIR_LOCK_MANAGER_ACQUIRE_ROLLBACK"
>   Revert "lock_driver: Introduce
>     VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA"
>   Revert "_virLockManagerLockDaemonPrivate: Move @hasRWDisks into dom
>     union"
>   Revert "lock_driver: Introduce new
>     VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON"
>   Revert "lock_driver_lockd: Introduce
>     VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_METADATA flag"
>   Revert "virlockspace: Allow caller to specify start and length offset
>     in virLockSpaceAcquireResource"
> 
>  cfg.mk                             |   4 +-
>  src/libvirt_private.syms           |   1 +
>  src/locking/lock_daemon_dispatch.c |  11 +-
>  src/locking/lock_driver.h          |  12 -
>  src/locking/lock_driver_lockd.c    | 421 ++++++++++-------------------
>  src/locking/lock_driver_lockd.h    |   1 -
>  src/locking/lock_driver_sanlock.c  |  44 +--
>  src/locking/lock_manager.c         |  10 +-
>  src/lxc/lxc_controller.c           |   3 +-
>  src/lxc/lxc_driver.c               |   2 +-
>  src/qemu/qemu_conf.c               |   1 -
>  src/qemu/qemu_conf.h               |   2 +-
>  src/qemu/qemu_domain.c             |   7 +
>  src/qemu/qemu_domain.h             |   3 +
>  src/qemu/qemu_driver.c             |   3 -
>  src/qemu/qemu_extdevice.c          |  16 +-
>  src/qemu/qemu_extdevice.h          |   4 +-
>  src/qemu/qemu_process.c            |   9 +-
>  src/qemu/qemu_security.c           |  87 ++++--
>  src/qemu/qemu_security.h           |   4 +-
>  src/qemu/qemu_tpm.c                |  24 +-
>  src/qemu/qemu_tpm.h                |   4 +-
>  src/security/security_dac.c        |  54 ++--
>  src/security/security_driver.h     |   3 +-
>  src/security/security_manager.c    | 259 +++++++++---------
>  src/security/security_manager.h    |  22 +-
>  src/security/security_selinux.c    |  53 ++--
>  src/security/security_stack.c      |   5 +-
>  src/util/virlockspace.c            |  15 +-
>  src/util/virlockspace.h            |   4 -
>  src/util/virprocess.c              |  82 ++++--
>  src/util/virprocess.h              |  16 ++
>  tests/seclabeltest.c               |   2 +-
>  tests/securityselinuxlabeltest.c   |   2 +-
>  tests/securityselinuxtest.c        |   2 +-
>  tests/testutilsqemu.c              |   2 +-
>  tests/virlockspacetest.c           |  29 +-
>  37 files changed, 573 insertions(+), 650 deletions(-)
> 

Consider the "Revert" patches all :

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

I ran the series through my Coverity checker and it didn't find anything new

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 00/15] Implement alternative metadata locking
Posted by Michal Privoznik 6 years ago
On 11/15/18 7:40 PM, John Ferlan wrote:
> 
> 
> On 11/14/18 7:44 AM, Michal Privoznik wrote:
>> v4 of:
>>
>> https://www.redhat.com/archives/libvir-list/2018-October/msg00861.html
>>
>> diff to v3:
>> - Introduced a config knob to enable/disable metadata locking (except
>>   not really). We want to have a knob that enables/disables remembering
>>   of original owner. This knob in turn enables metadata locking. The
>>   reason is that metadata locking on its own doesn't make any sense.
>>   Anyway, the qemu.conf change is not done (it'll be done in upcoming
>>   patch set that implements original owner remembering), so if you want
>>   to see these patches in action you'll need to apply the following
>>   patch:
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 32da9a7351..0080b0d021 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -347,6 +347,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>>      if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST)))
>>          goto error;
>>  
>> +    cfg->rememberOwner = true;
>> +
>>      if (privileged &&
>>          qemuDomainNamespaceAvailable(QEMU_DOMAIN_NS_MOUNT) &&
>>          virBitmapSetBit(cfg->namespaces, QEMU_DOMAIN_NS_MOUNT) < 0)
>>
>>
>> - I've fixed small issues raised in review of v3.
>> Note that patches 01 and 02 are ACKed already but I'm sending them for
>> completeness (probably doesn't make much sense to merge them while this
>> is still under review, does it?).
>>
>>
>> Michal Prívozník (15):
>>   virprocess: Introduce virProcessRunInFork
>>   virprocess: Make virProcessRunInMountNamespace use virProcessRunInFork
>>   qemu_tpm: Pass virDomainObjPtr instead of virDomainDefPtr
>>   qemu_domain: Track if domain remembers original owner
>>   virSecurityManagerTransactionCommit: Do metadata locking iff enabled
>>     in config
>>   security_manager: Rework metadata locking
>>   Revert "security_manager: Load lock plugin on init"
>>   Revert "qemu_conf: Introduce metadata_lock_manager"
>>   Revert "lock_manager: Allow disabling configFile for
>>     virLockManagerPluginNew"
>>   Revert "lock_driver: Introduce VIR_LOCK_MANAGER_ACQUIRE_ROLLBACK"
>>   Revert "lock_driver: Introduce
>>     VIR_LOCK_MANAGER_RESOURCE_TYPE_METADATA"
>>   Revert "_virLockManagerLockDaemonPrivate: Move @hasRWDisks into dom
>>     union"
>>   Revert "lock_driver: Introduce new
>>     VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON"
>>   Revert "lock_driver_lockd: Introduce
>>     VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_METADATA flag"
>>   Revert "virlockspace: Allow caller to specify start and length offset
>>     in virLockSpaceAcquireResource"
>>
>>  cfg.mk                             |   4 +-
>>  src/libvirt_private.syms           |   1 +
>>  src/locking/lock_daemon_dispatch.c |  11 +-
>>  src/locking/lock_driver.h          |  12 -
>>  src/locking/lock_driver_lockd.c    | 421 ++++++++++-------------------
>>  src/locking/lock_driver_lockd.h    |   1 -
>>  src/locking/lock_driver_sanlock.c  |  44 +--
>>  src/locking/lock_manager.c         |  10 +-
>>  src/lxc/lxc_controller.c           |   3 +-
>>  src/lxc/lxc_driver.c               |   2 +-
>>  src/qemu/qemu_conf.c               |   1 -
>>  src/qemu/qemu_conf.h               |   2 +-
>>  src/qemu/qemu_domain.c             |   7 +
>>  src/qemu/qemu_domain.h             |   3 +
>>  src/qemu/qemu_driver.c             |   3 -
>>  src/qemu/qemu_extdevice.c          |  16 +-
>>  src/qemu/qemu_extdevice.h          |   4 +-
>>  src/qemu/qemu_process.c            |   9 +-
>>  src/qemu/qemu_security.c           |  87 ++++--
>>  src/qemu/qemu_security.h           |   4 +-
>>  src/qemu/qemu_tpm.c                |  24 +-
>>  src/qemu/qemu_tpm.h                |   4 +-
>>  src/security/security_dac.c        |  54 ++--
>>  src/security/security_driver.h     |   3 +-
>>  src/security/security_manager.c    | 259 +++++++++---------
>>  src/security/security_manager.h    |  22 +-
>>  src/security/security_selinux.c    |  53 ++--
>>  src/security/security_stack.c      |   5 +-
>>  src/util/virlockspace.c            |  15 +-
>>  src/util/virlockspace.h            |   4 -
>>  src/util/virprocess.c              |  82 ++++--
>>  src/util/virprocess.h              |  16 ++
>>  tests/seclabeltest.c               |   2 +-
>>  tests/securityselinuxlabeltest.c   |   2 +-
>>  tests/securityselinuxtest.c        |   2 +-
>>  tests/testutilsqemu.c              |   2 +-
>>  tests/virlockspacetest.c           |  29 +-
>>  37 files changed, 573 insertions(+), 650 deletions(-)
>>
> 
> Consider the "Revert" patches all :
> 
> Reviewed-by: John Ferlan <jferlan@redhat.com>
> 
> John
> 
> I ran the series through my Coverity checker and it didn't find anything new
> 

Thank you for the review. I've pushed these.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list