[Top][All Lists]

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

Re: [Qemu-devel] [PATCH v7 3/6] qcow2: Reduce REFT_OFFSET_MASK

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v7 3/6] qcow2: Reduce REFT_OFFSET_MASK
Date: Fri, 29 Jun 2018 10:22:22 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0

On 06/29/2018 03:44 AM, Kevin Wolf wrote:
Am 28.06.2018 um 21:07 hat Eric Blake geschrieben:
Match our code to the spec change in the previous patch - there's
no reason for the refcount table to allow larger offsets than the
L1/L2 tables.

What about internal snapshots? And anyway, because of the metadata
overhead, the physical image size of a fully allocated image is always
going to be at least minimally larger than the virtual disk size.

I'm not necessarily opposed to making the change if there is a good
reason to make it, but I don't see a real need for it and the
justification used here and also in the previous patch is incorrect.

The fact that ext4 cannot hold an image this large is already an indication that setting this limit on the refcount table is NOT going to bite real users.

Yes, you can argue that with lots of metadata, including internal snapshots, and on a capable file system (such as tmpfs) that can even hold files this large to begin with, then yes, allowing the refcount to exceed this limit will allow slightly more metadata to be crammed into a single image. But will it actually help anyone?

Do I need to respin the series to split patch 2 into the obvious changes (stuff unrelated to capping refcount size) vs. the controversial stuff (refcount cap and this code change)?


In practice, no image has more than 64PB of
allocated clusters anyways, as anything beyond that can't be
expressed via L2 mappings to host offsets.

If you're opposed to an exact 56-bit limit on the grounds that 56-bit guest data plus minimal metadata should still be expressable, would it be better to cap refcount at 57-bits?

Suggested-by: Alberto Garcia <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>

+++ b/block/qcow2.h
@@ -439,7 +439,7 @@ typedef enum QCow2MetadataOverlap {
  #define L2E_OFFSET_MASK 0x00fffffffffffe00ULL
  #define L2E_COMPRESSED_OFFSET_SIZE_MASK 0x3fffffffffffffffULL

-#define REFT_OFFSET_MASK 0xfffffffffffffe00ULL
+#define REFT_OFFSET_MASK 0x00fffffffffffe00ULL

  static inline int64_t start_of_cluster(BDRVQcow2State *s, int64_t offset)

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]