| Message ID | 1365788268-24787-1-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
Le Friday 12 Apr 2013 à 19:37:48 (+0200), Stefan Hajnoczi a écrit : > Image file compression works at cluster granularity. It is not possible > to compress less than a cluster of data at a time. > > Print an error when attempting qemu-img convert -c with an input file > that is not a multiple of the cluster size. > > I considered automatically adjusting the output file size but think it's > better to be explicit. This avoids confusion when users notice that > image file size changed after conversion. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > qemu-img.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/qemu-img.c b/qemu-img.c > index 31627b0..2273851 100644 > --- a/qemu-img.c > +++ b/qemu-img.c > @@ -1370,6 +1370,13 @@ static int img_convert(int argc, char **argv) > goto out; > } > cluster_sectors = cluster_size >> 9; > + if (total_sectors % cluster_sectors) { > + error_report("compression requires that input file size is a " > + "multiple of %d bytes", > + cluster_size); > + ret = -1; > + goto out; > + } > sector_num = 0; > > nb_sectors = total_sectors; > -- > 1.8.1.4 > > Reviewed-by: Benoit Canet <benoit@irqsave.net>
On 04/12/2013 01:55 PM, Benoît Canet wrote: > Le Friday 12 Apr 2013 à 19:37:48 (+0200), Stefan Hajnoczi a écrit : >> Image file compression works at cluster granularity. It is not possible >> to compress less than a cluster of data at a time. Shouldn't it be possible to treat a file that is not rounded to a cluster boundary as though the rest of the final cluster is all NUL bytes, and compress that? As long as you know the original size, this operation is reversible, instead of having to error out just because the original wasn't a complete multiple. >> I considered automatically adjusting the output file size but think it's >> better to be explicit. This avoids confusion when users notice that >> image file size changed after conversion. I agree that changing the final image size on a round trip through compression is not appropriate, but does that mean we have to reject the file completely if it wasn't sized to a multiple to begin with?
On Fri, Apr 12, 2013 at 02:04:41PM -0600, Eric Blake wrote: > On 04/12/2013 01:55 PM, Benoît Canet wrote: > > Le Friday 12 Apr 2013 à 19:37:48 (+0200), Stefan Hajnoczi a écrit : > >> Image file compression works at cluster granularity. It is not possible > >> to compress less than a cluster of data at a time. > > Shouldn't it be possible to treat a file that is not rounded to a > cluster boundary as though the rest of the final cluster is all NUL > bytes, and compress that? As long as you know the original size, this > operation is reversible, instead of having to error out just because the > original wasn't a complete multiple. > > >> I considered automatically adjusting the output file size but think it's > >> better to be explicit. This avoids confusion when users notice that > >> image file size changed after conversion. > > I agree that changing the final image size on a round trip through > compression is not appropriate, but does that mean we have to reject the > file completely if it wasn't sized to a multiple to begin with? I will send a new patch that does this. kwolf also suggested it on Friday and it sounds like a good idea. Stefan
diff --git a/qemu-img.c b/qemu-img.c index 31627b0..2273851 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1370,6 +1370,13 @@ static int img_convert(int argc, char **argv) goto out; } cluster_sectors = cluster_size >> 9; + if (total_sectors % cluster_sectors) { + error_report("compression requires that input file size is a " + "multiple of %d bytes", + cluster_size); + ret = -1; + goto out; + } sector_num = 0; nb_sectors = total_sectors;
Image file compression works at cluster granularity. It is not possible to compress less than a cluster of data at a time. Print an error when attempting qemu-img convert -c with an input file that is not a multiple of the cluster size. I considered automatically adjusting the output file size but think it's better to be explicit. This avoids confusion when users notice that image file size changed after conversion. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- qemu-img.c | 7 +++++++ 1 file changed, 7 insertions(+)