1. Patchew
  2. QEMU
  3. View series

[PATCH] Revert "qapi: fix examples of blockdev-add with qcow2"

Markus Armbruster posted 1 patch 1 year, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220930171908.846769-1-armbru@redhat.com
Maintainers: Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
qapi/block-core.json | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[PATCH] Revert "qapi: fix examples of blockdev-add with qcow2"
Posted by Markus Armbruster 1 year, 6 months ago 
This reverts commit b6522938327141235b97ab38e40c6c4512587373.

Kevin Wolf NAKed this patch, because:

    'file' is a required member (defined in BlockdevOptionsGenericFormat),
    removing it makes the example invalid. 'data-file' is only an additional
    optional member to be used for external data files (i.e. when the guest
    data is kept separate from the metadata in the .qcow2 file).

However, it had already been merged then.  Revert.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/block-core.json | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index f21fa235f2..882b266532 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1541,8 +1541,8 @@
 # -> { "execute": "blockdev-add",
 #      "arguments": { "driver": "qcow2",
 #                     "node-name": "node1534",
-#                     "data-file": { "driver": "file",
-#                                    "filename": "hd1.qcow2" },
+#                     "file": { "driver": "file",
+#                               "filename": "hd1.qcow2" },
 #                     "backing": null } }
 #
 # <- { "return": {} }
@@ -4378,7 +4378,7 @@
 #      "arguments": {
 #           "driver": "qcow2",
 #           "node-name": "test1",
-#           "data-file": {
+#           "file": {
 #               "driver": "file",
 #               "filename": "test.qcow2"
 #            }
@@ -4395,7 +4395,7 @@
 #           "cache": {
 #              "direct": true
 #            },
-#           "data-file": {
+#            "file": {
 #              "driver": "file",
 #              "filename": "/tmp/test.qcow2"
 #            },
@@ -4477,7 +4477,7 @@
 #      "arguments": {
 #           "driver": "qcow2",
 #           "node-name": "node0",
-#           "data-file": {
+#           "file": {
 #               "driver": "file",
 #               "filename": "test.qcow2"
 #           }
-- 
2.37.2
Re: [PATCH] Revert "qapi: fix examples of blockdev-add with qcow2"
Posted by Kevin Wolf 1 year, 6 months ago 
Am 30.09.2022 um 19:19 hat Markus Armbruster geschrieben:
> This reverts commit b6522938327141235b97ab38e40c6c4512587373.
> 
> Kevin Wolf NAKed this patch, because:
> 
>     'file' is a required member (defined in BlockdevOptionsGenericFormat),
>     removing it makes the example invalid. 'data-file' is only an additional
>     optional member to be used for external data files (i.e. when the guest
>     data is kept separate from the metadata in the .qcow2 file).
> 
> However, it had already been merged then.  Revert.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Thanks, applied to the block branch.

Kevin
Re: [PATCH] Revert "qapi: fix examples of blockdev-add with qcow2"
Posted by Victor Toso 1 year, 6 months ago 
Hi,

Sorry taking some time to reply.

On Fri, Sep 30, 2022 at 07:19:08PM +0200, Markus Armbruster wrote:
> This reverts commit b6522938327141235b97ab38e40c6c4512587373.

Which is:
```
  qapi: fix examples of blockdev-add with qcow2

  The examples use "qcow2" driver with the wrong member name for
  BlockdevRef alternate type. This patch changes all wrong member names
  from "file" to "data-file" which is the correct member name in
  BlockdevOptionsQcow2 for the BlockdevRef field.

  Problem was noticed when using the example as a test case for Go
  bindings.
```

> Kevin Wolf NAKed this patch, because:
>
>     'file' is a required member (defined in BlockdevOptionsGenericFormat),
>     removing it makes the example invalid. 'data-file' is only an additional
>     optional member to be used for external data files (i.e. when the guest
>     data is kept separate from the metadata in the .qcow2 file).

You are correct. I apologize for the mistake. I trusted a bit too
much on the Go bindings and didn't realize that @file member was
actually present:
    @BlockdevOptionsQcow2 >
    @BlockdevOptionsGenericCOWFormat >
    @BlockdevOptionsGenericCOWFormat >
    @BlockdevOptionsGenericFormat

> However, it had already been merged then.  Revert.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Acked-by: Victor Toso <victortoso@redhat.com>

> ---
>  qapi/block-core.json | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index f21fa235f2..882b266532 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1541,8 +1541,8 @@
>  # -> { "execute": "blockdev-add",
>  #      "arguments": { "driver": "qcow2",
>  #                     "node-name": "node1534",
> -#                     "data-file": { "driver": "file",
> -#                                    "filename": "hd1.qcow2" },
> +#                     "file": { "driver": "file",
> +#                               "filename": "hd1.qcow2" },
>  #                     "backing": null } }
>  #
>  # <- { "return": {} }
> @@ -4378,7 +4378,7 @@
>  #      "arguments": {
>  #           "driver": "qcow2",
>  #           "node-name": "test1",
> -#           "data-file": {
> +#           "file": {
>  #               "driver": "file",
>  #               "filename": "test.qcow2"
>  #            }
> @@ -4395,7 +4395,7 @@
>  #           "cache": {
>  #              "direct": true
>  #            },
> -#           "data-file": {
> +#            "file": {
>  #              "driver": "file",
>  #              "filename": "/tmp/test.qcow2"
>  #            },
> @@ -4477,7 +4477,7 @@
>  #      "arguments": {
>  #           "driver": "qcow2",
>  #           "node-name": "node0",
> -#           "data-file": {
> +#           "file": {
>  #               "driver": "file",
>  #               "filename": "test.qcow2"
>  #           }
> -- 
> 2.37.2
>
Re: [PATCH] Revert "qapi: fix examples of blockdev-add with qcow2"
Posted by Victor Toso 1 year, 6 months ago 
Hi,

On Tue, Oct 04, 2022 at 10:13:17AM +0200, Victor Toso wrote:
> Hi,
> 
> Sorry taking some time to reply.
> 
> On Fri, Sep 30, 2022 at 07:19:08PM +0200, Markus Armbruster wrote:
> > This reverts commit b6522938327141235b97ab38e40c6c4512587373.
> 
> Which is:
> ```
>   qapi: fix examples of blockdev-add with qcow2
> 
>   The examples use "qcow2" driver with the wrong member name for
>   BlockdevRef alternate type. This patch changes all wrong member names
>   from "file" to "data-file" which is the correct member name in
>   BlockdevOptionsQcow2 for the BlockdevRef field.
> 
>   Problem was noticed when using the example as a test case for Go
>   bindings.
> ```
> 
> > Kevin Wolf NAKed this patch, because:
> >
> >     'file' is a required member (defined in BlockdevOptionsGenericFormat),
> >     removing it makes the example invalid. 'data-file' is only an additional
> >     optional member to be used for external data files (i.e. when the guest
> >     data is kept separate from the metadata in the .qcow2 file).
> 
> You are correct. I apologize for the mistake. I trusted a bit too
> much on the Go bindings and didn't realize that @file member was
> actually present:
>     @BlockdevOptionsQcow2 >
>     @BlockdevOptionsGenericCOWFormat >
>     @BlockdevOptionsGenericCOWFormat >
>     @BlockdevOptionsGenericFormat
> 
> > However, it had already been merged then.  Revert.
> >
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> Acked-by: Victor Toso <victortoso@redhat.com>

Just realized, this should have been a reviewed-by ...

> > ---
> >  qapi/block-core.json | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index f21fa235f2..882b266532 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -1541,8 +1541,8 @@
> >  # -> { "execute": "blockdev-add",
> >  #      "arguments": { "driver": "qcow2",
> >  #                     "node-name": "node1534",
> > -#                     "data-file": { "driver": "file",
> > -#                                    "filename": "hd1.qcow2" },
> > +#                     "file": { "driver": "file",
> > +#                               "filename": "hd1.qcow2" },
> >  #                     "backing": null } }
> >  #
> >  # <- { "return": {} }
> > @@ -4378,7 +4378,7 @@
> >  #      "arguments": {
> >  #           "driver": "qcow2",
> >  #           "node-name": "test1",
> > -#           "data-file": {
> > +#           "file": {
> >  #               "driver": "file",
> >  #               "filename": "test.qcow2"
> >  #            }
> > @@ -4395,7 +4395,7 @@
> >  #           "cache": {
> >  #              "direct": true
> >  #            },
> > -#           "data-file": {
> > +#            "file": {
> >  #              "driver": "file",
> >  #              "filename": "/tmp/test.qcow2"
> >  #            },
> > @@ -4477,7 +4477,7 @@
> >  #      "arguments": {
> >  #           "driver": "qcow2",
> >  #           "node-name": "node0",
> > -#           "data-file": {
> > +#           "file": {
> >  #               "driver": "file",
> >  #               "filename": "test.qcow2"
> >  #           }
> > -- 
> > 2.37.2
> >

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.