qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Alberto Garcia <berto@igalia.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Nir Soffer <nsoffer@redhat.com>,
	qemu-block@nongnu.org
Subject: Re: [PATCH v2 1/4] qcow2: Require that the virtual size is a multiple of the sector size
Date: Tue, 14 Jan 2020 15:03:31 +0100	[thread overview]
Message-ID: <70f3996c-e62d-c560-b7a3-e0653f38493d@redhat.com> (raw)
In-Reply-To: <w51sgkib0n3.fsf@maestria.local.igalia.com>


[-- Attachment #1.1: Type: text/plain, Size: 1760 bytes --]

On 14.01.20 14:58, Alberto Garcia wrote:
> On Tue 14 Jan 2020 02:47:53 PM CET, Max Reitz wrote:
> 
>>> Yes, but does it make sense to try to support images with unaligned
>>> sizes if no one is going to create them ever and QEMU cannot even
>>> generate them?
>>
>> If nobody uses such images ever, isn’t the current code fine as-is?
> 
> I'll rephrase:
> 
> 1. It is possible to have a qcow2 image with a size of 1 GB + 13 bytes
>    (the size field in the qcow2 header allows it).
> 
> 2. qemu-img cannot currently produce such images.
> 
> 3. QEMU can open them but it gets the size wrong.

Yes, but in a safe way.  The user simply doesn’t get access to those 13
bytes.

With your proposed change, it would just reject the image altogether.
As a user, I’d prefer not being able to access 13 bytes to not being
able to access 1 GB + 13 bytes.

Furthermore, the user couldn’t even fix the image then because they
couldn’t resize it with qemu-img resize.

> 4. Fixing this in QEMU involves non-trivial changes (at least at the
>    moment).
> 
> 5. I don't think there's any use case for such images so instead of
>    fixing QEMU I propose that we simply refuse to open them.

If there’s no use case, I don’t see how the current behavior is problematic.

The arguments I could immediately agree to are (1) simplicity, and (2)
the image is probably corrupted.

As for (1), this patch adds more lines than it removes, so from a pure
quantitative standpoint, it doesn’t make the code simpler.

As for (2), I’m always of the opinion we shouldn’t refuse to read
corrupted images just because they are corrupted.  We should open them
and let the user read as much as is reasonably possible.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2020-01-14 14:06 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-09 19:12 [PATCH v2 0/4] qcow2: Misc BDRV_SECTOR_SIZE updates Alberto Garcia
2020-01-09 19:12 ` [PATCH v2 1/4] qcow2: Require that the virtual size is a multiple of the sector size Alberto Garcia
2020-01-14 13:01   ` Max Reitz
2020-01-14 13:20     ` Alberto Garcia
2020-01-14 13:47       ` Max Reitz
2020-01-14 13:58         ` Alberto Garcia
2020-01-14 14:03           ` Max Reitz [this message]
2020-01-16 14:24             ` Alberto Garcia
2020-01-09 19:13 ` [PATCH v2 2/4] qcow2: Don't round the L1 table allocation up to " Alberto Garcia
2020-01-14 13:46   ` Max Reitz
2020-01-09 19:13 ` [PATCH v2 3/4] qcow2: Tighten cluster_offset alignment assertions Alberto Garcia
2020-01-10 12:14   ` Alberto Garcia
2020-01-14 13:58     ` Max Reitz
2020-01-14 13:56   ` Max Reitz
2020-01-09 19:13 ` [PATCH v2 4/4] qcow2: Use BDRV_SECTOR_SIZE instead of the hardcoded value Alberto Garcia
2020-01-14 14:15   ` Max Reitz
2020-01-16 23:26     ` Alberto Garcia
2020-01-17  9:12       ` Max Reitz
2020-01-17  9:55         ` Kevin Wolf
2020-01-17 11:01           ` Max Reitz
2020-01-17 11:51             ` Kevin Wolf
2020-01-17 14:34               ` Alberto Garcia
2020-01-18 18:07     ` Alberto Garcia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=70f3996c-e62d-c560-b7a3-e0653f38493d@redhat.com \
    --to=mreitz@redhat.com \
    --cc=berto@igalia.com \
    --cc=kwolf@redhat.com \
    --cc=nsoffer@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).