diff mbox series

[02/14] qapi: fix example of BLOCK_IMAGE_CORRUPTED event

Message ID 20220324175015.232794-3-victortoso@redhat.com (mailing list archive)
State New, archived
Headers show
Series Fix some qapi examples and a TODO section | expand

Commit Message

Victor Toso March 24, 2022, 5:50 p.m. UTC
Fatal is not optional.

Signed-off-by: Victor Toso <victortoso@redhat.com>
---
 qapi/block-core.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

John Snow March 24, 2022, 7:15 p.m. UTC | #1
On Thu, Mar 24, 2022, 1:50 PM Victor Toso <victortoso@redhat.com> wrote:

> Fatal is not optional.
>
> Signed-off-by: Victor Toso <victortoso@redhat.com>
> ---
>  qapi/block-core.json | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index e89f2dfb5b..585a9e020e 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -5008,7 +5008,7 @@
>  # <- { "event": "BLOCK_IMAGE_CORRUPTED",
>  #      "data": { "device": "ide0-hd0", "node-name": "node0",
>  #                "msg": "Prevented active L1 table overwrite", "offset":
> 196608,
> -#                "size": 65536 },
> +#                "size": 65536, "fatal": false },
>  #      "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
>  #
>  # Since: 1.7
> --
> 2.35.1
>

Is this the correct fatality setting for this particular case? Default is
implied to be true.

>
John Snow March 24, 2022, 8:40 p.m. UTC | #2
On Thu, Mar 24, 2022 at 3:15 PM John Snow <jsnow@redhat.com> wrote:
>
>
>
> On Thu, Mar 24, 2022, 1:50 PM Victor Toso <victortoso@redhat.com> wrote:
>>
>> Fatal is not optional.
>>
>> Signed-off-by: Victor Toso <victortoso@redhat.com>
>> ---
>>  qapi/block-core.json | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index e89f2dfb5b..585a9e020e 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -5008,7 +5008,7 @@
>>  # <- { "event": "BLOCK_IMAGE_CORRUPTED",
>>  #      "data": { "device": "ide0-hd0", "node-name": "node0",
>>  #                "msg": "Prevented active L1 table overwrite", "offset": 196608,
>> -#                "size": 65536 },
>> +#                "size": 65536, "fatal": false },
>>  #      "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
>>  #
>>  # Since: 1.7
>> --
>> 2.35.1
>
>
> Is this the correct fatality setting for this particular case? Default is implied to be true.

(1) We don't seem to actually emit this particular message anymore. I
don't think it exists in the tree.

(2) The only fatal=False messages I can see is
"Cannot free unaligned cluster %#llx"

(Try grepping for qcow2_signal_corruption)

so maybe we should pick a new example that might really exist. iotest
060 seems to test this, so that can be used as a guide.

--js
Markus Armbruster March 25, 2022, 12:43 p.m. UTC | #3
John Snow <jsnow@redhat.com> writes:

> On Thu, Mar 24, 2022 at 3:15 PM John Snow <jsnow@redhat.com> wrote:
>>
>>
>>
>> On Thu, Mar 24, 2022, 1:50 PM Victor Toso <victortoso@redhat.com> wrote:
>>>
>>> Fatal is not optional.
>>>
>>> Signed-off-by: Victor Toso <victortoso@redhat.com>
>>> ---
>>>  qapi/block-core.json | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index e89f2dfb5b..585a9e020e 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>>> @@ -5008,7 +5008,7 @@
>>>  # <- { "event": "BLOCK_IMAGE_CORRUPTED",
>>>  #      "data": { "device": "ide0-hd0", "node-name": "node0",
>>>  #                "msg": "Prevented active L1 table overwrite", "offset": 196608,
>>> -#                "size": 65536 },
>>> +#                "size": 65536, "fatal": false },
>>>  #      "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
>>>  #
>>>  # Since: 1.7
>>> --
>>> 2.35.1
>>
>>
>> Is this the correct fatality setting for this particular case? Default is implied to be true.
>
> (1) We don't seem to actually emit this particular message anymore. I
> don't think it exists in the tree.

I doubt we ever emitted it.

> (2) The only fatal=False messages I can see is
> "Cannot free unaligned cluster %#llx"
>
> (Try grepping for qcow2_signal_corruption)
>
> so maybe we should pick a new example that might really exist. iotest
> 060 seems to test this, so that can be used as a guide.

Yes, please.
Victor Toso March 25, 2022, 8:59 p.m. UTC | #4
Hi,

On Fri, Mar 25, 2022 at 01:43:06PM +0100, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
> > On Thu, Mar 24, 2022 at 3:15 PM John Snow <jsnow@redhat.com> wrote:
> >>
> >>
> >>
> >> On Thu, Mar 24, 2022, 1:50 PM Victor Toso <victortoso@redhat.com> wrote:
> >>>
> >>> Fatal is not optional.
> >>>
> >>> Signed-off-by: Victor Toso <victortoso@redhat.com>
> >>> ---
> >>>  qapi/block-core.json | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/qapi/block-core.json b/qapi/block-core.json
> >>> index e89f2dfb5b..585a9e020e 100644
> >>> --- a/qapi/block-core.json
> >>> +++ b/qapi/block-core.json
> >>> @@ -5008,7 +5008,7 @@
> >>>  # <- { "event": "BLOCK_IMAGE_CORRUPTED",
> >>>  #      "data": { "device": "ide0-hd0", "node-name": "node0",
> >>>  #                "msg": "Prevented active L1 table overwrite", "offset": 196608,
> >>> -#                "size": 65536 },
> >>> +#                "size": 65536, "fatal": false },
> >>>  #      "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
> >>>  #
> >>>  # Since: 1.7
> >>> --
> >>> 2.35.1
> >>
> >>
> >> Is this the correct fatality setting for this particular case? Default is implied to be true.
> >
> > (1) We don't seem to actually emit this particular message anymore. I
> > don't think it exists in the tree.
> 
> I doubt we ever emitted it.

Out of curiosity, shouldn't we deprecate it then?

> > (2) The only fatal=False messages I can see is
> > "Cannot free unaligned cluster %#llx"
> >
> > (Try grepping for qcow2_signal_corruption)
> >
> > so maybe we should pick a new example that might really exist. iotest
> > 060 seems to test this, so that can be used as a guide.
> 
> Yes, please.

I'll be changing it on v2
John Snow March 25, 2022, 9:40 p.m. UTC | #5
On Fri, Mar 25, 2022 at 4:59 PM Victor Toso <victortoso@redhat.com> wrote:
>
> Hi,
>
> On Fri, Mar 25, 2022 at 01:43:06PM +0100, Markus Armbruster wrote:
> > John Snow <jsnow@redhat.com> writes:
> >
> > > On Thu, Mar 24, 2022 at 3:15 PM John Snow <jsnow@redhat.com> wrote:
> > >>
> > >>
> > >>
> > >> On Thu, Mar 24, 2022, 1:50 PM Victor Toso <victortoso@redhat.com> wrote:
> > >>>
> > >>> Fatal is not optional.
> > >>>
> > >>> Signed-off-by: Victor Toso <victortoso@redhat.com>
> > >>> ---
> > >>>  qapi/block-core.json | 2 +-
> > >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/qapi/block-core.json b/qapi/block-core.json
> > >>> index e89f2dfb5b..585a9e020e 100644
> > >>> --- a/qapi/block-core.json
> > >>> +++ b/qapi/block-core.json
> > >>> @@ -5008,7 +5008,7 @@
> > >>>  # <- { "event": "BLOCK_IMAGE_CORRUPTED",
> > >>>  #      "data": { "device": "ide0-hd0", "node-name": "node0",
> > >>>  #                "msg": "Prevented active L1 table overwrite", "offset": 196608,
> > >>> -#                "size": 65536 },
> > >>> +#                "size": 65536, "fatal": false },
> > >>>  #      "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
> > >>>  #
> > >>>  # Since: 1.7
> > >>> --
> > >>> 2.35.1
> > >>
> > >>
> > >> Is this the correct fatality setting for this particular case? Default is implied to be true.
> > >
> > > (1) We don't seem to actually emit this particular message anymore. I
> > > don't think it exists in the tree.
> >
> > I doubt we ever emitted it.
>
> Out of curiosity, shouldn't we deprecate it then?
>

He means: we probably never emitted that specific error message. The
*event* is in use, see iotest 060.

> > > (2) The only fatal=False messages I can see is
> > > "Cannot free unaligned cluster %#llx"
> > >
> > > (Try grepping for qcow2_signal_corruption)
> > >
> > > so maybe we should pick a new example that might really exist. iotest
> > > 060 seems to test this, so that can be used as a guide.
> >
> > Yes, please.
>
> I'll be changing it on v2
diff mbox series

Patch

diff --git a/qapi/block-core.json b/qapi/block-core.json
index e89f2dfb5b..585a9e020e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -5008,7 +5008,7 @@ 
 # <- { "event": "BLOCK_IMAGE_CORRUPTED",
 #      "data": { "device": "ide0-hd0", "node-name": "node0",
 #                "msg": "Prevented active L1 table overwrite", "offset": 196608,
-#                "size": 65536 },
+#                "size": 65536, "fatal": false },
 #      "timestamp": { "seconds": 1378126126, "microseconds": 966463 } }
 #
 # Since: 1.7