All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: John Snow <jsnow@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: Re: [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 10:00:51 -0700	[thread overview]
Message-ID: <546B7B43.1090507@redhat.com> (raw)
In-Reply-To: <546B7771.2070608@redhat.com>

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

On 11/18/2014 09:44 AM, John Snow wrote:
>> 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.

Fair enough.

> 
> 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' }

Ouch.  Sounds like a bug in the code generator.  Obviously, someone will
have to patch that (and add a testsuite entry to make sure it doesn't
regress) before we can rely on it.

> 
> ...
> 
> {
>     .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.

We supposedly document it as working, but as no one is using it
(including no testsuite entry), I'm not surprised that it doesn't work yet.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

  reply	other threads:[~2014-11-18 17:01 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     ` [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) John Snow
2014-11-18 17:00       ` Eric Blake [this message]
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=546B7B43.1090507@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=famz@redhat.com \
    --cc=jsnow@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.