Re: [PATCH v3 3/5] qemu: Propagate shared_filesystems

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, May 02, 2024 at 19:39:40 +0200, Andrea Bolognani wrote:
> virFileIsSharedFS() is the function that ultimately decides
> whether a filesystem should be considered shared, but the list
> of manually configured shared filesystems is part of the QEMU
> driver's configuration, so we need to pass the information
> through several layers in order to make use of it.
> 
> Note that with this change the list is propagated all the way
> through, but its contents are still ignored, so the behavior
> remains the same for now.
> 
> Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
> ---
>  src/lxc/lxc_controller.c         |  3 +-
>  src/lxc/lxc_driver.c             |  2 +-
>  src/lxc/lxc_process.c            |  4 +-
>  src/qemu/qemu_domain.c           |  7 ++-
>  src/qemu/qemu_extdevice.c        |  2 +-
>  src/qemu/qemu_migration.c        | 23 ++++-----
>  src/qemu/qemu_security.c         | 85 ++++++++++++++++++++++++--------
>  src/qemu/qemu_tpm.c              | 29 +++++++----
>  src/qemu/qemu_tpm.h              | 10 ++--
>  src/security/security_apparmor.c |  8 ++-
>  src/security/security_dac.c      | 47 ++++++++++++++----
>  src/security/security_driver.h   |  8 ++-
>  src/security/security_manager.c  | 33 ++++++++++---
>  src/security/security_manager.h  |  9 +++-
>  src/security/security_nop.c      |  5 ++
>  src/security/security_selinux.c  | 50 ++++++++++++++-----
>  src/security/security_stack.c    | 32 +++++++++---
>  src/util/virfile.c               | 13 +++--
>  src/util/virfile.h               |  3 +-
>  tests/securityselinuxlabeltest.c |  2 +-
>  tests/virfiletest.c              |  2 +-
>  21 files changed, 281 insertions(+), 96 deletions(-)

A general note/nit for this patch:

I'd go with 'exportedFilesystems' instead of 'sharedFilesystems'
throughout this patch(set) for any case when the list contains only the
paths configured by user (from previous commit).

That way it'd be IMO more clear that the list contains only filesystems
which are local on this host, rather than having also anything that's
consiedered shared.

It IMO would make sense to consider that e.g. also for the name of the
config option.

[...]

> @@ -1611,20 +1612,23 @@ qemuMigrationSrcIsAllowed(virDomainObj *vm,
>  }
>  
>  static bool
> -qemuMigrationSrcIsSafe(virDomainDef *def,
> -                       virQEMUCaps *qemuCaps,
> +qemuMigrationSrcIsSafe(virDomainObj *vm,
>                         size_t nmigrate_disks,
>                         const char **migrate_disks,
>                         unsigned int flags)

Sneaky refactor :)

>  
>  {
> +    qemuDomainObjPrivate *priv = vm->privateData;
> +    virQEMUCaps *qemuCaps = priv->qemuCaps;
> +    virQEMUDriver *driver = priv->driver;
> +    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
>      bool storagemigration = flags & (VIR_MIGRATE_NON_SHARED_DISK |
>                                       VIR_MIGRATE_NON_SHARED_INC);

[...]

With the rename very strongly considered:

Reviewed-by: Peter Krempa <pkrempa@xxxxxxxxxx>
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux