diff mbox

Warn if a qcow (not qcow2) file is opened

Message ID 1246284289-25394-1-git-send-email-avi@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Avi Kivity June 29, 2009, 2:04 p.m. UTC
The qcow block driver format is no longer maintained and likely contains
serious data corruptors.  Urge users to stay away for it, and advertise
the new and improved replacement.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 block/qcow.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Comments

Kevin Wolf June 30, 2009, 8:34 a.m. UTC | #1
Avi Kivity schrieb:
> The qcow block driver format is no longer maintained and likely contains
> serious data corruptors.  Urge users to stay away for it, and advertise
> the new and improved replacement.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

vvfat is using qcow internally, so the warning will appear there, too.
Not that warning against vvfat would be a bad thing, but this error
message could be confusing.

Maybe we're lucky enough and vvfat survives a s/qcow/qcow2/, but I
really never wanted to touch that code...

Kevin
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amit Shah June 30, 2009, 12:05 p.m. UTC | #2
On (Mon) Jun 29 2009 [17:04:49], Avi Kivity wrote:
> The qcow block driver format is no longer maintained and likely contains
> serious data corruptors.  Urge users to stay away for it, and advertise
> the new and improved replacement.

Does this also print the message at the time of creating the image? (And
isn't qcow the default image format?)

		Amit
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Wolf June 30, 2009, 12:53 p.m. UTC | #3
Amit Shah schrieb:
> On (Mon) Jun 29 2009 [17:04:49], Avi Kivity wrote:
>> The qcow block driver format is no longer maintained and likely contains
>> serious data corruptors.  Urge users to stay away for it, and advertise
>> the new and improved replacement.
> 
> Does this also print the message at the time of creating the image? (And
> isn't qcow the default image format?)

No, I don't think the image is opened by qemu-img create, create is a
different operation. And the default format is raw.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anthony Liguori June 30, 2009, 1:32 p.m. UTC | #4
Kevin Wolf wrote:
> Avi Kivity schrieb:
>   
>> The qcow block driver format is no longer maintained and likely contains
>> serious data corruptors.  Urge users to stay away for it, and advertise
>> the new and improved replacement.
>>
>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>     
>
> vvfat is using qcow internally, so the warning will appear there, too.
> Not that warning against vvfat would be a bad thing, but this error
> message could be confusing.
>
> Maybe we're lucky enough and vvfat survives a s/qcow/qcow2/, but I
> really never wanted to touch that code...
>   

I'm not sure how I feel about this.  Can we prove qcow is broken?  Is it 
only broken for writes and not reads?

If we're printing a warning, does that mean we want to deprecate qcow 
and eventually remove it (or remove write support, at least)?

Regards,

Anthony Liguori

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Wolf June 30, 2009, 1:53 p.m. UTC | #5
Anthony Liguori schrieb:
> Kevin Wolf wrote:
>> Avi Kivity schrieb:
>>   
>>> The qcow block driver format is no longer maintained and likely contains
>>> serious data corruptors.  Urge users to stay away for it, and advertise
>>> the new and improved replacement.
>>>
>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>>     
>> vvfat is using qcow internally, so the warning will appear there, too.
>> Not that warning against vvfat would be a bad thing, but this error
>> message could be confusing.
>>
>> Maybe we're lucky enough and vvfat survives a s/qcow/qcow2/, but I
>> really never wanted to touch that code...
>>   
> 
> I'm not sure how I feel about this.  Can we prove qcow is broken?  Is it 
> only broken for writes and not reads?
> 
> If we're printing a warning, does that mean we want to deprecate qcow 
> and eventually remove it (or remove write support, at least)?

I haven't commented on the intention of deprecating qcow1, I'm not sure
either. But the bug that turned up yesterday was present for a month and
nobody saw it. So I guess we can take that as a sign that qcow isn't
really used that much any more.

On the other hand, I think maintaining qcow1 isn't that hard. It won't
get new features, so we'll not have a whole lot of changes. We must test
it from time to time. And when it comes to fixing, I think qcow1 is much
easier for us than, say, VMDK.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity June 30, 2009, 1:57 p.m. UTC | #6
On 06/30/2009 04:32 PM, Anthony Liguori wrote:
> Kevin Wolf wrote:
>> Avi Kivity schrieb:
>>> The qcow block driver format is no longer maintained and likely 
>>> contains
>>> serious data corruptors.  Urge users to stay away for it, and advertise
>>> the new and improved replacement.
>>>
>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>
>> vvfat is using qcow internally, so the warning will appear there, too.
>> Not that warning against vvfat would be a bad thing, but this error
>> message could be confusing.
>>
>> Maybe we're lucky enough and vvfat survives a s/qcow/qcow2/, but I
>> really never wanted to touch that code...
>
> I'm not sure how I feel about this.  Can we prove qcow is broken?  Is 
> it only broken for writes and not reads?

Well, Kevin posted a patch, so it is.  It's definitely unmaintained.  
Given it's a qemu native format, there is no interoperability value 
except with old qemu versions.

>
> If we're printing a warning, does that mean we want to deprecate qcow 
> and eventually remove it (or remove write support, at least)?

Yes.
Paul Brook June 30, 2009, 2:21 p.m. UTC | #7
> >>> The qcow block driver format is no longer maintained and likely
> >>> contains
> >>> serious data corruptors.  Urge users to stay away for it, and advertise
> >>> the new and improved replacement.
> >
> > I'm not sure how I feel about this.  Can we prove qcow is broken?  Is
> > it only broken for writes and not reads?
>
> Well, Kevin posted a patch, so it is.  It's definitely unmaintained.
> Given it's a qemu native format, there is no interoperability value
> except with old qemu versions.
>
> > If we're printing a warning, does that mean we want to deprecate qcow
> > and eventually remove it (or remove write support, at least)?
>
> Yes.

IMHO there's little value in just printing a warning. Until it actually goes 
away, people are liable to assume we're just being paranoid/awkward and keep 
using it anyway.

I suggest crippling it now and, assuming noone steps up to fix+maintain it, 
ripping out the write support altogether at next release.
I'm assuming the readonly code is in better shape, and can be supported with 
relatively little effort.

Paul
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel P. Berrangé June 30, 2009, 2:40 p.m. UTC | #8
On Tue, Jun 30, 2009 at 03:21:24PM +0100, Paul Brook wrote:
> > >>> The qcow block driver format is no longer maintained and likely
> > >>> contains
> > >>> serious data corruptors.  Urge users to stay away for it, and advertise
> > >>> the new and improved replacement.
> > >
> > > I'm not sure how I feel about this.  Can we prove qcow is broken?  Is
> > > it only broken for writes and not reads?
> >
> > Well, Kevin posted a patch, so it is.  It's definitely unmaintained.
> > Given it's a qemu native format, there is no interoperability value
> > except with old qemu versions.
> >
> > > If we're printing a warning, does that mean we want to deprecate qcow
> > > and eventually remove it (or remove write support, at least)?
> >
> > Yes.
> 
> IMHO there's little value in just printing a warning. Until it actually goes 
> away, people are liable to assume we're just being paranoid/awkward and keep 
> using it anyway.
>
> I suggest crippling it now and, assuming noone steps up to fix+maintain it, 
> ripping out the write support altogether at next release.
> I'm assuming the readonly code is in better shape, and can be supported with 
> relatively little effort.

I agree, if we want to phase it out we should be more discouraging than
just an ignoreable warning

 - Disable it in  qemu, and have code which looks for qcow1 magic
   bytes, prints an error message telling them to use qemu-img
   to convert to qcow2 and exits
 - Keep qcow1 in qemu-img as a source format only, to allow conversions
   to qcow2


Possibly have a 'configure' arg to let people re-enable full read-write
it if they badly need it, but tell them it'll be gone permanently in 
release qemu-0.12.0

Daniel
Anthony Liguori June 30, 2009, 4:42 p.m. UTC | #9
Paul Brook wrote:
> IMHO there's little value in just printing a warning. Until it actually goes 
> away, people are liable to assume we're just being paranoid/awkward and keep 
> using it anyway.
>
> I suggest crippling it now and, assuming noone steps up to fix+maintain it, 
> ripping out the write support altogether at next release.
> I'm assuming the readonly code is in better shape, and can be supported with 
> relatively little effort.
>   

Yes, that's what I was getting at.  We need to support read-only in 
order to allow people to convert to qcow2.

If we're sufficiently concerned that there is data corruption in the 
write path, we should disable it to keep a user from shooting themselves 
in the foot.

Regards,

Anthony Liguori

> Paul
>   

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Färber July 1, 2009, 5:14 p.m. UTC | #10
Am 30.06.2009 um 15:32 schrieb Anthony Liguori:

> Kevin Wolf wrote:
>> Avi Kivity schrieb:
>>
>>> The qcow block driver format is no longer maintained and likely  
>>> contains
>>> serious data corruptors.  Urge users to stay away for it, and  
>>> advertise
>>> the new and improved replacement.
>>>
>>> Signed-off-by: Avi Kivity <avi@redhat.com>
>>
>> vvfat is using qcow internally, so the warning will appear there,  
>> too.
>> Not that warning against vvfat would be a bad thing, but this error
>> message could be confusing.
>>
>> Maybe we're lucky enough and vvfat survives a s/qcow/qcow2/, but I
>> really never wanted to touch that code...
>
> I'm not sure how I feel about this.  Can we prove qcow is broken?   
> Is it only broken for writes and not reads?
>
> If we're printing a warning, does that mean we want to deprecate  
> qcow and eventually remove it (or remove write support, at least)?

I'm confused now. Only recently someone stepped up, saying that qcow2  
was broken and that qcow should be used instead for safety reasons.  
Now all of a sudden, it's the exact opposite, you're even considering  
replacing qcow with qcow2 for vvfat and dropping qcow support.

Andreas

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Wolf July 2, 2009, 7:22 a.m. UTC | #11
Andreas Färber schrieb:
> I'm confused now. Only recently someone stepped up, saying that qcow2  
> was broken and that qcow should be used instead for safety reasons.  
> Now all of a sudden, it's the exact opposite, you're even considering  
> replacing qcow with qcow2 for vvfat and dropping qcow support.

Basically, qcow1 is unmaintained. On the one hand this means that its
code is rarely changed and is quite stable in that respect, on the other
hand it means that nobody cares if it's broken e.g. by changes to the
block layer in general. There is no breakage I know of in the current
stable release, however git master has been broken for a month now.

And I'm pretty sure that qcow2 receives more testing than qcow.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/block/qcow.c b/block/qcow.c
index 55a68a6..2aef6a6 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -101,6 +101,11 @@  static int qcow_open(BlockDriverState *bs, const char *filename, int flags)
         return ret;
     if (bdrv_pread(s->hd, 0, &header, sizeof(header)) != sizeof(header))
         goto fail;
+
+    fprintf(stderr,
+            "WARNING: the qcow file format is no longer supported.\n"
+            "         Please convert your images to qcow2.\n");
+
     be32_to_cpus(&header.magic);
     be32_to_cpus(&header.version);
     be64_to_cpus(&header.backing_file_offset);