On Tue, Feb 11, 2020 at 01:36:16AM +0200, Mykola Ivanets wrote: > +=item C<blocksize> > + > +The integer parameter C<blocksize> allows specifying both physical and > logical > +block size the disk will report to the libguestfs appliance. > + > +Physical block size would be the value returned by the C<BLKPBSZGET> ioctl > and > +describes the disk's hardware sector size which can be relevant for the > +alignment of disk data. > + > +Logical block size would be the value returned by the C<BLKSSZGET> ioctl and > +describes the smallest units for disk I/O. (C<guestfs_blockdev_getss> API > call > +will return this value). > + > +Possible values for C<blocksize> parameter are 0, 512 and 4096. F<0> is a > +special value which instructs libguestfs to do nothing special and it is up > to > +the current backend what block size to expose (usually 512 bytes). > + > +Only a subset of the backends support this parameter (currently only the > +libvirt and direct backends do). > + > +The default value is 0.
This is kind of wordy and yet manages not to explain what the parameter is actually useful for. I would forget about explaining physical vs logical block size and the intricacies of ioctl, and instead write this below. Note I've also got rid of the special "0" case, because it's actually blocksize-not-defined, which is different from 0. =item C<blocksize> This parameter sets the sector size of the disk. Possible values are C<512> (the default if the parameter is omitted) or C<4096>. Use C<4096> when handling an "Advanced Format" disk that uses 4K sector size (L<https://en.wikipedia.org/wiki/Advanced_Format>). =back > +$TEST_FUNCTIONS > +skip_if_skipped > + > +# Test valid values > +for expected_bs in 0 512 4096; do > + actual_bs=$(guestfish --ro add /dev/null blocksize:${expected_bs} : run > : blockdev-getss /dev/sda) No, this is wrong. In the expected_bs == 0 case it must omit the blocksize: parameter entirely. The easiest thing is to omit the 0 case entirely here, because you're already testing the no parameter case below. > + if [ ${expected_bs} -eq 0 ]; then > + expected_bs=512 > + fi > + > + if [ "${actual_bs}" != "${expected_bs}" ]; then > + echo "$0: error: actual blocksize doesn't match expected: > ${actual_bs} != ${expected_bs}" > + exit 1 > + fi > +done > + > +# Test without blocksize parameter > +expected_bs=512 > +actual_bs=$(guestfish --ro add /dev/null : run : blockdev-getss /dev/sda) > + > +if [ "${actual_bs}" != "${expected_bs}" ]; then > + echo "$0: error: actual blocksize doesn't match expected: ${actual_bs} > != ${expected_bs}" > + exit 1 > +fi Rest of the patch looks fine to me. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
