Re: [Qemu-devel] [PATCH] block: Expose host_* drivers in blockdev-add
Kevin Wolf writes:
> Am 24.04.2014 um 09:27 hat Markus Armbruster geschrieben:
>> Kevin Wolf writes:
>>
>> > Am 23.04.2014 um 17:34 hat Eric Blake geschrieben:
>> >> On 04/23/2014 09:12 AM, Kevin Wolf wrote:
>> >> > All the functionality to use the host_device, host_cdrom and host_floppy
>> >> > drivers is already there, they just need to be added to the schema.
>> >> >
>> >> > Signed-off-by: Kevin Wolf
>> >> > ---
>> >> > qapi-schema.json | 6 +-
>> >> > 1 file changed, 5 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/qapi-schema.json b/qapi-schema.json
>> >> > index 391356f..0fc0f12 100644
>> >> > --- a/qapi-schema.json
>> >> > +++ b/qapi-schema.json
>> >> > @@ -4288,7 +4288,8 @@
>> >> > # Since: 2.0
>> >>
>> >> We haven't been good at tracking enum growth, but it can't hurt to try.
>> >> It might be worth changing this line to read:
>> >>
>> >> # Since: 2.0, 'host_device', 'host_cdrom', 'host_floppy' since 2.1
>> >
>> > I'm fine with documenting the changes, but this format doesn't look like
>> > it works very well in the long term.
>> >
>> >> > ##
>> >> > { 'enum': 'BlockdevDriver',
>> >> > - 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp',
>> >> > vvfat', 'blkdebug',
>> >> > + 'data': [ 'file', 'host_device', 'host_cdrom', 'host_floppy',
>> >>
>> >> Any reason you used _ instead of - in these names? Newer QMP tends to
>> >> prefer - unless there is a good reason why _ has already been baked in
>> >> due to back-compat.
>> >
>> > The block driver has always been called host_device with an underscore.
>> > We can't change it because that would break compatibility on the command
>> > line.
>>
>> A simple indirection could buy us a little more consistency in QMP. If
>> you say that's not worth the trouble because there's so much
>> inconsistency already, you have a point. I still hate it, though :)
>
> Is inconsistency between block device configuration in QMP and block
> device configuration on the command line any better than inconsistency
> between one QMP command and part of the other QMP commands?
Matter of taste :)
The command line is internally inconsistent, too: -add-fd, -mem-path,
-machine dump-guest-core=..., but also -virtfs_synth, -machine
kernel_irqchip...
No adult supervision...
So, similar problem, similar solution space:
>> Probably the only way to stop the proliferation of this '-' vs. '_'
>> nuisance is to accept both '-' and '_' everywhere in QMP, and use only
>> '-' in documentation. Clearly beyond the scope of your patch.
>
> Not sure how hard this would be to implement, but I agree that this
> would be the best solution.
Re: [Qemu-devel] [PATCH] block: Expose host_* drivers in blockdev-add
Am 24.04.2014 um 09:27 hat Markus Armbruster geschrieben:
> Kevin Wolf writes:
>
> > Am 23.04.2014 um 17:34 hat Eric Blake geschrieben:
> >> On 04/23/2014 09:12 AM, Kevin Wolf wrote:
> >> > All the functionality to use the host_device, host_cdrom and host_floppy
> >> > drivers is already there, they just need to be added to the schema.
> >> >
> >> > Signed-off-by: Kevin Wolf
> >> > ---
> >> > qapi-schema.json | 6 +-
> >> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/qapi-schema.json b/qapi-schema.json
> >> > index 391356f..0fc0f12 100644
> >> > --- a/qapi-schema.json
> >> > +++ b/qapi-schema.json
> >> > @@ -4288,7 +4288,8 @@
> >> > # Since: 2.0
> >>
> >> We haven't been good at tracking enum growth, but it can't hurt to try.
> >> It might be worth changing this line to read:
> >>
> >> # Since: 2.0, 'host_device', 'host_cdrom', 'host_floppy' since 2.1
> >
> > I'm fine with documenting the changes, but this format doesn't look like
> > it works very well in the long term.
> >
> >> > ##
> >> > { 'enum': 'BlockdevDriver',
> >> > - 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp',
> >> > vvfat', 'blkdebug',
> >> > + 'data': [ 'file', 'host_device', 'host_cdrom', 'host_floppy',
> >>
> >> Any reason you used _ instead of - in these names? Newer QMP tends to
> >> prefer - unless there is a good reason why _ has already been baked in
> >> due to back-compat.
> >
> > The block driver has always been called host_device with an underscore.
> > We can't change it because that would break compatibility on the command
> > line.
>
> A simple indirection could buy us a little more consistency in QMP. If
> you say that's not worth the trouble because there's so much
> inconsistency already, you have a point. I still hate it, though :)
Is inconsistency between block device configuration in QMP and block
device configuration on the command line any better than inconsistency
between one QMP command and part of the other QMP commands?
> Probably the only way to stop the proliferation of this '-' vs. '_'
> nuisance is to accept both '-' and '_' everywhere in QMP, and use only
> '-' in documentation. Clearly beyond the scope of your patch.
Not sure how hard this would be to implement, but I agree that this
would be the best solution.
Kevin
Re: [Qemu-devel] [PATCH] block: Expose host_* drivers in blockdev-add
Kevin Wolf writes:
> Am 23.04.2014 um 17:34 hat Eric Blake geschrieben:
>> On 04/23/2014 09:12 AM, Kevin Wolf wrote:
>> > All the functionality to use the host_device, host_cdrom and host_floppy
>> > drivers is already there, they just need to be added to the schema.
>> >
>> > Signed-off-by: Kevin Wolf
>> > ---
>> > qapi-schema.json | 6 +-
>> > 1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/qapi-schema.json b/qapi-schema.json
>> > index 391356f..0fc0f12 100644
>> > --- a/qapi-schema.json
>> > +++ b/qapi-schema.json
>> > @@ -4288,7 +4288,8 @@
>> > # Since: 2.0
>>
>> We haven't been good at tracking enum growth, but it can't hurt to try.
>> It might be worth changing this line to read:
>>
>> # Since: 2.0, 'host_device', 'host_cdrom', 'host_floppy' since 2.1
>
> I'm fine with documenting the changes, but this format doesn't look like
> it works very well in the long term.
>
>> > ##
>> > { 'enum': 'BlockdevDriver',
>> > - 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp',
>> > vvfat', 'blkdebug',
>> > + 'data': [ 'file', 'host_device', 'host_cdrom', 'host_floppy',
>>
>> Any reason you used _ instead of - in these names? Newer QMP tends to
>> prefer - unless there is a good reason why _ has already been baked in
>> due to back-compat.
>
> The block driver has always been called host_device with an underscore.
> We can't change it because that would break compatibility on the command
> line.
A simple indirection could buy us a little more consistency in QMP. If
you say that's not worth the trouble because there's so much
inconsistency already, you have a point. I still hate it, though :)
Probably the only way to stop the proliferation of this '-' vs. '_'
nuisance is to accept both '-' and '_' everywhere in QMP, and use only
'-' in documentation. Clearly beyond the scope of your patch.
Re: [Qemu-devel] [PATCH] block: Expose host_* drivers in blockdev-add
On Wed, 04/23 17:12, Kevin Wolf wrote:
> All the functionality to use the host_device, host_cdrom and host_floppy
> drivers is already there, they just need to be added to the schema.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi-schema.json | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 391356f..0fc0f12 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4288,7 +4288,8 @@
> # Since: 2.0
> ##
> { 'enum': 'BlockdevDriver',
> - 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat',
> 'blkdebug',
> + 'data': [ 'file', 'host_device', 'host_cdrom', 'host_floppy',
> +'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'blkdebug',
> 'blkverify', 'bochs', 'cloop', 'cow', 'dmg', 'parallels', 'qcow',
> 'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'quorum' ] }
>
> @@ -4555,6 +4556,9 @@
>'discriminator': 'driver',
>'data': {
>'file': 'BlockdevOptionsFile',
> + 'host_device':'BlockdevOptionsFile',
> + 'host_cdrom': 'BlockdevOptionsFile',
> + 'host_floppy':'BlockdevOptionsFile',
>'http': 'BlockdevOptionsFile',
>'https': 'BlockdevOptionsFile',
>'ftp':'BlockdevOptionsFile',
Reviewed-by: Fam Zheng
Re: [Qemu-devel] [PATCH] block: Expose host_* drivers in blockdev-add
On 04/23/2014 09:39 AM, Kevin Wolf wrote:
> Am 23.04.2014 um 17:34 hat Eric Blake geschrieben:
>> On 04/23/2014 09:12 AM, Kevin Wolf wrote:
>>> All the functionality to use the host_device, host_cdrom and host_floppy
>>> drivers is already there, they just need to be added to the schema.
>>>
>>> Signed-off-by: Kevin Wolf
>>> ---
>>> qapi-schema.json | 6 +-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index 391356f..0fc0f12 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -4288,7 +4288,8 @@
>>> # Since: 2.0
>>
>> We haven't been good at tracking enum growth, but it can't hurt to try.
>> It might be worth changing this line to read:
>>
>> # Since: 2.0, 'host_device', 'host_cdrom', 'host_floppy' since 2.1
>
> I'm fine with documenting the changes, but this format doesn't look like
> it works very well in the long term.
Any better suggestions? I'm open to ideas.
>
>>> ##
>>> { 'enum': 'BlockdevDriver',
>>> - 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat',
>>> 'blkdebug',
>>> + 'data': [ 'file', 'host_device', 'host_cdrom', 'host_floppy',
>>
>> Any reason you used _ instead of - in these names? Newer QMP tends to
>> prefer - unless there is a good reason why _ has already been baked in
>> due to back-compat.
>
> The block driver has always been called host_device with an underscore.
> We can't change it because that would break compatibility on the command
> line.
That's what I was missing. If you add that to the commit message, then
I'm okay with:
Reviewed-by: Eric Blake
and we can save the meta-issue about documenting enum growth over time
to a broader cleanup as a separate series, if we can even come to a
consensus on what would be most useful.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] block: Expose host_* drivers in blockdev-add
Am 23.04.2014 um 17:34 hat Eric Blake geschrieben:
> On 04/23/2014 09:12 AM, Kevin Wolf wrote:
> > All the functionality to use the host_device, host_cdrom and host_floppy
> > drivers is already there, they just need to be added to the schema.
> >
> > Signed-off-by: Kevin Wolf
> > ---
> > qapi-schema.json | 6 +-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 391356f..0fc0f12 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4288,7 +4288,8 @@
> > # Since: 2.0
>
> We haven't been good at tracking enum growth, but it can't hurt to try.
> It might be worth changing this line to read:
>
> # Since: 2.0, 'host_device', 'host_cdrom', 'host_floppy' since 2.1
I'm fine with documenting the changes, but this format doesn't look like
it works very well in the long term.
> > ##
> > { 'enum': 'BlockdevDriver',
> > - 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat',
> > 'blkdebug',
> > + 'data': [ 'file', 'host_device', 'host_cdrom', 'host_floppy',
>
> Any reason you used _ instead of - in these names? Newer QMP tends to
> prefer - unless there is a good reason why _ has already been baked in
> due to back-compat.
The block driver has always been called host_device with an underscore.
We can't change it because that would break compatibility on the command
line.
Kevin
pgpMeUFRz3YvB.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH] block: Expose host_* drivers in blockdev-add
On 04/23/2014 09:12 AM, Kevin Wolf wrote:
> All the functionality to use the host_device, host_cdrom and host_floppy
> drivers is already there, they just need to be added to the schema.
>
> Signed-off-by: Kevin Wolf
> ---
> qapi-schema.json | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 391356f..0fc0f12 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4288,7 +4288,8 @@
> # Since: 2.0
We haven't been good at tracking enum growth, but it can't hurt to try.
It might be worth changing this line to read:
# Since: 2.0, 'host_device', 'host_cdrom', 'host_floppy' since 2.1
> ##
> { 'enum': 'BlockdevDriver',
> - 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat',
> 'blkdebug',
> + 'data': [ 'file', 'host_device', 'host_cdrom', 'host_floppy',
Any reason you used _ instead of - in these names? Newer QMP tends to
prefer - unless there is a good reason why _ has already been baked in
due to back-compat.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH] block: Expose host_* drivers in blockdev-add
All the functionality to use the host_device, host_cdrom and host_floppy
drivers is already there, they just need to be added to the schema.
Signed-off-by: Kevin Wolf
---
qapi-schema.json | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 391356f..0fc0f12 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4288,7 +4288,8 @@
# Since: 2.0
##
{ 'enum': 'BlockdevDriver',
- 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat',
'blkdebug',
+ 'data': [ 'file', 'host_device', 'host_cdrom', 'host_floppy',
+'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'blkdebug',
'blkverify', 'bochs', 'cloop', 'cow', 'dmg', 'parallels', 'qcow',
'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'quorum' ] }
@@ -4555,6 +4556,9 @@
'discriminator': 'driver',
'data': {
'file': 'BlockdevOptionsFile',
+ 'host_device':'BlockdevOptionsFile',
+ 'host_cdrom': 'BlockdevOptionsFile',
+ 'host_floppy':'BlockdevOptionsFile',
'http': 'BlockdevOptionsFile',
'https': 'BlockdevOptionsFile',
'ftp':'BlockdevOptionsFile',
--
1.8.3.1
