Libguestfs - Feb 2020 - Re: [RFC] lib: allow to specify physical/logical block size for disks

пн, 10 лют. 2020 о 10:53 Richard W.M. Jones <rjones@redhat.com>
пише:
>
> On Sat, Feb 08, 2020 at 01:25:28AM +0200, Mykola Ivanets wrote:
> > From: Nikolay Ivanets <stenavin@gmail.com>
> >
> > I faced with situation where libguestfs cannot recognize partitions on
a
> > disk image which was partitioned on a system with "4K
native" sector
> > size support.
> >
> > In order to fix the issue we need to allow users to specify desired
> > physical and/or logical block size per drive basis.
> >
> > It is definitely not a complete patch but rather a way to request for
a
> > comments.  Nevertheless it is already working patch.  I've added
an
> > optional parameters to add_drive API method which allow specifying
> > physical and logical block size for a drive separetely.
> >
> > There are no documentation and tests yet. Input parameters are not
> > validated for correctness.
>
> The patch is generally fine, but ...
>
> > Here are my questions:
> > - Am I move in the right direction adding support for physical/logical
> > block size?
>
> I'm fairly sure we only need one of these.  I'm just not
> sure which one we need.  I think we need to ask an expert
> or look at the qemu / kernel code to find out exactly what
> each setting really does.
Here are what they do stand for:
physical_block_size: The physical block size the disk will report to
the guest OS. For Linux this would be the value returned by the
BLKPBSZGET ioctl and describes the disk's hardware sector size which
can be relevant for the alignment of disk data. We don't have an API
to get this one.

logical_block_size: The logical block size the disk will report to the
guest OS. For Linux this would be the value returned by the BLKSSZGET
ioctl and describes the smallest units for disk I/O. We have
blockdev-getsz API to get this value.

How do they use. If your HDD has physical block size = 4096 you might
want make I/O request equals to (or multiple) this value. Otherwise
you might hit performance penalty. I think, the same is valid for
virtual disk image which is located on physical storage with 4K
physical sector size.
> > - I'm not sure I've made a good choise for parameter names:
'pblocksize'
> > and 'lblocksize' respectively.  I've tried to avoid long
names like
> > 'physicalblocksize' while keeping readability and semantic.
>
> If we only have one, we can use "blocksize".  But it does
> require us to answer the previous one.
Here are possible combinations of [pl]blocksize in real world:

physical | logical
-----------------------
4096      | 4096
4096      | 512
512        | 512

Having both values equals works except for case #2 which will not
impact on functionality, only performance might be hit.
If we want to simplify our API - we might expose the only parameter
called 'blocksize' and set physical_block_size and logical_block_size
to the same value.

On the other hand, reading this values from libvirt XML we will take
logical_block_size and attach disk to libguestfs with
physical_block_size == logical_block_size.
> > - Do we want to add the same optional parameters to
'add_drive_scratch'
> > API method?  I think it would be nice but it is up to you.
>
> It should also be added to add_domain and add_libvirt_dom (note all
> the APIs which have 'discard' and 'copyonread' already).
Yeah, already did that.
> It could be added to add_drive_scratch, I guess.  However it doesn't
> seem very useful for scratch drives (why create a scratch drive with
> 4K sectors which will be thrown away in the end?)
For testing purpose and API consistency, or again if you have 4K
backing storage.
> > - What about 'add_dom', 'add_libvirt_dom' API methods?
Should they also
> > handle 'blokio' tag in domain XML and act respectively?
>
> Yes.
Yeah, already did that.
> > - Anything else I didn't spot yet?
> > - Do we want guestfish to accept physical/logical block size per drive
> > from command line?
>
> If you can be bothered, but best to put it in a second patch.
OK.
> > - What about other virt tools like virt-df, virt-cat and so on?
>
> If you change the command line handling, then it should apply to other
> tools mostly automatically.  Have a look at how the --format option
> works.
Yeah, I spot that already.

--
  Mykola Ivanets

On Mon, Feb 10, 2020 at 02:22:01PM +0200, Nikolay Ivanets
wrote:
> Here are what they do stand for:
> physical_block_size: The physical block size the disk will report to
> the guest OS. For Linux this would be the value returned by the
> BLKPBSZGET ioctl and describes the disk's hardware sector size which
> can be relevant for the alignment of disk data. We don't have an API
> to get this one.
> 
> logical_block_size: The logical block size the disk will report to the
> guest OS. For Linux this would be the value returned by the BLKSSZGET
> ioctl and describes the smallest units for disk I/O. We have
> blockdev-getsz API to get this value.
Interestingly parted uses BLKSSZGET (logical_block_size), but does not
use BLKPBSZGET (physical_block_size), so I guess that fits with my
observations.
> How do they use. If your HDD has physical block size = 4096 you might
> want make I/O request equals to (or multiple) this value. Otherwise
> you might hit performance penalty. I think, the same is valid for
> virtual disk image which is located on physical storage with 4K
> physical sector size.
...
> Here are possible combinations of [pl]blocksize in real world:
> 
> physical | logical
> -----------------------
> 4096      | 4096
> 4096      | 512
> 512        | 512
> 
> Having both values equals works except for case #2 which will not
> impact on functionality, only performance might be hit.
> If we want to simplify our API - we might expose the only parameter
> called 'blocksize' and set physical_block_size and
logical_block_size
> to the same value.
Right, but libguestfs rarely deals with real physical disks.  I think
even if eg. you attach libguestfs to /dev/sdX there's still going to
be a host kernel in between doing its thing.  So I'm not clear that
the 4K physical / 512b logical case is actually something that
libguestfs cares about.  (But OTOH I'm also not clear why qemu cares
about this case either.)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

пн, 10 лют. 2020 о 14:59 Richard W.M. Jones <rjones@redhat.com>
пише:
>
> On Mon, Feb 10, 2020 at 02:22:01PM +0200, Nikolay Ivanets wrote:
> > Here are what they do stand for:
> > physical_block_size: The physical block size the disk will report to
> > the guest OS. For Linux this would be the value returned by the
> > BLKPBSZGET ioctl and describes the disk's hardware sector size
which
> > can be relevant for the alignment of disk data. We don't have an
API
> > to get this one.
> >
> > logical_block_size: The logical block size the disk will report to the
> > guest OS. For Linux this would be the value returned by the BLKSSZGET
> > ioctl and describes the smallest units for disk I/O. We have
> > blockdev-getsz API to get this value.
>
> Interestingly parted uses BLKSSZGET (logical_block_size), but does not
> use BLKPBSZGET (physical_block_size), so I guess that fits with my
> observations.
>
> > How do they use. If your HDD has physical block size = 4096 you might
> > want make I/O request equals to (or multiple) this value. Otherwise
> > you might hit performance penalty. I think, the same is valid for
> > virtual disk image which is located on physical storage with 4K
> > physical sector size.
> ...
> > Here are possible combinations of [pl]blocksize in real world:
> >
> > physical | logical
> > -----------------------
> > 4096      | 4096
> > 4096      | 512
> > 512        | 512
> >
> > Having both values equals works except for case #2 which will not
> > impact on functionality, only performance might be hit.
> > If we want to simplify our API - we might expose the only parameter
> > called 'blocksize' and set physical_block_size and
logical_block_size
> > to the same value.
>
> Right, but libguestfs rarely deals with real physical disks.  I think
> even if eg. you attach libguestfs to /dev/sdX there's still going to
> be a host kernel in between doing its thing.  So I'm not clear that
> the 4K physical / 512b logical case is actually something that
> libguestfs cares about.  (But OTOH I'm also not clear why qemu cares
> about this case either.)
I think because QEMU emulates real hardware. And on real hardware you
can get different physical/logical block size. So QEMU allows you to create
such a configuration.

--
  Mykola Ivanets