[PATCH] Document qemu-img options data_file and data_file_raw

[PATCH] Document qemu-img options data_file and data_file_raw

[Connor Kuehl]

The contents of this patch were initially developed and posted by Han
Han[1], however, it appears the original patch was not applied. Since
then, the relevant documentation has been moved and adapted to a new
format.

I've taken most of the original wording and tweaked it according to
some of the feedback from the original patch submission. I've also
adapted it to restructured text, which is the format the documentation
currently uses.

[1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html

Reported-by: Han Han <hhan@redhat.com>
Co-developed-by: Han Han <hhan@redhat.com>
Fixes: https://bugzilla.redhat.com/1763105
Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
---
 docs/tools/qemu-img.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index b615aa8419..5cc585dc27 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -866,6 +866,18 @@ Supported image file formats:
     issue ``lsattr filename`` to check if the NOCOW flag is set or not
     (Capital 'C' is NOCOW flag).
 
+  ``data_file``
+    Pathname that refers to a file that will store all guest data. If
+    this option is used, the qcow2 file will only contain the image's
+    metadata.
+
+  ``data_file_raw``
+    If this option is set to ``on``, QEMU will always keep the external
+    data file consistent as a standalone read-only raw image. The default
+    value is ``off``.
+
+    This option can only be enabled if ``data_file`` is set.
+
 ``Other``
 
   QEMU also supports various other image file formats for
-- 
2.29.2

Re: [PATCH] Document qemu-img options data_file and data_file_raw

[Eric Blake]

On 3/1/21 11:28 AM, Connor Kuehl wrote:
> The contents of this patch were initially developed and posted by Han
> Han[1], however, it appears the original patch was not applied. Since
> then, the relevant documentation has been moved and adapted to a new
> format.
> 
> I've taken most of the original wording and tweaked it according to
> some of the feedback from the original patch submission. I've also
> adapted it to restructured text, which is the format the documentation
> currently uses.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html
> 
> Reported-by: Han Han <hhan@redhat.com>
> Co-developed-by: Han Han <hhan@redhat.com>

Unusual tag, and his original email has Signed-off-by.  It's easier to
just reuse S-o-b here.

> Fixes: https://bugzilla.redhat.com/1763105
> Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
> ---
>  docs/tools/qemu-img.rst | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
> index b615aa8419..5cc585dc27 100644
> --- a/docs/tools/qemu-img.rst
> +++ b/docs/tools/qemu-img.rst
> @@ -866,6 +866,18 @@ Supported image file formats:
>      issue ``lsattr filename`` to check if the NOCOW flag is set or not
>      (Capital 'C' is NOCOW flag).
>  
> +  ``data_file``
> +    Pathname that refers to a file that will store all guest data. If
> +    this option is used, the qcow2 file will only contain the image's
> +    metadata.
> +
> +  ``data_file_raw``
> +    If this option is set to ``on``, QEMU will always keep the external
> +    data file consistent as a standalone read-only raw image. The default
> +    value is ``off``.
> +
> +    This option can only be enabled if ``data_file`` is set.
> +
>  ``Other``
>  
>    QEMU also supports various other image file formats for
> 

Otherwise,
Reviewed-by: Eric Blake <eblake@redhat.com>

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

Re: [PATCH] Document qemu-img options data_file and data_file_raw

[Stefan Hajnoczi]

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

On Mon, Mar 01, 2021 at 11:28:37AM -0600, Connor Kuehl wrote:
> The contents of this patch were initially developed and posted by Han
> Han[1], however, it appears the original patch was not applied. Since
> then, the relevant documentation has been moved and adapted to a new
> format.
> 
> I've taken most of the original wording and tweaked it according to
> some of the feedback from the original patch submission. I've also
> adapted it to restructured text, which is the format the documentation
> currently uses.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html
> 
> Reported-by: Han Han <hhan@redhat.com>
> Co-developed-by: Han Han <hhan@redhat.com>
> Fixes: https://bugzilla.redhat.com/1763105
> Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
> ---
>  docs/tools/qemu-img.rst | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [PATCH] Document qemu-img options data_file and data_file_raw

[Connor Kuehl]

On 3/1/21 11:28 AM, Connor Kuehl wrote:
> The contents of this patch were initially developed and posted by Han
> Han[1], however, it appears the original patch was not applied. Since
> then, the relevant documentation has been moved and adapted to a new
> format.
> 
> I've taken most of the original wording and tweaked it according to
> some of the feedback from the original patch submission. I've also
> adapted it to restructured text, which is the format the documentation
> currently uses.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html
> 
> Reported-by: Han Han <hhan@redhat.com>
> Co-developed-by: Han Han <hhan@redhat.com>
> Fixes: https://bugzilla.redhat.com/1763105
> Signed-off-by: Connor Kuehl <ckuehl@redhat.com>

I am not sure whose tree this is best suited for; 
./scripts/get_maintainers.pl only showed other contributors to this file 
and not an explicit maintainer, per se.

Connor

Re: [PATCH] Document qemu-img options data_file and data_file_raw

[Connor Kuehl]

Ping (+Kevin Wolf to CC)

Kevin, would this be appropriate for your tree?

On 3/1/21 11:28 AM, Connor Kuehl wrote:
> The contents of this patch were initially developed and posted by Han
> Han[1], however, it appears the original patch was not applied. Since
> then, the relevant documentation has been moved and adapted to a new
> format.
> 
> I've taken most of the original wording and tweaked it according to
> some of the feedback from the original patch submission. I've also
> adapted it to restructured text, which is the format the documentation
> currently uses.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html
> 
> Reported-by: Han Han <hhan@redhat.com>
> Co-developed-by: Han Han <hhan@redhat.com>
> Fixes: https://bugzilla.redhat.com/1763105
> Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
> ---
>   docs/tools/qemu-img.rst | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
> index b615aa8419..5cc585dc27 100644
> --- a/docs/tools/qemu-img.rst
> +++ b/docs/tools/qemu-img.rst
> @@ -866,6 +866,18 @@ Supported image file formats:
>       issue ``lsattr filename`` to check if the NOCOW flag is set or not
>       (Capital 'C' is NOCOW flag).
>   
> +  ``data_file``
> +    Pathname that refers to a file that will store all guest data. If
> +    this option is used, the qcow2 file will only contain the image's
> +    metadata.
> +
> +  ``data_file_raw``
> +    If this option is set to ``on``, QEMU will always keep the external
> +    data file consistent as a standalone read-only raw image. The default
> +    value is ``off``.
> +
> +    This option can only be enabled if ``data_file`` is set.
> +
>   ``Other``
>   
>     QEMU also supports various other image file formats for
> 

Re: [PATCH] Document qemu-img options data_file and data_file_raw

[Stefan Hajnoczi]

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

On Mon, Mar 08, 2021 at 10:43:54AM -0600, Connor Kuehl wrote:
> On 3/1/21 11:28 AM, Connor Kuehl wrote:
> > The contents of this patch were initially developed and posted by Han
> > Han[1], however, it appears the original patch was not applied. Since
> > then, the relevant documentation has been moved and adapted to a new
> > format.
> > 
> > I've taken most of the original wording and tweaked it according to
> > some of the feedback from the original patch submission. I've also
> > adapted it to restructured text, which is the format the documentation
> > currently uses.
> > 
> > [1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html
> > 
> > Reported-by: Han Han <hhan@redhat.com>
> > Co-developed-by: Han Han <hhan@redhat.com>
> > Fixes: https://bugzilla.redhat.com/1763105
> > Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
> 
> I am not sure whose tree this is best suited for;
> ./scripts/get_maintainers.pl only showed other contributors to this file and
> not an explicit maintainer, per se.

It can go through Kevin Wolf or Max Reitz's trees. They maintain
qemu-img.

Stefan

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

Re: [PATCH] Document qemu-img options data_file and data_file_raw

[John Snow]

On 3/1/21 12:28 PM, Connor Kuehl wrote:
> The contents of this patch were initially developed and posted by Han
> Han[1], however, it appears the original patch was not applied. Since
> then, the relevant documentation has been moved and adapted to a new
> format.
> 
> I've taken most of the original wording and tweaked it according to
> some of the feedback from the original patch submission. I've also
> adapted it to restructured text, which is the format the documentation
> currently uses.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html
> 
> Reported-by: Han Han <hhan@redhat.com>
> Co-developed-by: Han Han <hhan@redhat.com>

I think it's okay to just keep the "Signed-off-by" from Han Han here, 
and the implication is that you are signing off on modifications you've 
made since.

> Fixes: https://bugzilla.redhat.com/1763105
> Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
> ---
>   docs/tools/qemu-img.rst | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
> index b615aa8419..5cc585dc27 100644
> --- a/docs/tools/qemu-img.rst
> +++ b/docs/tools/qemu-img.rst
> @@ -866,6 +866,18 @@ Supported image file formats:
>       issue ``lsattr filename`` to check if the NOCOW flag is set or not
>       (Capital 'C' is NOCOW flag).
>   
> +  ``data_file``
> +    Pathname that refers to a file that will store all guest data. If
> +    this option is used, the qcow2 file will only contain the image's
> +    metadata.
> +

Might recommend "filename" simply for parity with *FILENAME* argument.

(This is the first appearance of "Pathname" in this file without spaces, 
though "Path name" is indeed used several times.)

> +  ``data_file_raw``
> +    If this option is set to ``on``, QEMU will always keep the external
> +    data file consistent as a standalone read-only raw image. The default
> +    value is ``off``.
> +
> +    This option can only be enabled if ``data_file`` is set.
> +

How does this interact with caching options, if it does? What happens in 
the negative case -- how does the file become inconsistent?

>   ``Other``
>   
>     QEMU also supports various other image file formats for
> 

Re: [PATCH] Document qemu-img options data_file and data_file_raw

[Max Reitz]

On 01.03.21 18:28, Connor Kuehl wrote:
> The contents of this patch were initially developed and posted by Han
> Han[1], however, it appears the original patch was not applied. Since
> then, the relevant documentation has been moved and adapted to a new
> format.
> 
> I've taken most of the original wording and tweaked it according to
> some of the feedback from the original patch submission. I've also
> adapted it to restructured text, which is the format the documentation
> currently uses.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg01253.html
> 
> Reported-by: Han Han <hhan@redhat.com>
> Co-developed-by: Han Han <hhan@redhat.com>
> Fixes: https://bugzilla.redhat.com/1763105
> Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
> ---
>   docs/tools/qemu-img.rst | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
> index b615aa8419..5cc585dc27 100644
> --- a/docs/tools/qemu-img.rst
> +++ b/docs/tools/qemu-img.rst
> @@ -866,6 +866,18 @@ Supported image file formats:
>       issue ``lsattr filename`` to check if the NOCOW flag is set or not
>       (Capital 'C' is NOCOW flag).
>   
> +  ``data_file``
> +    Pathname that refers to a file that will store all guest data. If
> +    this option is used, the qcow2 file will only contain the image's
> +    metadata.

I think I would like a note here about the fact that when passing this 
option to qemu-img create, the given data file will be newly created, 
i.e. if it already contains data, all that data will be lost.  And 
perhaps also note that qemu-img amend on the other hand will only change 
the reference in the qcow2 file, so the given file should already exist 
and will not be overwritten.

(“Pathname that refers to a file” sounds like the file may already exist 
before this operation, which may give people ideas.  (Not that the ideas 
were bad, it’s just that they have to take care.  Referencing qemu-img 
amend should give them a hint on how to do it right.))

Max

> +
> +  ``data_file_raw``
> +    If this option is set to ``on``, QEMU will always keep the external
> +    data file consistent as a standalone read-only raw image. The default
> +    value is ``off``.
> +
> +    This option can only be enabled if ``data_file`` is set.
> +
>   ``Other``
>   
>     QEMU also supports various other image file formats for
> 

Re: [PATCH] Document qemu-img options data_file and data_file_raw

[Connor Kuehl]

On 3/26/21 4:24 AM, Max Reitz wrote:
> On 01.03.21 18:28, Connor Kuehl wrote:
>> [..]
>> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
>> index b615aa8419..5cc585dc27 100644
>> --- a/docs/tools/qemu-img.rst
>> +++ b/docs/tools/qemu-img.rst
>> @@ -866,6 +866,18 @@ Supported image file formats:
>>       issue ``lsattr filename`` to check if the NOCOW flag is set or not
>>       (Capital 'C' is NOCOW flag).
>> +  ``data_file``
>> +    Pathname that refers to a file that will store all guest data. If
>> +    this option is used, the qcow2 file will only contain the image's
>> +    metadata.
> 
> I think I would like a note here about the fact that when passing this 
> option to qemu-img create, the given data file will be newly created, 
> i.e. if it already contains data, all that data will be lost.  And 
> perhaps also note that qemu-img amend on the other hand will only change 
> the reference in the qcow2 file, so the given file should already exist 
> and will not be overwritten.
> 
> (“Pathname that refers to a file” sounds like the file may already exist 
> before this operation, which may give people ideas.  (Not that the ideas 
> were bad, it’s just that they have to take care.  Referencing qemu-img 
> amend should give them a hint on how to do it right.))

Hey, I just wanted to leave a note indicating that I'm not ignoring this 
review; I've incorporated it in my next version but I am waiting until 
after the freeze to send it.

Thank you!

Connor

Re: [PATCH] Document qemu-img options data_file and data_file_raw

[Connor Kuehl]

On 3/23/21 6:15 PM, John Snow wrote:
> On 3/1/21 12:28 PM, Connor Kuehl wrote:
>> [..]
>> diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
>> index b615aa8419..5cc585dc27 100644
>> --- a/docs/tools/qemu-img.rst
>> +++ b/docs/tools/qemu-img.rst
>> @@ -866,6 +866,18 @@ Supported image file formats:
>>       issue ``lsattr filename`` to check if the NOCOW flag is set or not
>>       (Capital 'C' is NOCOW flag).
>> +  ``data_file``
>> +    Pathname that refers to a file that will store all guest data. If
>> +    this option is used, the qcow2 file will only contain the image's
>> +    metadata.
>> +
> 
> Might recommend "filename" simply for parity with *FILENAME* argument.
> 
> (This is the first appearance of "Pathname" in this file without spaces, 
> though "Path name" is indeed used several times.)
> 
>> +  ``data_file_raw``
>> +    If this option is set to ``on``, QEMU will always keep the external
>> +    data file consistent as a standalone read-only raw image. The 
>> default
>> +    value is ``off``.
>> +
>> +    This option can only be enabled if ``data_file`` is set.
>> +
> 
> How does this interact with caching options, if it does? What happens in 
> the negative case -- how does the file become inconsistent?

Hi,

First, just wanted to share the same note I'm sending to Max's review:

> Hey, I just wanted to leave a note indicating that I'm not ignoring this review; I've incorporated it in my next version but I am waiting until after the freeze to send it.

Second: I've been trying to figure out if and how this affects caching. 
I do not know yet. I will keep digging. My next version of the patch 
does touch on the negative case though, describing how it becomes 
inconsistent.

Thank you!

Connor