On Tue, Oct 31, 2023 at 03:03:50PM -0400, Peter Xu wrote:
> On Wed, Oct 25, 2023 at 11:07:33AM -0300, Fabiano Rosas wrote:
> > >> +static int parse_ramblock_fixed_ram(QEMUFile *f, RAMBlock *block,
> > >> ram_addr_t length)
> > >> +{
> > >> + g_autofree unsigned long *bitmap = NULL;
> > >> + struct FixedRamHeader header;
> > >> + size_t bitmap_size;
> > >> + long num_pages;
> > >> + int ret = 0;
> > >> +
> > >> + ret = fixed_ram_read_header(f, &header);
> > >> + if (ret < 0) {
> > >> + error_report("Error reading fixed-ram header");
> > >> + return -EINVAL;
> > >> + }
> > >> +
> > >> + block->pages_offset = header.pages_offset;
> > >
> > > Do you think it is worth sanity checking that 'pages_offset' is aligned
> > > in some way.
> > >
> > > It is nice that we have flexibility to change the alignment in future
> > > if we find 1 MB is not optimal, so I wouldn't want to force 1MB align
> > > check htere. Perhaps we could at least sanity check for alignment at
> > > TARGET_PAGE_SIZE, to detect a gross data corruption problem ?
> > >
> >
> > I don't see why not. I'll add it.
>
> Is there any explanation on why that 1MB offset, and how the number is
> chosen? Thanks,
The fixed-ram format is anticipating the use of O_DIRECT.
With O_DIRECT both the buffers in memory, and the file handle offset
have alignment requirements. The buffer alignments are usually page
sized, and QEMU RAM blocks will trivially satisfy those.
The file handle offset alignment varies per filesystem. While you can
query the alignment for the FS holding the file with statx(), that is
not appropriate todo. If a user saves/restores QEMU state to file, we
must assume there is a chance the user will copy the saved state to a
different filesystem.
IOW, we want alignment to satisfy the likely worst case.
Picking 1 MB is a nice round number that is large enough that it is
almost certainly going to satisfy any filesystem alignment. In fact
it is likely massive overkill. None the less 1 MB is also still tiny
in the context of guest RAM sizes, so no one is going to notice the
padding holes in the file from this.
IOW, the 1 MB choice is an arbitrary, but somewhat informed choice.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
