Re: [PATCH 8/9] virStorageFileGetMetadataRecurse: Allow format probing under special circumstances

>> Namely a user creates an overlay on top of single raw/qcow2 image and >> expects it to work. And it's not just random users, libguestfs and >> openstack also neglected to set the backing format. >> > > Yes, and they are getting patched. Belatedly, but better late than never. In that case, qemu-img should also be fixed to forbid 'create' without -F. Otherwise it's hard to argue that it's a wrong thing to do.

>>>> The price for this is that libvirt will need to keep the image format >>>> detector still current and working or replace it by invocation of >>>> qemu-img. >>> >>> Maybe. Any format that qemu recognizes but libvirt does not risks a case >>> where libvirt probes the image as raw but lets qemu re-probe the image and >> >> That doesn't happen with blockdev. We dont' let qemu probe. > > We are just shifting the burden on who causes the data corruption when a > probe goes wrong - it used to be qemu, now it is libvirt. > > >>> >>> I disagree with the logic here. What we really need is: >>> >>> If the backing format was not specified, we probe to see what is there. If >>> the result of that probe is raw, it is safe to treat the image as raw. If >>> the result is anything else, we must report an error stating that what we >>> probed could not be trusted unless the user adds an explicit backing format >>> (either confirming that our probe was correct, or with the correct format >>> overriding what we mis-probed). >> >> As noted above, using this logic would be pointless. We are better off >> just reporting the error always if we also don't allow qcow2 without >> backing file to be used as it was previously used. > > Then report the error always. Well that's what we do right now. It seems kind of tempting to call this a right thing but let me summarize the semantics: - The "true" detection cases are not problematic - advantage is that existing (arguably suboptimal) configurations will keep working if we detect - For the "false" detection cases: - misdetection does not breach security (given that sVirt is used) - data corruption can occur if libvirt detects something else than qemu - this is true both directions (qcow2->raw, raw->qcow2) and the tradeoff: 1) If we allow detection, we trade compatibility for the (small) possibility of having to deal with corruption. 2) If we disallow detection we trade regression of behaviour for data corruption not being our problem. I started this trhead because I feel that the value of 1) is more than 2). Especially short term since qemu-img's default behaviour is allowing creation of images which would break with libvirt and the fact that we've tolerated the wrong behaviour for years. Additionally I think that we could just get rid of the copy of the image detection copy in libvirt and replace it by invocation of qemu-img. That way we could avoid any discrepancies in the detection process in the first place.