[Libguestfs] Re: [PATCH nbdkit 3/3] vddk: do_extents: Avoid reading partial chunk beyond the end of the disk
On Mon, Jan 06, 2025 at 10:30:01AM -0600, Eric Blake wrote:
On Mon, Jan 06, 2025 at 04:12:08PM +0000, Richard W.M. Jones wrote:
> + capacity = info->capacity; /* in sectors! */
> + VDDK_CALL_START (VixDiskLib_FreeInfo, "info")
> + VixDiskLib_FreeInfo (info);
> + VDDK_CALL_END (VixDiskLib_FreeInfo, 0);
Is this code we should call once when we first .open the device (and
stash it in the vddk_handle), rather than querying every time .extents
is called?
Yes, we could stash it as we have already called it (when opening the
disk). I believe the overhead of calling into VDDK again is
negligible in this case, certainly compared to QueryAllocatedBlocks
which is very heavyweight. Nevertheless I'll add a comment.
> + /* If 'end' spanned beyond the last chunk of the
disk, then we
> + * reduced it above to avoid reading a chunk that extends beyond the
> + * end of the underlying disk. We have to synthesize an allocated
> + * block here, which is what VDDK's example code does
> + * (doc/samples/diskLib/vixDiskLibSample.cpp: DoGetAllocatedBlocks).
> + */
> + if (end < offset + count) {
> + if (add_extent (extents, &position, offset + count, false) == -1)
> + return -1;
> + }
...and then synthesizing a final extent for everything beyond end,
whether or not you were asked to provide that much information. But
that is safe - we intentionally designed add_extent() to take as much
or as little as you want to provide, and synthesizing the allocated
chunk always is easier than deciding whether or not you reduced end.
I think it's slightly different. We'll only call add_extent here if
end < offset + count, which should only happen in the case where we
reduced 'end' because we detected the special case of a disk capacity
which is not aligned to the chunk size.
In all cases, it'll return what the user requested (offset + count),
never more or less.
However ... looking again at the code, I'm not sure this handles the
req_one case properly. I'm going to make another small change first.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW