Re: [PATCH 4/4] util: qcow2GetExtensions: Remove support for 'data file' extension

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux