[libvirt] [PATCH 1/1] Revert "Revert "qemu: Temporary disable owner remembering""

After this commit, QEMU domains with PCI hostdevs running with managed=true started to fail to launch with the following error: error : virProcessRunInFork:1170 : internal error: child reported (status=125): unable to open /dev/vfio/1: Device or resource busy One way to avoid this issue is to disable this new feature in qemu.conf, setting remember_owner=0. However, given that this feature is enabled by default and it is breaking domains that were running before it, it is best to revert the change until it is fixed for this scenario as well. This reverts commit 8695793d726e37e31498f9fcbf0bce0adc56dc91. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- docs/news.xml | 13 ------------- src/qemu/libvirtd_qemu.aug | 1 - src/qemu/qemu.conf | 5 ----- src/qemu/qemu_conf.c | 4 ---- src/qemu/test_libvirtd_qemu.aug.in | 1 - 5 files changed, 24 deletions(-) diff --git a/docs/news.xml b/docs/news.xml index 07d4575a65..8931f74bb9 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -25,19 +25,6 @@ <section title="New features"> </section> <section title="Improvements"> - <change> - <summary> - Remember original owners and SELinux labels of files - </summary> - <description> - When a domain is starting up libvirt changes DAC and - SELinux labels so that domain can access it. However, - it never remembered the original labels and therefore - the file was returned back to <code>root:root</code>. - With this release, the original labels are remembered - and restored properly. - </description> - </change> </section> <section title="Bug fixes"> </section> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index eea9094d39..6821cc4a29 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -72,7 +72,6 @@ module Libvirtd_qemu = | str_entry "user" | str_entry "group" | bool_entry "dynamic_ownership" - | bool_entry "remember_owner" | str_array_entry "cgroup_controllers" | str_array_entry "cgroup_device_acl" | int_entry "seccomp_sandbox" diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index fd2ed9dc21..1969b3f0a1 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -456,11 +456,6 @@ # Set to 0 to disable file ownership changes. #dynamic_ownership = 1 -# Whether libvirt should remember and restore the original -# ownership over files it is relabeling. Defaults to 1, set -# to 0 to disable the feature. -#remember_owner = 1 - # What cgroup controllers to make use of with QEMU guests # # - 'cpu' - use for scheduler tunables diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index e0195dac29..8312f99f80 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -125,7 +125,6 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) cfg->group = (gid_t)-1; } cfg->dynamicOwnership = privileged; - cfg->rememberOwner = privileged; cfg->cgroupControllers = -1; /* -1 == auto-detect */ @@ -891,9 +890,6 @@ virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg, if (virConfGetValueBool(conf, "dynamic_ownership", &cfg->dynamicOwnership) < 0) return -1; - if (virConfGetValueBool(conf, "remember_owner", &cfg->rememberOwner) < 0) - return -1; - if (virConfGetValueStringList(conf, "cgroup_controllers", false, &controllers) < 0) return -1; diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 388ba24b8b..50b728ad22 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -44,7 +44,6 @@ module Test_libvirtd_qemu = { "user" = "root" } { "group" = "root" } { "dynamic_ownership" = "1" } -{ "remember_owner" = "1" } { "cgroup_controllers" { "1" = "cpu" } { "2" = "devices" } -- 2.21.0

On 7/17/19 7:20 PM, Daniel Henrique Barboza wrote:
After this commit, QEMU domains with PCI hostdevs running with managed=true started to fail to launch with the following error:
error : virProcessRunInFork:1170 : internal error: child reported (status=125): unable to open /dev/vfio/1: Device or resource busy
One way to avoid this issue is to disable this new feature in qemu.conf, setting remember_owner=0. However, given that this feature is enabled by default and it is breaking domains that were running before it, it is best to revert the change until it is fixed for this scenario as well.
Well, ideally, we want the feature to be turned on by default, just like namespaces for instance. I've temporarily disabled the feature back in the day because we were close to release and it turned out the feature was not ready. But now there is still plenty of time to fix it. Anyway, I'll investigate. Meanwhile, can you share your <hostdev/> config or even better the full domain definition please? Michal

On 7/18/19 8:46 AM, Michal Privoznik wrote:
On 7/17/19 7:20 PM, Daniel Henrique Barboza wrote:
After this commit, QEMU domains with PCI hostdevs running with managed=true started to fail to launch with the following error:
error : virProcessRunInFork:1170 : internal error: child reported (status=125): unable to open /dev/vfio/1: Device or resource busy
One way to avoid this issue is to disable this new feature in qemu.conf, setting remember_owner=0. However, given that this feature is enabled by default and it is breaking domains that were running before it, it is best to revert the change until it is fixed for this scenario as well.
Well, ideally, we want the feature to be turned on by default, just like namespaces for instance. I've temporarily disabled the feature back in the day because we were close to release and it turned out the feature was not ready. But now there is still plenty of time to fix it.
Anyway, I'll investigate. Meanwhile, can you share your <hostdev/> config or even better the full domain definition please?
Okay, so I think I know what is going on. You have two <hostdev/>-s in your domain and both of them belong to the same IOMMU group, right? If that is the case, then the same path appears twice in the list of paths passed to virSecurityManagerMetadataLock(). For instance, in my case: Thread 2.1 "libvirtd" hit Breakpoint 3, virSecurityManagerMetadataLock (mgr=0x7f365c026660, paths=0x7f36a001c1e0, npaths=6) at security/security_manager.c:1283 1283 { virSecurityManagerMetadataLock 1 # p *paths@npaths $6 = {0x7f36a0027720 "/var/lib/libvirt/images/fedora.qcow2", 0x7f36a001a660 "/var/lib/libvirt/images/fd.img", 0x7f36a001c470 "/dev/disk/by-path/ip-10.37.132.232:3260-iscsi-iqn.2017-03.com.mprivozn:server-lun-1", 0x7f36a00262b0 "/dev/vfio/9", 0x7f36a0025e20 "/dev/vfio/9", 0x7f36a001d880 "/var/lib/libvirt/qemu/channel/target/domain-1-fedora/org.qemu.guest_agent.0"} The "/dev/vfio/9" path is there twice because I have this graphics card that has two functions and I'm passing them both into the domain. The problem here is that when virSecurityManagerMetadataLock() gets to first /dev/vfio/9 path, it opens it and locks it. Then it wants to process the next path (which is the same path), but it fails open()-ing the path, because it's locked. Honestly, I don't know if that is expected behaviour, but even it if wasn't then we would fail to lock the path second time. I'm proposing a fix in no time. Michal

On 7/18/19 5:48 AM, Michal Privoznik wrote:
On 7/18/19 8:46 AM, Michal Privoznik wrote:
On 7/17/19 7:20 PM, Daniel Henrique Barboza wrote:
After this commit, QEMU domains with PCI hostdevs running with managed=true started to fail to launch with the following error:
error : virProcessRunInFork:1170 : internal error: child reported (status=125): unable to open /dev/vfio/1: Device or resource busy
One way to avoid this issue is to disable this new feature in qemu.conf, setting remember_owner=0. However, given that this feature is enabled by default and it is breaking domains that were running before it, it is best to revert the change until it is fixed for this scenario as well.
Well, ideally, we want the feature to be turned on by default, just like namespaces for instance. I've temporarily disabled the feature back in the day because we were close to release and it turned out the feature was not ready. But now there is still plenty of time to fix it.
Anyway, I'll investigate. Meanwhile, can you share your <hostdev/> config or even better the full domain definition please?
Okay, so I think I know what is going on. You have two <hostdev/>-s in your domain and both of them belong to the same IOMMU group, right?
Yes. I forgot to mention that in this reversal patch, sorry. The error is reproduced with a VM using a PCI Multifunction card - alll 4 functions in the IOMMU group 1. I'll test your already proposed fix in a few moments. Just need a hit of coffee first :) Thanks, DHB
If that is the case, then the same path appears twice in the list of paths passed to virSecurityManagerMetadataLock(). For instance, in my case:
Thread 2.1 "libvirtd" hit Breakpoint 3, virSecurityManagerMetadataLock (mgr=0x7f365c026660, paths=0x7f36a001c1e0, npaths=6) at security/security_manager.c:1283 1283 { virSecurityManagerMetadataLock 1 # p *paths@npaths $6 = {0x7f36a0027720 "/var/lib/libvirt/images/fedora.qcow2", 0x7f36a001a660 "/var/lib/libvirt/images/fd.img", 0x7f36a001c470 "/dev/disk/by-path/ip-10.37.132.232:3260-iscsi-iqn.2017-03.com.mprivozn:server-lun-1", 0x7f36a00262b0 "/dev/vfio/9", 0x7f36a0025e20 "/dev/vfio/9", 0x7f36a001d880 "/var/lib/libvirt/qemu/channel/target/domain-1-fedora/org.qemu.guest_agent.0"}
The "/dev/vfio/9" path is there twice because I have this graphics card that has two functions and I'm passing them both into the domain. The problem here is that when virSecurityManagerMetadataLock() gets to first /dev/vfio/9 path, it opens it and locks it. Then it wants to process the next path (which is the same path), but it fails open()-ing the path, because it's locked. Honestly, I don't know if that is expected behaviour, but even it if wasn't then we would fail to lock the path second time. I'm proposing a fix in no time.
Michal
participants (2)
-
Daniel Henrique Barboza
-
Michal Privoznik