[PATCH] block/nvme: Do not allow image creation with NVMe block driver

[PATCH] block/nvme: Do not allow image creation with NVMe block driver

[Philippe Mathieu-Daudé]

The NVMe driver does not support image creation.
The full drive has to be passed to the guest.

Before:

  $ qemu-img create -f raw nvme://0000:04:00.0/1 20G
  Formatting 'nvme://0000:04:00.0/1', fmt=raw size=21474836480

  $ qemu-img info nvme://0000:04:00.0/1
  image: nvme://0000:04:00.0/1
  file format: raw
  virtual size: 349 GiB (375083606016 bytes)
  disk size: unavailable

After:

  $ qemu-img create -f raw nvme://0000:04:00.0/1 20G
  qemu-img: nvme://0000:04:00.0/1: Protocol driver 'nvme' does not support image creation

Fixes: 5a5e7f8cd86 ("block: trickle down the fallback image creation function use to the block drivers")
Reported-by: Xueqiang Wei <xuwei@redhat.com>
Suggested-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Cc: Maxim Levitsky <mlevitsk@redhat.com>
---
 block/nvme.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index a06a188d530..73ddf837c2b 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1515,9 +1515,6 @@ static BlockDriver bdrv_nvme = {
     .protocol_name            = "nvme",
     .instance_size            = sizeof(BDRVNVMeState),
 
-    .bdrv_co_create_opts      = bdrv_co_create_opts_simple,
-    .create_opts              = &bdrv_create_opts_simple,
-
     .bdrv_parse_filename      = nvme_parse_filename,
     .bdrv_file_open           = nvme_file_open,
     .bdrv_close               = nvme_close,
-- 
2.26.2

Re: [PATCH] block/nvme: Do not allow image creation with NVMe block driver

[Philippe Mathieu-Daudé]

On 12/4/20 5:57 PM, Philippe Mathieu-Daudé wrote:
> The NVMe driver does not support image creation.
> The full drive has to be passed to the guest.
> 
> Before:
> 
>   $ qemu-img create -f raw nvme://0000:04:00.0/1 20G
>   Formatting 'nvme://0000:04:00.0/1', fmt=raw size=21474836480
> 
>   $ qemu-img info nvme://0000:04:00.0/1
>   image: nvme://0000:04:00.0/1
>   file format: raw
>   virtual size: 349 GiB (375083606016 bytes)
>   disk size: unavailable
> 
> After:
> 
>   $ qemu-img create -f raw nvme://0000:04:00.0/1 20G
>   qemu-img: nvme://0000:04:00.0/1: Protocol driver 'nvme' does not support image creation
> 
> Fixes: 5a5e7f8cd86 ("block: trickle down the fallback image creation function use to the block drivers")
> Reported-by: Xueqiang Wei <xuwei@redhat.com>
> Suggested-by: Max Reitz <mreitz@redhat.com>

Well Max didn't suggest the change but pointed me to commit 5a5e7f8cd86.

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Cc: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  block/nvme.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index a06a188d530..73ddf837c2b 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -1515,9 +1515,6 @@ static BlockDriver bdrv_nvme = {
>      .protocol_name            = "nvme",
>      .instance_size            = sizeof(BDRVNVMeState),
>  
> -    .bdrv_co_create_opts      = bdrv_co_create_opts_simple,
> -    .create_opts              = &bdrv_create_opts_simple,
> -
>      .bdrv_parse_filename      = nvme_parse_filename,
>      .bdrv_file_open           = nvme_file_open,
>      .bdrv_close               = nvme_close,
> 

Re: [PATCH] block/nvme: Do not allow image creation with NVMe block driver

[Philippe Mathieu-Daudé]

On 12/4/20 11:28 PM, Philippe Mathieu-Daudé wrote:
> On 12/4/20 5:57 PM, Philippe Mathieu-Daudé wrote:
>> The NVMe driver does not support image creation.
>> The full drive has to be passed to the guest.
>>
>> Before:
>>
>>   $ qemu-img create -f raw nvme://0000:04:00.0/1 20G
>>   Formatting 'nvme://0000:04:00.0/1', fmt=raw size=21474836480
>>
>>   $ qemu-img info nvme://0000:04:00.0/1
>>   image: nvme://0000:04:00.0/1
>>   file format: raw
>>   virtual size: 349 GiB (375083606016 bytes)
>>   disk size: unavailable

Maybe I should not forbid all formats... But 'raw' is kinda
dangerous, as there is no way to enforce the next layer to
access beside the size allocated.

Safe drive partitioning can be achieved creating namespaces,
feature which is not yet implemented.

>>
>> After:
>>
>>   $ qemu-img create -f raw nvme://0000:04:00.0/1 20G
>>   qemu-img: nvme://0000:04:00.0/1: Protocol driver 'nvme' does not support image creation
>>
>> Fixes: 5a5e7f8cd86 ("block: trickle down the fallback image creation function use to the block drivers")
>> Reported-by: Xueqiang Wei <xuwei@redhat.com>
>> Suggested-by: Max Reitz <mreitz@redhat.com>
> 
> Well Max didn't suggest the change but pointed me to commit 5a5e7f8cd86.
> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> Cc: Maxim Levitsky <mlevitsk@redhat.com>
>> ---
>>  block/nvme.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/block/nvme.c b/block/nvme.c
>> index a06a188d530..73ddf837c2b 100644
>> --- a/block/nvme.c
>> +++ b/block/nvme.c
>> @@ -1515,9 +1515,6 @@ static BlockDriver bdrv_nvme = {
>>      .protocol_name            = "nvme",
>>      .instance_size            = sizeof(BDRVNVMeState),
>>  
>> -    .bdrv_co_create_opts      = bdrv_co_create_opts_simple,
>> -    .create_opts              = &bdrv_create_opts_simple,
>> -
>>      .bdrv_parse_filename      = nvme_parse_filename,
>>      .bdrv_file_open           = nvme_file_open,
>>      .bdrv_close               = nvme_close,
>>
> 

Re: [PATCH] block/nvme: Do not allow image creation with NVMe block driver

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

On Mon, Dec 07, 2020 at 06:16:04PM +0100, Philippe Mathieu-Daudé wrote:
> On 12/4/20 11:28 PM, Philippe Mathieu-Daudé wrote:
> > On 12/4/20 5:57 PM, Philippe Mathieu-Daudé wrote:
> >> The NVMe driver does not support image creation.
> >> The full drive has to be passed to the guest.
> >>
> >> Before:
> >>
> >>   $ qemu-img create -f raw nvme://0000:04:00.0/1 20G
> >>   Formatting 'nvme://0000:04:00.0/1', fmt=raw size=21474836480
> >>
> >>   $ qemu-img info nvme://0000:04:00.0/1
> >>   image: nvme://0000:04:00.0/1
> >>   file format: raw
> >>   virtual size: 349 GiB (375083606016 bytes)
> >>   disk size: unavailable
> 
> Maybe I should not forbid all formats... But 'raw' is kinda
> dangerous, as there is no way to enforce the next layer to
> access beside the size allocated.
> 
> Safe drive partitioning can be achieved creating namespaces,
> feature which is not yet implemented.

I don't see the need for this patch. Or if there is a need then
block/file-posix.c, block/iscsi.c, and block/nbd.c should also be
changed (anything that uses bdrv_co_create_opts_simple()).

Instead I suggest adding a warning at creation time if a raw format
image is created on top of a BDS that is larger than requested. The
warning should remind the user that they need to use the raw format
drivers's size= open option to restrict the disk capacity when opening
the image.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] block/nvme: Do not allow image creation with NVMe block driver

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 1572 bytes --]

Am 17.12.2020 um 17:17 hat Stefan Hajnoczi geschrieben:
> On Mon, Dec 07, 2020 at 06:16:04PM +0100, Philippe Mathieu-Daudé wrote:
> > On 12/4/20 11:28 PM, Philippe Mathieu-Daudé wrote:
> > > On 12/4/20 5:57 PM, Philippe Mathieu-Daudé wrote:
> > >> The NVMe driver does not support image creation.
> > >> The full drive has to be passed to the guest.
> > >>
> > >> Before:
> > >>
> > >>   $ qemu-img create -f raw nvme://0000:04:00.0/1 20G
> > >>   Formatting 'nvme://0000:04:00.0/1', fmt=raw size=21474836480
> > >>
> > >>   $ qemu-img info nvme://0000:04:00.0/1
> > >>   image: nvme://0000:04:00.0/1
> > >>   file format: raw
> > >>   virtual size: 349 GiB (375083606016 bytes)
> > >>   disk size: unavailable
> > 
> > Maybe I should not forbid all formats... But 'raw' is kinda
> > dangerous, as there is no way to enforce the next layer to
> > access beside the size allocated.
> > 
> > Safe drive partitioning can be achieved creating namespaces,
> > feature which is not yet implemented.
> 
> I don't see the need for this patch. Or if there is a need then
> block/file-posix.c, block/iscsi.c, and block/nbd.c should also be
> changed (anything that uses bdrv_co_create_opts_simple()).
> 
> Instead I suggest adding a warning at creation time if a raw format
> image is created on top of a BDS that is larger than requested. The
> warning should remind the user that they need to use the raw format
> drivers's size= open option to restrict the disk capacity when opening
> the image.

This sounds like a good idea to me.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]