[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command |
Date: |
Fri, 20 Sep 2013 17:34:22 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 20.09.2013 um 17:22 hat Eric Blake geschrieben:
> On 09/20/2013 05:54 AM, Kevin Wolf wrote:
> > For examples see the changes to qmp-commands.hx.
> >
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> > blockdev.c | 57 ++++++++++++
> > qapi-schema.json | 270
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > qmp-commands.hx | 59 ++++++++++++
> > 3 files changed, 386 insertions(+)
> >
>
>
> > +# Since: 1.7
> > +##
> > +{ 'type': 'BlockdevOptionsBase',
> > + 'data': { 'driver': 'str',
> > + '*id': 'str',
> > + '*discard': 'BlockdevDiscardOptions',
> > + '*cache': 'BlockdevCacheOptions',
> > + '*aio': 'BlockdevAIOOptions',
>
> Is the double space intentional? Harmless, but inconsistent.
No, I'll fix it.
> > + '*rerror': 'BlockdevOnError',
> > + '*werror': 'BlockdevOnError',
> > + '*throttling': 'BlockdevThrottlingOptions',
> > + '*read-only': 'bool' } }
> ...
>
> > +##
> > +# @BlockdevOptionsVVFAT
> > +#
> > +# Driver specific block device options for the vvfat protocol.
> > +#
> > +# @dir: directory to be exported as FAT image
> > +# @fat-type: #optional FAT type: 12, 16 or 32
> > +# @floppy: #optional whether to export a floppy imae (true) or
> > partitioned
> > +# hard disk (false; default)
> > +# @rw: #optional whether to allow write operations (default:
> > false)
>
> Why do we have 'read-only' in base, and 'rw' in vvfat? It feels like
> the vvfat option is redundant.
I guess it is kind of redundant. The reason for it is that it's always
been there, encoded in the filename as "fat:rw:..." - and the reason for
that is that read-write vvfat is even more unreliable than read-only
vvfat. So we have a read-only=false default in base, and a
read-only=true default in vvfat.
I'm not sure if changing this without breaking the command line
is possible; but if it is, it involves vvfat-specific magic in
drive_init(), which this simply isn't worth.
> > +##
> > +# @BlockdevOptionsGenericCOWFormat
> > +#
> > +# Driver specific block device options for image format that have no option
> > +# besides their data source and an optional backing file.
> > +#
> > +# @file: reference to or definition of the data source block device
>
> Do you need to document this field...
>
> > +# @backing: #optional reference to or definition of the backing file
> > block
> > +# device (if missing, taken from the image file content). It
> > is
> > +# allowed to pass an empty string here in order to disable
> > the
> > +# default backing file.
> > +# @copy-on-read: #optional whether to enable copy on read for the device
> > +# (default: false). Copy on read can only be used if the
> > +# image is not read-only.
> > +#
> > +# Since: 1.7
> > +##
> > +{ 'type': 'BlockdevOptionsGenericCOWFormat',
> > + 'base': 'BlockdevOptionsGenericFormat',
> > + 'data': { '*backing': 'BlockdevRef',
> > + '*copy-on-read': 'bool' } }
>
> ...given that it is only present by inheritence?
>
> > +##
> > +# @BlockdevOptionsQcow2
> > +#
> > +# Driver specific block device options for qcow2.
> > +#
> > +# @file: reference to or definition of the data source block device
> > +#
> > +# @backing: #optional reference to or definition of the backing file
> > block
> > +# device (if missing, taken from the image file content)
>
> Same question. If you DO document inherited fields, you missed
> @copy-on-read; if you DON'T document inherited fields, these two aren't
> necessary (instead, you could just have some statement about: "In
> addition to the fields documented in BlockdevOptionsGenericCOWFormat,
> this struct includes:")
Nope, I don't think they are necessary. I just wasn't careful enough
when I added the inheritance. I'll drop them.
Kevin
- Re: [Qemu-devel] [PATCH 05/17] blockdev: Separate ID generation from DriveInfo creation, (continued)
- [Qemu-devel] [PATCH 06/17] blockdev: Pass QDict to blockdev_init(), Kevin Wolf, 2013/09/20
- [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command, Kevin Wolf, 2013/09/20
- Re: [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command, Max Reitz, 2013/09/20
- Re: [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command, Benoît Canet, 2013/09/20
- Re: [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command, Eric Blake, 2013/09/20
- Re: [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command,
Kevin Wolf <=
- Re: [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command, Fam Zheng, 2013/09/24
- Re: [Qemu-devel] [PATCH 04/17] blockdev: 'blockdev-add' QMP command, Paolo Bonzini, 2013/09/24
- [Qemu-devel] [PATCH 07/17] blockdev: Move parsing of 'media' option to drive_init, Kevin Wolf, 2013/09/20
- [Qemu-devel] [PATCH 08/17] blockdev: Move parsing of 'if' option to drive_init, Kevin Wolf, 2013/09/20
- [Qemu-devel] [PATCH 09/17] blockdev: Moving parsing of geometry options to drive_init, Kevin Wolf, 2013/09/20