iso file ownership not restored after usage

Status changed to 'Confirmed' because the bug affects multiple users.

I think it makes sense to change the ownership of the iso while it is in use. This prevents an absent-minded developer from trying to remove or modify the iso while an install is going.

Since this issue was discovered only because of bug 2002773, there could be some added error handling such that if libvirt fails to install with the iso it's given, libvirt makes sure to set the permissions back to what they were when the iso was fed to libvirt.

Michal please recreate this on all our active release.
Summarize the behavior.

Most likely we will then have you debug why .qcow and .iso behave differently.

Bionic, Focal, Jammy, Kinetic - reproduced the issue. All of those are affected. Moreover, lunar is affected by that issue.

Thanks Michal for verifying,

We went into some debugging and found more as we wondered that the .qcow works fine and the .iso did not. So what is the difference?

We found the behavior depending on how things get added/removed.
TL;DR:
- any form of hot-remove => no restore of permissions
- and proper shutdown while attached => restore permissions

Details:

0. start & shutdown -> restores disk permission
perms: root:root before
virsh start vm1-1
perms: libvirt-qemu:kvm while running
virsh shutdown vm1-1
perms: root:root after

1. insert & eject cdrom -> does not restore permissions
perms: root:root before
virsh start vm1-1
virsh change-media vm1-1 hdb /var/lib/libvirt/images/ubuntu-22.04.2-desktop-amd64.iso --insert
perms: libvirt-qemu:kvm while running
virsh change-media vm1-1 hdb --eject
perms: libvirt-qemu:kvm after

2. hot attach & hot detach disk -> does not restore permissions
perms: root:root before
virsh start vm1-1
virsh attach-device vm1-1 test.xml
perms: libvirt-qemu:kvm while running
virsh detach-device vm1-1 test.xml
perms: libvirt-qemu:kvm after

3. hot attach & shutdown -> does restore permissions
perms: root:root before
virsh start vm1-1
virsh attach-device vm1-1 test.xml
perms: libvirt-qemu:kvm while running
virsh shutdown vm1-1
perms: libvirt-qemu:kvm after

4. insert cdrom & shutdown -> does not restore permissions
perms: root:root before
virsh start vm1-1
virsh change-media vm1-1 hdb /var/lib/libvirt/images/ubuntu-22.04.2-desktop-amd64.iso --insert
perms: libvirt-qemu:kvm while running
virsh shutdown vm1-1
perms: root:root before

From here:
1. please get a larger testbed based on >=jammy (to not hunt old issues) or even mantic to have debuginfod for source
2. Please debug #1 and #2 as they should be the most simple cases.
   2.1 Find in either logs [1] why/what it is doing
   2.2 If that isn't sufficient debug with gdb

[1]: https://libvirt.org/kbase/debuglogs.html

After investigating that bug - using gdb and libvirt debug logs (https://libvirt.org/kbase/debuglogs.html), I haven't noticed anything suspicious that would affect ownership restoration.
From here, I'd recommend forwarding that issue upstream.
They might have deeper, more specific knowledge to help with fixing that.
Moreover, the next argument against doing that and not fixing it straight away is that the workaround exists for that issue, and fixing it directly in Ubuntu - taking into consideration and taking a look -> https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/691590 - it possibly require a change within the control or rules files, which would not be recommended in that regard because those files are important to be equal "everywhere".

WDYT @Christian?

Yes, please repro on Fedora and anywhere on upstream libvirt git.
Just to make sure we haven't missed a delta that interacts unexpectedly.
After that yes, please report upstream before you debug even further.
They might already know something and shortcut your work, you can go back to debugging if they are not at all aware as well.

Note: Not exactly the same case, but in a very similar area.
I've found that using block devs changes back the group, wrong.
Once we have - any kind - of fix for this here we need to try that again and check if it is fixed by the same or a different issue.

$ qemu-img create -f qcow2 /tmp/test.qcow2 100M
Formatting '/tmp/test.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=104857600 lazy_refcounts=off refcount_bits=16
$ sudo modprobe nbd max_part=16
$ sudo qemu-nbd --connect=/dev/nbd0 /tmp/test.qcow2
$ sudo sgdisk -n 0:0:0 /dev/nbd0
Creating new GPT entries in memory.
The operation has completed successfully
$ ll /dev/nbd0
brw-rw---- 1 root disk 43, 0 Jul 12 05:55 /dev/nbd0
$ cat > testdisk.xml << EOF
> <disk type='block' device='disk'>
> <driver name='qemu' type='raw' cache='none'/>
> <source dev='/dev/nbd0'/>
> <target dev='vdc' bus='virtio'/>
> </disk>
> EOF
$ virsh attach-device j testdisk.xml
Device attached successfully
$ virsh detach-device j testdisk.xml
$ ll /dev/nbd0
brw-rw---- 1 root root 43, 0 Jul 12 06:36 /dev/nbd0

=> So it restored the user (interestingly in both hotplug and static config!!) but it restored the group badly (kvm->root instead of kvm->disk).

The issue also affects libvirt 9.6.0 (upstream) (I did build that one on Mantic VM).

I've created the bug report:
https://gitlab.com/libvirt/libvirt/-/issues/512

Waiting for response

I got the answer that this is caused by design.
Full answer in the gitlab link above.

I'm partially ok with this, it explains:
1. Why it isn't restored in the first place
2. Why our experiments in comment 5 behaved so different as the hot add/remove was a qcow that was not readonly, due to that it behaved differently than cdrom insert (which automatically is readonly).

To be sure I combined all former experiments in one and re-run them checking if it all is consistent in regard to the explanation we got.

The last element of confusion out of our experiments was about the nbd (comment 7) being changed back to root:root is also clear now. This reminded me of bug 691590 where we fixed this for "normal" files.
/dev/nbd is stored on devfs which has no xattr support that is needed to "remember" the owner.
I'll bring that up upstream as well for awareness.
=> https://gitlab.com/libvirt/libvirt/-/issues/517

Notes on full repro of all cases:

# Prep different kind of potential attachments
```
# 1. Create a libvirt guest your preferred way
# 2. get an iso of your choice
$ wget https://releases.ubuntu.com/22.04.2/ubuntu-22.04.2-live-server-amd64.iso
$ sudo mv ubuntu-22.04.2-live-server-amd64.iso /var/lib/libvirt/images/
$ sudo ls -laF /var/lib/libvirt/images/ubuntu-22.04.2-live-server-amd64.iso
-rw-rw-r-- 1 ubuntu ubuntu 1975971840 Feb 17 21:57 /var/lib/libvirt/images/ubuntu-22.04.2-live-server-amd64.iso
# 3. Create test image
$ sudo qemu-img create -f qcow2 /var/lib/libvirt/images/test.qcow2 100M
$ ll /var/lib/libvirt/images/test.qcow2
-rw-r--r-- 1 root root 196616 Aug 7 08:58 /var/lib/libvirt/images/test.qcow2
# 4. Create test to be available as disk
$sudo qemu-img create -f qcow2 /var/lib/libvirt/images/test-nbd.qcow2 100M
$ sudo qemu-nbd --connect=/dev/nbd0 /var/lib/libvirt/images/test-nbd.qcow2
$ sudo sgdisk -n 0:0:0 /dev/nbd0
Creating new GPT entries in memory.
The operation has completed successfully.
$ ll /dev/nbd0
brw-rw---- 1 root disk 43, 0 Aug 7 09:00 /dev/nbd0
```

So after this we have:
- an .iso that is user owned
- a .qcow2 that is root:root
- a block device that is root:disk

In one shot that looks like:
```
$ sudo ls -laF /var/lib/libvirt/images/ubuntu-22.04.2-live-server-amd64.iso /var/lib/libvirt/images/test.qcow2 /dev/nbd0
brw-rw---- 1 root disk 43, 0 Aug 7 09:00 /dev/nbd0
-rw-r--r-- 1 root root 196616 Aug 7 08:58 /var/lib/libvirt/images/test.qcow2
-rw-rw-r-- 1 ubuntu ubuntu 1975971840 Feb 17 21:57 /var/lib/libvirt/images/ubuntu-22.04.2-live-server-amd64.iso
```

# Variant A - start and stop statically with the guest

Guest XML section for those devices:
```
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' cache='none'/>
      <source file='/var/lib/libvirt/images/test.qcow2'/>
      <target dev='vdd' bus='virtio'/>
    </disk>
    <disk type='block' device='disk'>
      <driver name='qemu' type='raw' cache='none'/>
      <source dev='/dev/nbd0'/>
      <target dev='vde' bus='virtio'/>
    </disk>
    <disk type='file' device='cdrom'>
      <driver name='qemu' type='raw'/>
      <source file='/var/lib/libvirt/images/ubuntu-22.04.2-live-server-amd64.iso'/>
      <target dev='sda' bus='scsi'/>
      <readonly/>
    </disk>
```

Pre
```
$ sudo ls -laF /var/lib/libvirt/...