qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>, Fam Zheng <famz@redhat.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
	Markus Armbruster <armbru@redhat.com>
Subject: [Qemu-devel] qmp-commands.hx and inherited types (Was: Re: [PATCH v6 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove)
Date: Tue, 18 Nov 2014 11:44:33 -0500	[thread overview]
Message-ID: <546B7771.2070608@redhat.com> (raw)
In-Reply-To: <545CC25E.5090405@redhat.com>



On 11/07/2014 08:00 AM, Eric Blake wrote:
> On 10/30/2014 04:22 AM, Fam Zheng wrote:

[snip]

>> +++ b/qapi/block-core.json
>> @@ -865,6 +865,61 @@
>>               '*on-target-error': 'BlockdevOnError' } }
>>
>>   ##
>> +# @BlockDirtyBitmap
>> +#
>> +# @device: name of device which the bitmap is tracking
>> +#
>> +# @name: name of the dirty bitmap
>> +#
>> +# Since 2.3
>> +##
>> +{ 'type': 'BlockDirtyBitmap',
>> +  'data': { 'device': 'str', 'name': 'str' } }
>> +
>> +##
>> +# @BlockDirtyBitmapAdd
>> +#
>> +# @device: name of device which the bitmap is tracking
>> +#
>> +# @name: name of the dirty bitmap
>> +#
>> +# @granularity: #optional the bitmap granularity, default is 64k for
>> +#               block-dirty-bitmap-add
>
> Do you still need to call out the command, given that it is the only
> client of this type?
>
>> +#
>> +# Since 2.3
>> +##
>> +{ 'type': 'BlockDirtyBitmapAdd',
>> +  'data': { 'device': 'str', 'name': 'str', '*granularity': 'int' } }
>
> Is it worth using type inheritance, as in:
>
> { 'type': 'BlockDirtyBitmapAdd',
>    'base': 'BlockDirtyBitmap',
>    'data': { '*granularity': 'int' } }
>

Strictly speaking, I would argue against inheritance here because 
"BlockDirtyBitmapAdd" is not "isa" "BlockDirtyBitmap". It's more of a 
"Hasa" relationship.

At any rate, I tried to implement this for giggles to see if I could, 
and ran into the following issue with which I'd be curious to get an 
answer for.

As an example, If you have some type:

{ 'type': 'example',
   'data': { 'foo': 'int' } }

And an extension of it:

{ 'type': 'academicExample'
   'base': 'example',
   'data': { 'bar': 'str' } }

How would you write a command that expected both "foo" and "bar"?
The following doesn't seem appropriate (the generated code SKIPS the 
base fields, which leads to missing arguments in the prototype:

{ 'command': 'academic-command',
   'data': 'academicExample' }

...

{
	.name = "academic-command",
	.args_type = "foo:i,bar:s",
	.mhandler.cmd_new = qmp_marshal_input_academic_command,
},


The generated prototype appears to skip the "foo" argument, including 
only the arguments associated with the base type, in this case, 'bar'.

Do we support this kind of use? I didn't see it in-use currently, but I 
only gave it a cursory skimming.

  reply	other threads:[~2014-11-18 16:44 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-30  3:22 [Qemu-devel] [PATCH v6 00/10] block: Incremental backup series Fam Zheng
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 01/10] qapi: Add optional field "name" to block dirty bitmap Fam Zheng
2014-11-04  9:08   ` Max Reitz
2014-11-07 12:48   ` Eric Blake
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove Fam Zheng
2014-11-04  9:26   ` Max Reitz
2014-11-07 13:00   ` Eric Blake
2014-11-18 16:44     ` John Snow [this message]
2014-11-18 17:00       ` [Qemu-devel] qmp-commands.hx and inherited types (Was: Re: [PATCH v6 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove) Eric Blake
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 03/10] block: Introduce bdrv_dirty_bitmap_granularity() Fam Zheng
2014-11-04  9:44   ` Max Reitz
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 04/10] hbitmap: Add hbitmap_copy Fam Zheng
2014-11-04  9:58   ` Max Reitz
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 05/10] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap Fam Zheng
2014-11-04 10:08   ` Max Reitz
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 06/10] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable Fam Zheng
2014-11-04 10:17   ` Max Reitz
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup Fam Zheng
2014-11-04 10:53   ` Max Reitz
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 08/10] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} Fam Zheng
2014-11-04 11:03   ` Max Reitz
2014-11-21 22:24     ` John Snow
2014-11-24  8:35       ` Max Reitz
2014-11-24  9:41         ` Paolo Bonzini
2014-11-24  9:46           ` Max Reitz
2014-11-24  9:54             ` Paolo Bonzini
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 09/10] qmp: Add dirty bitmap 'enabled' field in query-block Fam Zheng
2014-11-04 11:05   ` Max Reitz
2014-10-30  3:22 ` [Qemu-devel] [PATCH v6 10/10] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap Fam Zheng
2014-11-04 11:10   ` Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=546B7771.2070608@redhat.com \
    --to=jsnow@redhat.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=famz@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).