[Qemu-block] [PATCH 1/3] blockjob: add manual-cull property

2017-10-02 Thread John Snow
Add a "manual cull" property to block jobs that forces them to linger
in the block job list (visible to QMP queries) until the user
explicitly dismisses them via QMP.

The cull command itself is implemented in the next commit, and the
feature is exposed to drive-backup and blockdev-backup in the subsequent
commit.

Signed-off-by: John Snow 
---
 block/backup.c   | 20 +--
 block/commit.c   |  2 +-
 block/mirror.c   |  2 +-
 block/replication.c  |  5 +++--
 block/stream.c   |  2 +-
 blockdev.c   |  8 
 blockjob.c   | 46 ++--
 include/block/block_int.h|  8 +---
 include/block/blockjob.h | 21 
 include/block/blockjob_int.h |  2 +-
 qapi/block-core.json |  7 ---
 tests/test-blockjob-txn.c|  2 +-
 tests/test-blockjob.c|  2 +-
 13 files changed, 97 insertions(+), 30 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 517c300..0b00908 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -532,15 +532,15 @@ static const BlockJobDriver backup_job_driver = {
 .drain  = backup_drain,
 };
 
-BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
-  BlockDriverState *target, int64_t speed,
-  MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
-  bool compress,
-  BlockdevOnError on_source_error,
-  BlockdevOnError on_target_error,
-  int creation_flags,
-  BlockCompletionFunc *cb, void *opaque,
-  BlockJobTxn *txn, Error **errp)
+BlockJob *backup_job_create(const char *job_id, bool manual_cull,
+BlockDriverState *bs, BlockDriverState *target,
+int64_t speed, MirrorSyncMode sync_mode,
+BdrvDirtyBitmap *sync_bitmap, bool compress,
+BlockdevOnError on_source_error,
+BlockdevOnError on_target_error,
+int creation_flags,
+BlockCompletionFunc *cb, void *opaque,
+BlockJobTxn *txn, Error **errp)
 {
 int64_t len;
 BlockDriverInfo bdi;
@@ -608,7 +608,7 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 }
 
 /* job->common.len is fixed, so we can't allow resize */
-job = block_job_create(job_id, _job_driver, bs,
+job = block_job_create(job_id, _job_driver, manual_cull, bs,
BLK_PERM_CONSISTENT_READ,
BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE |
BLK_PERM_WRITE_UNCHANGED | BLK_PERM_GRAPH_MOD,
diff --git a/block/commit.c b/block/commit.c
index 8f0e835..308a5fd 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -304,7 +304,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
 return;
 }
 
-s = block_job_create(job_id, _job_driver, bs, 0, BLK_PERM_ALL,
+s = block_job_create(job_id, _job_driver, false, bs, 0, 
BLK_PERM_ALL,
  speed, BLOCK_JOB_DEFAULT, NULL, NULL, errp);
 if (!s) {
 return;
diff --git a/block/mirror.c b/block/mirror.c
index 6f5cb9f..013e73a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1180,7 +1180,7 @@ static void mirror_start_job(const char *job_id, 
BlockDriverState *bs,
 }
 
 /* Make sure that the source is not resized while the job is running */
-s = block_job_create(job_id, driver, mirror_top_bs,
+s = block_job_create(job_id, driver, false, mirror_top_bs,
  BLK_PERM_CONSISTENT_READ,
  BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
  BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed,
diff --git a/block/replication.c b/block/replication.c
index 3a4e682..6c59f00 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -539,8 +539,9 @@ static void replication_start(ReplicationState *rs, 
ReplicationMode mode,
 bdrv_op_block_all(top_bs, s->blocker);
 bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);
 
-job = backup_job_create(NULL, s->secondary_disk->bs, 
s->hidden_disk->bs,
-0, MIRROR_SYNC_MODE_NONE, NULL, false,
+job = backup_job_create(NULL, false, s->secondary_disk->bs,
+s->hidden_disk->bs, 0, MIRROR_SYNC_MODE_NONE,
+NULL, false,
 BLOCKDEV_ON_ERROR_REPORT,
 BLOCKDEV_ON_ERROR_REPORT, BLOCK_JOB_INTERNAL,
 backup_job_completed, bs, NULL, _err);
diff --git a/block/stream.c b/block/stream.c
index e6f7234..c644f34 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -244,7 +244,7 

Re: [Qemu-block] [Qemu-devel] [PATCH v4 23/23] qemu-io: Relax 'alloc' now that block-status doesn't assert

2017-10-02 Thread John Snow


On 10/02/2017 07:56 PM, Eric Blake wrote:
> On 10/02/2017 04:27 PM, John Snow wrote:
>>
>>
>> On 09/13/2017 12:03 PM, Eric Blake wrote:
>>> Previously, the alloc command required that input parameters be
>>> sector-aligned and clamped to 32 bits, because the underlying
>>> bdrv_is_allocated used a 32-bit parameter and asserted aligned
>>> inputs.  But now that we have fixed block status to report a
>>> 64-bit bytes value, and to properly round requests on behalf of
>>> guests, we can pass any values, and can use qemu-io to add
>>> coverage that our rounding is correct regardless of the guest
>>> alignment constraints.
>>>
>>> Update iotest 177 to intentionally probe block status at
>>> unaligned boundaries as well as with a bytes value that does not
>>> map to 32-bit sectors, which also required tweaking the image
>>> prep to leave an unallocated portion to the image under test.
>>>
>>> Signed-off-by: Eric Blake 
>>>
> 
>>>  echo
>>> +echo "== block status smaller than alignment =="
>>> +limits=align=4k
>>> +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \
>>> +-c "alloc 1 1" -c "alloc 0x6d0 1000" -c "alloc 127m 5P" \
>>> +-c map | _filter_qemu_io
>>> +
> 
>>
>> aaand I'll hold off on this one until the respin so I don't have to
>> review the test twice.
> 
> Fair enough; thanks for the reviews.
> 
> By the way, it's operations like the above additions where you can step
> through bdrv_co_block_status in gdb to see all the rounding/alignment
> steps in action, so I do feel pretty confident that my changes in 21/23
> were fairly well covered.
> 

I do actually trust you, but I wasn't able to *quickly* convince myself,
so I held up on the r-b. I didn't spot any problems either, to be fair...!

>>
>> I'll say I'm done for v4 for now :)
> 
> I'll try to get v5 posted in the next day or two.
> 

No rush on my end ...

--js



[Qemu-block] [PATCH 3/3] blockjob: expose manual-cull property

2017-10-02 Thread John Snow
For drive-backup and blockdev-backup, expose the manual-cull
property, having it default to false. There are no universal
creation parameters, so it must be added to each job type that
it makes sense for individually.

Signed-off-by: John Snow 
---
 blockdev.c   | 10 --
 qapi/block-core.json | 21 -
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index ee07bca..ba2ebfb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3198,6 +3198,9 @@ static BlockJob *do_drive_backup(DriveBackup *backup, 
BlockJobTxn *txn,
 if (!backup->has_job_id) {
 backup->job_id = NULL;
 }
+if (!backup->has_manual_cull) {
+backup->manual_cull = false;
+}
 if (!backup->has_compress) {
 backup->compress = false;
 }
@@ -3290,7 +3293,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, 
BlockJobTxn *txn,
 }
 }
 
-job = backup_job_create(backup->job_id, false, bs, target_bs,
+job = backup_job_create(backup->job_id, backup->manual_cull, bs, target_bs,
 backup->speed, backup->sync, bmap, 
backup->compress,
 backup->on_source_error, backup->on_target_error,
 BLOCK_JOB_DEFAULT, NULL, NULL, txn, _err);
@@ -3341,6 +3344,9 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, 
BlockJobTxn *txn,
 if (!backup->has_job_id) {
 backup->job_id = NULL;
 }
+if (!backup->has_manual_cull) {
+backup->manual_cull = false;
+}
 if (!backup->has_compress) {
 backup->compress = false;
 }
@@ -3369,7 +3375,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, 
BlockJobTxn *txn,
 goto out;
 }
 }
-job = backup_job_create(backup->job_id, false, bs, target_bs,
+job = backup_job_create(backup->job_id, backup->manual_cull, bs, target_bs,
 backup->speed, backup->sync, NULL, 
backup->compress,
 backup->on_source_error, backup->on_target_error,
 BLOCK_JOB_DEFAULT, NULL, NULL, txn, _err);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index de322d1..c646743 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1104,6 +1104,11 @@
 # @job-id: identifier for the newly-created block job. If
 #  omitted, the device name will be used. (Since 2.7)
 #
+# @manual-cull: Whether or not the job created by this command needs to be
+#   cleaned up manually via block-job-cull or not. The default is
+#   false. When true, the job will remain in a "completed" state
+#   until culled manually with block-job-cull. (Since 2.11)
+#
 # @device: the device name or node-name of a root node which should be copied.
 #
 # @target: the target of the new image. If the file exists, or if it
@@ -1144,9 +1149,10 @@
 # Since: 1.6
 ##
 { 'struct': 'DriveBackup',
-  'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
-'*format': 'str', 'sync': 'MirrorSyncMode', '*mode': 
'NewImageMode',
-'*speed': 'int', '*bitmap': 'str', '*compress': 'bool',
+  'data': { '*job-id': 'str', '*manual-cull': 'bool', 'device': 'str',
+'target': 'str', '*format': 'str', 'sync': 'MirrorSyncMode',
+'*mode': 'NewImageMode', '*speed': 'int', '*bitmap': 'str',
+'*compress': 'bool',
 '*on-source-error': 'BlockdevOnError',
 '*on-target-error': 'BlockdevOnError' } }
 
@@ -1156,6 +1162,11 @@
 # @job-id: identifier for the newly-created block job. If
 #  omitted, the device name will be used. (Since 2.7)
 #
+# @manual-cull: Whether or not the job created by this command needs to be
+#   cleaned up manually via block-job-cull or not. The default is
+#   false. When true, the job will remain in a "completed" state
+#   until culled manually with block-job-cull. (Since 2.11)
+#
 # @device: the device name or node-name of a root node which should be copied.
 #
 # @target: the device name or node-name of the backup target node.
@@ -1185,8 +1196,8 @@
 # Since: 2.3
 ##
 { 'struct': 'BlockdevBackup',
-  'data': { '*job-id': 'str', 'device': 'str', 'target': 'str',
-'sync': 'MirrorSyncMode',
+  'data': { '*job-id': 'str', '*manual-cull': 'bool', 'device': 'str',
+'target': 'str', 'sync': 'MirrorSyncMode',
 '*speed': 'int',
 '*compress': 'bool',
 '*on-source-error': 'BlockdevOnError',
-- 
2.9.5




[Qemu-block] [PATCH 0/3] blockjobs: add explicit job culling

2017-10-02 Thread John Snow
For jobs that complete when a monitor isn't looking, there's no way to
tell what the job's final return code was. We need to allow jobs to
remain in the list until queried for reliable management.

This is an RFC; tests are on the way.
(Tested only manually via qmp-shell for now.)

John Snow (3):
  blockjob: add manual-cull property
  qmp: add block-job-cull command
  blockjob: expose manual-cull property

 block/backup.c   | 20 +-
 block/commit.c   |  2 +-
 block/mirror.c   |  2 +-
 block/replication.c  |  5 +++--
 block/stream.c   |  2 +-
 block/trace-events   |  1 +
 blockdev.c   | 28 +
 blockjob.c   | 46 +++--
 include/block/block_int.h|  8 +---
 include/block/blockjob.h | 21 +++
 include/block/blockjob_int.h |  2 +-
 qapi/block-core.json | 49 
 tests/test-blockjob-txn.c|  2 +-
 tests/test-blockjob.c|  2 +-
 14 files changed, 155 insertions(+), 35 deletions(-)

-- 
2.9.5




[Qemu-block] [PATCH 2/3] qmp: add block-job-cull command

2017-10-02 Thread John Snow
For jobs that have finished (either completed or canceled), allow the
user to dismiss the job's status reports via block-job-cull.

Signed-off-by: John Snow 
---
 block/trace-events   |  1 +
 blockdev.c   | 14 ++
 qapi/block-core.json | 21 +
 3 files changed, 36 insertions(+)

diff --git a/block/trace-events b/block/trace-events
index 25dd5a3..cb64e91 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -46,6 +46,7 @@ qmp_block_job_cancel(void *job) "job %p"
 qmp_block_job_pause(void *job) "job %p"
 qmp_block_job_resume(void *job) "job %p"
 qmp_block_job_complete(void *job) "job %p"
+qmp_block_job_cull(void *job) "job %p"
 qmp_block_stream(void *bs, void *job) "bs %p job %p"
 
 # block/file-win32.c
diff --git a/blockdev.c b/blockdev.c
index eeb4986..ee07bca 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3766,6 +3766,20 @@ void qmp_block_job_complete(const char *device, Error 
**errp)
 aio_context_release(aio_context);
 }
 
+void qmp_block_job_cull(const char *device, Error **errp)
+{
+AioContext *aio_context;
+BlockJob *job = find_block_job(device, _context, errp);
+
+if (!job) {
+return;
+}
+
+trace_qmp_block_job_cull(job);
+block_job_cull(, errp);
+aio_context_release(aio_context);
+}
+
 void qmp_change_backing_file(const char *device,
  const char *image_node_name,
  const char *backing_file,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a4f5e10..de322d1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2161,6 +2161,27 @@
 { 'command': 'block-job-complete', 'data': { 'device': 'str' } }
 
 ##
+# @block-job-cull:
+#
+# For jobs that have already completed, remove them from the block-job-query
+# list. This command only needs to be run for jobs which were started with the
+# manual-cull=true option.
+#
+# This command will refuse to operate on any job that has not yet reached
+# its terminal state. "cancel" or "complete" will still need to be used as
+# appropriate.
+#
+# @device: The job identifier. This used to be a device name (hence
+#  the name of the parameter), but since QEMU 2.7 it can have
+#  other values.
+#
+# Returns: Nothing on success
+#
+# Since: 2.11
+##
+{ 'command': 'block-job-cull', 'data': { 'device': 'str' } }
+
+##
 # @BlockdevDiscardOptions:
 #
 # Determines how to handle discard requests.
-- 
2.9.5




[Qemu-block] xen-pci-passthrough PCI Express support? (Re: [Qemu-devel] [Xen-devel] [PATCH v2 4/5] pci: Add INTERFACE_CONVENTIONAL_PCI_DEVICE to Conventional PCI devices)

2017-10-02 Thread Eduardo Habkost
On Thu, Sep 28, 2017 at 10:12:34AM -0300, Eduardo Habkost wrote:
> On Thu, Sep 28, 2017 at 02:33:57AM -0600, Jan Beulich wrote:
> > >>> On 27.09.17 at 21:56,  wrote:
> > > --- a/hw/xen/xen_pt.c
> > > +++ b/hw/xen/xen_pt.c
> > > @@ -964,6 +964,10 @@ static const TypeInfo xen_pci_passthrough_info = {
> > >  .instance_size = sizeof(XenPCIPassthroughState),
> > >  .instance_finalize = xen_pci_passthrough_finalize,
> > >  .class_init = xen_pci_passthrough_class_init,
> > > +.interfaces = (InterfaceInfo[]) {
> > > +{ INTERFACE_CONVENTIONAL_PCI_DEVICE },
> > > +{ },
> > > +},
> > >  };
> > 
> > Passed through devices can be both PCI and PCIe, so following
> > the description of the patch I don't think these can be statically
> > given either property. Granted quite a bit of PCIe specific
> > functionality may be missing in the Xen code ...
> 
> This is just static data about what the device type supports, not
> about what a given device instance really is.  Deciding if the
> device is PCIe or Conventional at runtime is out of the scope of
> this series.
> 
> That said, if passed through PCI Express devices are really
> supported, it looks like this should be marked as hybrid.

Can anybody confirm if PCI Express devices are really supported
by xen-pci-passthrough?

I suggest we add only INTERFACE_CONVENTIONAL_PCI_DEVICE to the
class info until we confirm that.

(In other words, apply this patch as-is, and add
INTERFACE_PCIE_DEVICE later as a follow-up patch if appropriate.)

-- 
Eduardo



Re: [Qemu-block] [Qemu-devel] [PATCH v4 23/23] qemu-io: Relax 'alloc' now that block-status doesn't assert

2017-10-02 Thread Eric Blake
On 10/02/2017 04:27 PM, John Snow wrote:
> 
> 
> On 09/13/2017 12:03 PM, Eric Blake wrote:
>> Previously, the alloc command required that input parameters be
>> sector-aligned and clamped to 32 bits, because the underlying
>> bdrv_is_allocated used a 32-bit parameter and asserted aligned
>> inputs.  But now that we have fixed block status to report a
>> 64-bit bytes value, and to properly round requests on behalf of
>> guests, we can pass any values, and can use qemu-io to add
>> coverage that our rounding is correct regardless of the guest
>> alignment constraints.
>>
>> Update iotest 177 to intentionally probe block status at
>> unaligned boundaries as well as with a bytes value that does not
>> map to 32-bit sectors, which also required tweaking the image
>> prep to leave an unallocated portion to the image under test.
>>
>> Signed-off-by: Eric Blake 
>>

>>  echo
>> +echo "== block status smaller than alignment =="
>> +limits=align=4k
>> +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \
>> + -c "alloc 1 1" -c "alloc 0x6d0 1000" -c "alloc 127m 5P" \
>> + -c map | _filter_qemu_io
>> +

> 
> aaand I'll hold off on this one until the respin so I don't have to
> review the test twice.

Fair enough; thanks for the reviews.

By the way, it's operations like the above additions where you can step
through bdrv_co_block_status in gdb to see all the rounding/alignment
steps in action, so I do feel pretty confident that my changes in 21/23
were fairly well covered.

> 
> I'll say I'm done for v4 for now :)

I'll try to get v5 posted in the next day or two.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-block] [Qemu-devel] [PATCH v4 21/23] block: Align block status requests

2017-10-02 Thread Eric Blake
On 10/02/2017 03:24 PM, John Snow wrote:
> 
> 
> On 09/13/2017 12:03 PM, Eric Blake wrote:
>> Any device that has request_alignment greater than 512 should be
>> unable to report status at a finer granularity; it may also be
>> simpler for such devices to be guaranteed that the block layer
>> has rounded things out to the granularity boundary (the way the
>> block layer already rounds all other I/O out).  Besides, getting
>> the code correct for super-sector alignment also benefits us
>> for the fact that our public interface now has byte granularity,
>> even though none of our drivers have byte-level callbacks.
>>
>> Add an assertion in blkdebug that proves that the block layer
>> never requests status of unaligned sections, similar to what it
>> does on other requests (while still keeping the generic helper
>> in place for when future patches add a throttle driver).  Note
>> that iotest 177 already covers this (it would fail if you use
>> just the blkdebug.c hunk without the io.c changes).  Meanwhile,
>> we can drop assertions in callers that no longer have to pass
>> in sector-aligned addresses.
>>
>> There is a mid-function scope added for 'int count', for a
>> couple of reasons: first, an upcoming patch will add an 'if'
>> statement that checks whether a driver has an old- or new-style
>> callback, and can conveniently use the same scope for less
>> indentation churn at that time.  Second, since we are trying
>> to get rid of sector-based computations, wrapping things in
>> a scope makes it easier to group and see what will be deleted
>> in a final cleanup patch once all drivers have been converted
>> to the new-style callback.
>>
>> Signed-off-by: Eric Blake 
>>

>> @@ -1815,28 +1816,45 @@ static int64_t coroutine_fn 
>> bdrv_co_block_status(BlockDriverState *bs,
>>  }
>>
>>  bdrv_inc_in_flight(bs);
>> -/*
>> - * TODO: Rather than require aligned offsets, we could instead
>> - * round to the driver's request_alignment here, then touch up
>> - * count afterwards back to the caller's expectations.
>> - */
>> -assert(QEMU_IS_ALIGNED(offset | bytes, BDRV_SECTOR_SIZE));
>> -bytes = MIN(bytes, BDRV_REQUEST_MAX_BYTES);
>> -ret = bs->drv->bdrv_co_get_block_status(bs, offset >> BDRV_SECTOR_BITS,
>> -bytes >> BDRV_SECTOR_BITS, 
>> ,
>> -_file);
>> -if (ret < 0) {
>> -*pnum = 0;
>> -goto out;
>> +
>> +/* Round out to request_alignment boundaries */
>> +align = MAX(bs->bl.request_alignment, BDRV_SECTOR_SIZE);
> 
> There's something funny to me about an alignment request getting itself
> aligned...

Pre-patch, we are asserting that all callers are passing in
sector-aligned requests (even though we've switched the interface to
allow byte-based granularity, none of the callers are yet taking
advantage of that), then passing on sector-aligned requests to the
driver (regardless of whether the driver has 1-byte alignment, like
posix-file, or is using 4k alignment, like some block devices).
Post-patch, we are going with the larger of the driver's preferred
alignment, and our minimum of 512 (mainly because until series 4, we
have no way to pass byte values on to the driver, even if the driver
otherwise supports smaller alignments).  This MAX() disappears later in
series 4, once the driver callback is made byte-based, first by becoming
conditional on whether the driver is old sector-based or new byte-based
callback:
 https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03814.html
and then altogether when the sector-based is deleted:
 https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03833.html

So, this patch, coupled with the new driver callback in series 4, is
what lets us introduce clients that are able to finally pass in values
that are not sector-aligned; and the rounding up of alignment here is a
stop-gap measure to keep things working until the transition is complete.

> 
>> +aligned_offset = QEMU_ALIGN_DOWN(offset, align);
>> +aligned_bytes = ROUND_UP(offset + bytes, align) - aligned_offset;
>> +
>> +{
>> +int count; /* sectors */
>> +
>> +assert(QEMU_IS_ALIGNED(aligned_offset | aligned_bytes,
>> +   BDRV_SECTOR_SIZE));
>> +ret = bs->drv->bdrv_co_get_block_status(
>> +bs, aligned_offset >> BDRV_SECTOR_BITS,
>> +MIN(INT_MAX, aligned_bytes) >> BDRV_SECTOR_BITS, ,
> 
> I guess under the belief that INT_MAX will be strictly less than
> BDRV_REQUEST_MAX_BYTES, or some other reason I'm missing for the change?

INT_MAX is larger than BDRV_REQUEST_MAX_BYTES.  aligned_bytes, however,
may be larger than BDRV_REQUEST_MAX_BYTES (or even larger than INT_MAX).
 Once series 4 introduces the driver callback with a 64-bit length, then
we can pass in aligned_bytes as-is; but until then, while we are stuck
with a 32-bit sector length callback, we are