On Fri, Apr 24, 2020 at 10:25:15 -0500, Eric Blake wrote: > On 4/24/20 10:18 AM, Peter Krempa wrote: > > On Fri, Apr 24, 2020 at 09:52:00 -0500, Eric Blake wrote: > > > On 4/24/20 4:24 AM, Peter Krempa wrote: > > > > The implementation was never finished in libvirt. Remove it. > > > > > > > > Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> > > > > --- > > > > src/util/virstoragefile.c | 19 ++----------------- > > > > 1 file changed, 2 insertions(+), 17 deletions(-) > > > > > > Shouldn't we at least recognize if there is a qcow2 file with the extension > > > header set, at which point we can error out and tell the user their image is > > > unsupported, rather than trying to use it without the data file? > > > > I don't think it's scalable to do so unless qemu exposes a whitelist of > > supported extensions we can declare we support. Otherwise we are > > band-aiding stuff we lear that exists and ignore any other extensions. > > external data files have an incompatible feature bit set in addition to the > header extension. Maybe whitelisting header extensions is prohibitive, but > we should be able to recognize when a qcow2 file has an incompatible feature > bit set that we are not aware of, and assume that since we don't know about > the feature, we also don't know how to safely pass that file to qemu. > > That is, we could quickly reject qcow2 files with offset 72-79 containing > bit 2 set, until we are ready to support external data files in libvirt. Are there any specific reasons to do so? If so we should track them in a bug/issue in the first place. Additionally I'm against extending the qcow2 header parser beyond what's strictly necessary. In the end our reimplementation of of the parser is considered a security measure. I've pushed the patches for now. If somebody will require the 'data file' field the last patch can always be reverted.