On 3/28/19 11:04 AM, Michal Privoznik wrote: > Here is the problem: If all disks had XATTRs (i.e. domains using > them were started with owner remembering turned on) then > refcounting implemented in XATTRs would work nicely and we could > set the whole backing chain and restore it later. But world is > not that simple. As soon as there is one domain that was started > with the feature turned off (or simply by older libvirt), the > XATTR refounting does not reflect the actual number of uses by > running domains and therefore any attempt to restore might cut > off the old domain. > > There is no simple way around this. Except artificially turning > the feature off for the rest of the backing chain. > Okay so I studied this some more, here's my understanding. VM has disk foo.qcow2 with a backing image backing.qcow2. We start the VM, foo.qcow2 gets an svirt_image_t:... exclusive label. backing.qcow2 which is readonly for the VM and may be shared with other VMs, gets the readonly label virt_content_t With label remembering added, we need to consider the case of VMs that were started without any remembering xattrs applied, so like from a previous libvirt version, or maybe a VM started with remembering or labeling disabled. In those cases, we should skip the xattr remembering altogether. The problem is, we don't have any way to really detect this scenario or its not important enough to implement. In the case of the top foo.qcow2 image, even if it's in use by some pre-remembering scenario, because our VM is requesting exclusive access to it anyways we don't need to care about any back compat, so we set our labels and use xattrs from here on out. That parts easy. But the readonly backing.qcow2 could validly be in use by pre-remembering VMs, and that's not feasible to detect one way or the other. So for correctness we assume backing.qcow2 is pre-remembering and maintain that behavior. In other words, we always skip manipulating xattr refcounts for backing images. Which seems reasonable to me. So for the approach and the code here: Reviewed-by: Cole Robinson <crobinso@xxxxxxxxxx> But the follow on from that explanation is that this isn't really a distinction between top vs backing images, it's exclusive vs shared resources. Anything that's plausibly shareable we can't use xattrs (unless we come up with a different approach). Specifically it means that this logic should be applied to other shared cases like disk->src->shared and disk->src->readonly. I tested a <shareable/> disk and xattrs are incremented but never decremented, I think because of conditions that skip label restore in virSecuritySELinuxRestoreImageLabelInt. That's a separate set of issues that should be fixed before the last patch is applied. Thanks, Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
