diff mbox

[for-2.10,01/16] block: Add PreallocMode to BD.bdrv_truncate()

Message ID 20170313214001.26339-2-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz March 13, 2017, 9:39 p.m. UTC
Add a PreallocMode parameter to the bdrv_truncate() function implemented
by each block driver. Currently, we always pass PREALLOC_MODE_OFF and no
driver accepts anything else.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/block_int.h |  3 ++-
 block.c                   |  2 +-
 block/blkdebug.c          |  9 ++++++++-
 block/crypto.c            |  8 +++++++-
 block/file-posix.c        |  9 ++++++++-
 block/file-win32.c        |  9 ++++++++-
 block/gluster.c           |  6 +++++-
 block/iscsi.c             |  7 ++++++-
 block/nfs.c               |  8 +++++++-
 block/qcow2.c             |  9 ++++++++-
 block/qed.c               |  9 ++++++++-
 block/raw-format.c        |  9 ++++++++-
 block/rbd.c               |  7 ++++++-
 block/sheepdog.c          | 11 +++++++++--
 14 files changed, 91 insertions(+), 15 deletions(-)

Comments

Stefan Hajnoczi March 20, 2017, 10:10 a.m. UTC | #1
On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
> Add a PreallocMode parameter to the bdrv_truncate() function implemented
> by each block driver. Currently, we always pass PREALLOC_MODE_OFF and no
> driver accepts anything else.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  include/block/block_int.h |  3 ++-
>  block.c                   |  2 +-
>  block/blkdebug.c          |  9 ++++++++-
>  block/crypto.c            |  8 +++++++-
>  block/file-posix.c        |  9 ++++++++-
>  block/file-win32.c        |  9 ++++++++-
>  block/gluster.c           |  6 +++++-
>  block/iscsi.c             |  7 ++++++-
>  block/nfs.c               |  8 +++++++-
>  block/qcow2.c             |  9 ++++++++-
>  block/qed.c               |  9 ++++++++-
>  block/raw-format.c        |  9 ++++++++-
>  block/rbd.c               |  7 ++++++-
>  block/sheepdog.c          | 11 +++++++++--
>  14 files changed, 91 insertions(+), 15 deletions(-)

This patch reminds me that some block drivers aren't using Error **errp
yet.  I have added a BiteSizedTasks entry so that this can be fixed
someday.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Stefan Hajnoczi March 20, 2017, 10:18 a.m. UTC | #2
On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
> diff --git a/block/iscsi.c b/block/iscsi.c
> index ab559a6f71..5d6265c4a6 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
>      }
>  }
>  
> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
> +                          PreallocMode prealloc, Error **errp)
>  {
>      IscsiLun *iscsilun = bs->opaque;
>      Error *local_err = NULL;
>  
> +    if (prealloc != PREALLOC_MODE_OFF) {
> +        return -ENOTSUP;
> +    }
> +
>      if (iscsilun->type != TYPE_DISK) {
>          return -ENOTSUP;
>      }

Nevermind what I said about adding a BiteSizedTasks entry:

The missing errp usage is not in qemu.git/master yet.  Please fix up
your bdrv_truncate() errp patch to use errp in all cases, e.g.
error_setg("Unable to truncate non-disk LUN").

stefan
Max Reitz March 20, 2017, 3:07 p.m. UTC | #3
On 20.03.2017 11:18, Stefan Hajnoczi wrote:
> On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index ab559a6f71..5d6265c4a6 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
>>      }
>>  }
>>  
>> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
>> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
>> +                          PreallocMode prealloc, Error **errp)
>>  {
>>      IscsiLun *iscsilun = bs->opaque;
>>      Error *local_err = NULL;
>>  
>> +    if (prealloc != PREALLOC_MODE_OFF) {
>> +        return -ENOTSUP;
>> +    }
>> +
>>      if (iscsilun->type != TYPE_DISK) {
>>          return -ENOTSUP;
>>      }
> 
> Nevermind what I said about adding a BiteSizedTasks entry:
> 
> The missing errp usage is not in qemu.git/master yet.  Please fix up
> your bdrv_truncate() errp patch to use errp in all cases, e.g.
> error_setg("Unable to truncate non-disk LUN").

The thing is that I wasn't comfortable doing that for all block drivers.
I mean, I can take another look but I'd rather have vague error messages
("truncation failed: #{strerror}") than outright wrong ones because I
didn't know what error message to use.

Of course you could argue that this may probably come out during review
but that implies that every submaintainer for every block driver would
actually come out for review...

Max
Stefan Hajnoczi March 22, 2017, 4:28 p.m. UTC | #4
On Mon, Mar 20, 2017 at 04:07:16PM +0100, Max Reitz wrote:
> On 20.03.2017 11:18, Stefan Hajnoczi wrote:
> > On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
> >> diff --git a/block/iscsi.c b/block/iscsi.c
> >> index ab559a6f71..5d6265c4a6 100644
> >> --- a/block/iscsi.c
> >> +++ b/block/iscsi.c
> >> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
> >>      }
> >>  }
> >>  
> >> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
> >> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
> >> +                          PreallocMode prealloc, Error **errp)
> >>  {
> >>      IscsiLun *iscsilun = bs->opaque;
> >>      Error *local_err = NULL;
> >>  
> >> +    if (prealloc != PREALLOC_MODE_OFF) {
> >> +        return -ENOTSUP;
> >> +    }
> >> +
> >>      if (iscsilun->type != TYPE_DISK) {
> >>          return -ENOTSUP;
> >>      }
> > 
> > Nevermind what I said about adding a BiteSizedTasks entry:
> > 
> > The missing errp usage is not in qemu.git/master yet.  Please fix up
> > your bdrv_truncate() errp patch to use errp in all cases, e.g.
> > error_setg("Unable to truncate non-disk LUN").
> 
> The thing is that I wasn't comfortable doing that for all block drivers.
> I mean, I can take another look but I'd rather have vague error messages
> ("truncation failed: #{strerror}") than outright wrong ones because I
> didn't know what error message to use.
> 
> Of course you could argue that this may probably come out during review
> but that implies that every submaintainer for every block driver would
> actually come out for review...

I'm worried about errp being set in only a subset of error cases.

This is likely to cause bugs if callers use if (local_err).  Grepping
through the codebase I can see instances of:

  ret = foo(..., &local_err);
  if (local_err) { /* no ret check! */
      ...
  }

The code would work fine with qcow2 but not iscsi, for example.

IMO we should always set errp, even if the error message is vague
("truncation failed: #{strerror}").

Stefan
Max Reitz March 22, 2017, 4:50 p.m. UTC | #5
On 22.03.2017 17:28, Stefan Hajnoczi wrote:
> On Mon, Mar 20, 2017 at 04:07:16PM +0100, Max Reitz wrote:
>> On 20.03.2017 11:18, Stefan Hajnoczi wrote:
>>> On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
>>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>>> index ab559a6f71..5d6265c4a6 100644
>>>> --- a/block/iscsi.c
>>>> +++ b/block/iscsi.c
>>>> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
>>>>      }
>>>>  }
>>>>  
>>>> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
>>>> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
>>>> +                          PreallocMode prealloc, Error **errp)
>>>>  {
>>>>      IscsiLun *iscsilun = bs->opaque;
>>>>      Error *local_err = NULL;
>>>>  
>>>> +    if (prealloc != PREALLOC_MODE_OFF) {
>>>> +        return -ENOTSUP;
>>>> +    }
>>>> +
>>>>      if (iscsilun->type != TYPE_DISK) {
>>>>          return -ENOTSUP;
>>>>      }
>>>
>>> Nevermind what I said about adding a BiteSizedTasks entry:
>>>
>>> The missing errp usage is not in qemu.git/master yet.  Please fix up
>>> your bdrv_truncate() errp patch to use errp in all cases, e.g.
>>> error_setg("Unable to truncate non-disk LUN").
>>
>> The thing is that I wasn't comfortable doing that for all block drivers.
>> I mean, I can take another look but I'd rather have vague error messages
>> ("truncation failed: #{strerror}") than outright wrong ones because I
>> didn't know what error message to use.
>>
>> Of course you could argue that this may probably come out during review
>> but that implies that every submaintainer for every block driver would
>> actually come out for review...
> 
> I'm worried about errp being set in only a subset of error cases.
> 
> This is likely to cause bugs if callers use if (local_err).  Grepping
> through the codebase I can see instances of:

Yes, but the generic bdrv_truncate() will always set errp if the driver
hasn't done so.

Max

>   ret = foo(..., &local_err);
>   if (local_err) { /* no ret check! */
>       ...
>   }
> 
> The code would work fine with qcow2 but not iscsi, for example.
> 
> IMO we should always set errp, even if the error message is vague
> ("truncation failed: #{strerror}").
> 
> Stefan
>
Stefan Hajnoczi March 23, 2017, 3:32 p.m. UTC | #6
On Wed, Mar 22, 2017 at 05:50:59PM +0100, Max Reitz wrote:
> On 22.03.2017 17:28, Stefan Hajnoczi wrote:
> > On Mon, Mar 20, 2017 at 04:07:16PM +0100, Max Reitz wrote:
> >> On 20.03.2017 11:18, Stefan Hajnoczi wrote:
> >>> On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
> >>>> diff --git a/block/iscsi.c b/block/iscsi.c
> >>>> index ab559a6f71..5d6265c4a6 100644
> >>>> --- a/block/iscsi.c
> >>>> +++ b/block/iscsi.c
> >>>> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
> >>>>      }
> >>>>  }
> >>>>  
> >>>> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
> >>>> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
> >>>> +                          PreallocMode prealloc, Error **errp)
> >>>>  {
> >>>>      IscsiLun *iscsilun = bs->opaque;
> >>>>      Error *local_err = NULL;
> >>>>  
> >>>> +    if (prealloc != PREALLOC_MODE_OFF) {
> >>>> +        return -ENOTSUP;
> >>>> +    }
> >>>> +
> >>>>      if (iscsilun->type != TYPE_DISK) {
> >>>>          return -ENOTSUP;
> >>>>      }
> >>>
> >>> Nevermind what I said about adding a BiteSizedTasks entry:
> >>>
> >>> The missing errp usage is not in qemu.git/master yet.  Please fix up
> >>> your bdrv_truncate() errp patch to use errp in all cases, e.g.
> >>> error_setg("Unable to truncate non-disk LUN").
> >>
> >> The thing is that I wasn't comfortable doing that for all block drivers.
> >> I mean, I can take another look but I'd rather have vague error messages
> >> ("truncation failed: #{strerror}") than outright wrong ones because I
> >> didn't know what error message to use.
> >>
> >> Of course you could argue that this may probably come out during review
> >> but that implies that every submaintainer for every block driver would
> >> actually come out for review...
> > 
> > I'm worried about errp being set in only a subset of error cases.
> > 
> > This is likely to cause bugs if callers use if (local_err).  Grepping
> > through the codebase I can see instances of:
> 
> Yes, but the generic bdrv_truncate() will always set errp if the driver
> hasn't done so.

I prefer consistently setting errp to make patch review easy (no
checking all callers to see how they behave), making it safe to
copy-paste the code elsewhere, etc.

It's safer to be consistent, can produce more detailed error messages,
and the cost of writing error_setg() calls is low.  But it's a matter of
style, you've pointed out the code is technically correct right now.

Stefan
Max Reitz March 27, 2017, 2:19 p.m. UTC | #7
On 23.03.2017 16:32, Stefan Hajnoczi wrote:
> On Wed, Mar 22, 2017 at 05:50:59PM +0100, Max Reitz wrote:
>> On 22.03.2017 17:28, Stefan Hajnoczi wrote:
>>> On Mon, Mar 20, 2017 at 04:07:16PM +0100, Max Reitz wrote:
>>>> On 20.03.2017 11:18, Stefan Hajnoczi wrote:
>>>>> On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
>>>>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>>>>> index ab559a6f71..5d6265c4a6 100644
>>>>>> --- a/block/iscsi.c
>>>>>> +++ b/block/iscsi.c
>>>>>> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
>>>>>>      }
>>>>>>  }
>>>>>>  
>>>>>> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
>>>>>> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
>>>>>> +                          PreallocMode prealloc, Error **errp)
>>>>>>  {
>>>>>>      IscsiLun *iscsilun = bs->opaque;
>>>>>>      Error *local_err = NULL;
>>>>>>  
>>>>>> +    if (prealloc != PREALLOC_MODE_OFF) {
>>>>>> +        return -ENOTSUP;
>>>>>> +    }
>>>>>> +
>>>>>>      if (iscsilun->type != TYPE_DISK) {
>>>>>>          return -ENOTSUP;
>>>>>>      }
>>>>>
>>>>> Nevermind what I said about adding a BiteSizedTasks entry:
>>>>>
>>>>> The missing errp usage is not in qemu.git/master yet.  Please fix up
>>>>> your bdrv_truncate() errp patch to use errp in all cases, e.g.
>>>>> error_setg("Unable to truncate non-disk LUN").
>>>>
>>>> The thing is that I wasn't comfortable doing that for all block drivers.
>>>> I mean, I can take another look but I'd rather have vague error messages
>>>> ("truncation failed: #{strerror}") than outright wrong ones because I
>>>> didn't know what error message to use.
>>>>
>>>> Of course you could argue that this may probably come out during review
>>>> but that implies that every submaintainer for every block driver would
>>>> actually come out for review...
>>>
>>> I'm worried about errp being set in only a subset of error cases.
>>>
>>> This is likely to cause bugs if callers use if (local_err).  Grepping
>>> through the codebase I can see instances of:
>>
>> Yes, but the generic bdrv_truncate() will always set errp if the driver
>> hasn't done so.
> 
> I prefer consistently setting errp to make patch review easy (no
> checking all callers to see how they behave), making it safe to
> copy-paste the code elsewhere, etc.

The thing is, there is only a single caller for all of the
BlockDriver.bdrv_truncate() functions and that is the generic
bdrv_truncate() function.

I mean, of course I can copy-paste my generic error message from the
global bdrv_truncate() function into every driver where I'm not sure
what error to emit, but I find that a bit... Well, it looks bad when you
just look at the code without the history behind it.

But since you insist, I'll do so gladly. :-)

Max

> It's safer to be consistent, can produce more detailed error messages,
> and the cost of writing error_setg() calls is low.  But it's a matter of
> style, you've pointed out the code is technically correct right now.
> 
> Stefan
>
diff mbox

Patch

diff --git a/include/block/block_int.h b/include/block/block_int.h
index ee55d5e9cd..2615356581 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -196,7 +196,8 @@  struct BlockDriver {
     int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
 
     const char *protocol_name;
-    int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset, Error **errp);
+    int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset,
+                         PreallocMode prealloc, Error **errp);
 
     int64_t (*bdrv_getlength)(BlockDriverState *bs);
     bool has_variable_length;
diff --git a/block.c b/block.c
index 267c761ff7..57ae40a84e 100644
--- a/block.c
+++ b/block.c
@@ -3237,7 +3237,7 @@  int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp)
         return -EACCES;
     }
 
-    ret = drv->bdrv_truncate(bs, offset, errp);
+    ret = drv->bdrv_truncate(bs, offset, PREALLOC_MODE_OFF, errp);
     if (ret == 0) {
         ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
         bdrv_dirty_bitmap_truncate(bs);
diff --git a/block/blkdebug.c b/block/blkdebug.c
index c795ae9e72..31a71a34d3 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -661,8 +661,15 @@  static int64_t blkdebug_getlength(BlockDriverState *bs)
     return bdrv_getlength(bs->file->bs);
 }
 
-static int blkdebug_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int blkdebug_truncate(BlockDriverState *bs, int64_t offset,
+                             PreallocMode prealloc, Error **errp)
 {
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Unsupported preallocation mode '%s'",
+                   PreallocMode_lookup[prealloc]);
+        return -ENOTSUP;
+    }
+
     return bdrv_truncate(bs->file, offset, errp);
 }
 
diff --git a/block/crypto.c b/block/crypto.c
index 17b3140998..fa61aef380 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -382,12 +382,18 @@  static int block_crypto_create_generic(QCryptoBlockFormat format,
 }
 
 static int block_crypto_truncate(BlockDriverState *bs, int64_t offset,
-                                 Error **errp)
+                                 PreallocMode prealloc, Error **errp)
 {
     BlockCrypto *crypto = bs->opaque;
     size_t payload_offset =
         qcrypto_block_get_payload_offset(crypto->block);
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Unsupported preallocation mode '%s'",
+                   PreallocMode_lookup[prealloc]);
+        return -ENOTSUP;
+    }
+
     offset += payload_offset;
 
     return bdrv_truncate(bs->file, offset, errp);
diff --git a/block/file-posix.c b/block/file-posix.c
index 3a402151f1..dde8c101c8 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1384,12 +1384,19 @@  static void raw_close(BlockDriverState *bs)
     }
 }
 
-static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int raw_truncate(BlockDriverState *bs, int64_t offset,
+                        PreallocMode prealloc, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     struct stat st;
     int ret;
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Unsupported preallocation mode '%s'",
+                   PreallocMode_lookup[prealloc]);
+        return -ENOTSUP;
+    }
+
     if (fstat(s->fd, &st)) {
         ret = -errno;
         error_setg_errno(errp, -ret, "Failed to fstat() the file");
diff --git a/block/file-win32.c b/block/file-win32.c
index 3f3925623f..bf6f20d3b0 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -461,12 +461,19 @@  static void raw_close(BlockDriverState *bs)
     }
 }
 
-static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int raw_truncate(BlockDriverState *bs, int64_t offset,
+                        PreallocMode prealloc, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
     LONG low, high;
     DWORD dwPtrLow;
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Unsupported preallocation mode '%s'",
+                   PreallocMode_lookup[prealloc]);
+        return -ENOTSUP;
+    }
+
     low = offset;
     high = offset >> 32;
 
diff --git a/block/gluster.c b/block/gluster.c
index 00b8240562..2d4c278500 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1085,11 +1085,15 @@  static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
 }
 
 static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset,
-                                 Error **errp)
+                                 PreallocMode prealloc, Error **errp)
 {
     int ret;
     BDRVGlusterState *s = bs->opaque;
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        return -ENOTSUP;
+    }
+
     ret = glfs_ftruncate(s->fd, offset);
     if (ret < 0) {
         return -errno;
diff --git a/block/iscsi.c b/block/iscsi.c
index ab559a6f71..5d6265c4a6 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2060,11 +2060,16 @@  static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
     }
 }
 
-static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
+                          PreallocMode prealloc, Error **errp)
 {
     IscsiLun *iscsilun = bs->opaque;
     Error *local_err = NULL;
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        return -ENOTSUP;
+    }
+
     if (iscsilun->type != TYPE_DISK) {
         return -ENOTSUP;
     }
diff --git a/block/nfs.c b/block/nfs.c
index 57d12efc51..a44c526b80 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -757,9 +757,15 @@  static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
     return (task.ret < 0 ? task.ret : st.st_blocks * 512);
 }
 
-static int nfs_file_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int nfs_file_truncate(BlockDriverState *bs, int64_t offset,
+                             PreallocMode prealloc, Error **errp)
 {
     NFSClient *client = bs->opaque;
+
+    if (prealloc != PREALLOC_MODE_OFF) {
+        return -ENOTSUP;
+    }
+
     return nfs_ftruncate(client->context, client->fh, offset);
 }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 53b0bd61a7..8d180bbf9d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2524,12 +2524,19 @@  static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs,
     return ret;
 }
 
-static int qcow2_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
+                          PreallocMode prealloc, Error **errp)
 {
     BDRVQcow2State *s = bs->opaque;
     int64_t new_l1_size;
     int ret;
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Unsupported preallocation mode '%s'",
+                   PreallocMode_lookup[prealloc]);
+        return -ENOTSUP;
+    }
+
     if (offset & 511) {
         error_setg(errp, "The new size must be a multiple of 512");
         return -EINVAL;
diff --git a/block/qed.c b/block/qed.c
index eb346d645b..b5102f5d7f 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1518,12 +1518,19 @@  static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
     return cb.ret;
 }
 
-static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset,
+                             PreallocMode prealloc, Error **errp)
 {
     BDRVQEDState *s = bs->opaque;
     uint64_t old_image_size;
     int ret;
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Unsupported preallocation mode '%s'",
+                   PreallocMode_lookup[prealloc]);
+        return -ENOTSUP;
+    }
+
     if (!qed_is_image_size_valid(offset, s->header.cluster_size,
                                  s->header.table_size)) {
         error_setg(errp, "Invalid image size specified");
diff --git a/block/raw-format.c b/block/raw-format.c
index 36e65036f0..aeaa13e3f5 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -327,10 +327,17 @@  static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
     }
 }
 
-static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int raw_truncate(BlockDriverState *bs, int64_t offset,
+                        PreallocMode prealloc, Error **errp)
 {
     BDRVRawState *s = bs->opaque;
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Unsupported preallocation mode '%s'",
+                   PreallocMode_lookup[prealloc]);
+        return -ENOTSUP;
+    }
+
     if (s->has_size) {
         error_setg(errp, "Cannot resize fixed-size raw disks");
         return -ENOTSUP;
diff --git a/block/rbd.c b/block/rbd.c
index f7d4dd93fe..fd31f8626e 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1028,11 +1028,16 @@  static int64_t qemu_rbd_getlength(BlockDriverState *bs)
     return info.size;
 }
 
-static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset,
+                             PreallocMode prealloc, Error **errp)
 {
     BDRVRBDState *s = bs->opaque;
     int r;
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        return -ENOTSUP;
+    }
+
     r = rbd_resize(s->image, offset);
     if (r < 0) {
         return r;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 27126ea474..546708adaf 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2133,13 +2133,20 @@  static int64_t sd_getlength(BlockDriverState *bs)
     return s->inode.vdi_size;
 }
 
-static int sd_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int sd_truncate(BlockDriverState *bs, int64_t offset,
+                       PreallocMode prealloc, Error **errp)
 {
     BDRVSheepdogState *s = bs->opaque;
     int ret, fd;
     unsigned int datalen;
     uint64_t max_vdi_size;
 
+    if (prealloc != PREALLOC_MODE_OFF) {
+        error_setg(errp, "Unsupported preallocation mode '%s'",
+                   PreallocMode_lookup[prealloc]);
+        return -ENOTSUP;
+    }
+
     max_vdi_size = (UINT64_C(1) << s->inode.block_size_shift) * MAX_DATA_OBJS;
     if (offset < s->inode.vdi_size) {
         error_setg(errp, "shrinking is not supported");
@@ -2428,7 +2435,7 @@  static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
     BDRVSheepdogState *s = bs->opaque;
 
     if (offset > s->inode.vdi_size) {
-        ret = sd_truncate(bs, offset, NULL);
+        ret = sd_truncate(bs, offset, PREALLOC_MODE_OFF, NULL);
         if (ret < 0) {
             return ret;
         }